Re: [RFC v2] device coredump: add new device coredump class

[Arik Nemtsov]

On Fri, Sep 5, 2014 at 11:50 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
> From: Johannes Berg <johannes.berg@xxxxxxxxx>
>
> Many devices run firmware and/or complex hardware, and most of that
> can have bugs. When it misbehaves, however, it is often much harder
> to debug than software running on the host.
>

Very nice concept IMO :)

[....]
> +void dev_coredumpm(struct device *dev, struct module *owner,
> +                  const void *data, size_t datalen, gfp_t gfp,
> +                  ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +                                  const void *data, size_t datalen),
> +                  void (*free)(const void *data))
> +{
> +       static atomic_t devcd_count = ATOMIC_INIT(0);
> +       struct devcd_entry *devcd;
> +       struct device *existing;
> +
> +       existing = class_find_device(&devcd_class, NULL, dev,
> +                                    devcd_match_failing);
> +       if (existing) {
> +               put_device(existing);
> +               return;
> +       }
> +
> +       if (!try_module_get(owner))
> +               return;
> +
> +       devcd = kzalloc(sizeof(*devcd), gfp);
> +       if (!devcd)
> +               goto put_module;
> +
> +       devcd->owner = owner;
> +       devcd->data = data;
> +       devcd->datalen = datalen;
> +       devcd->read = read;
> +       devcd->free = free;
> +       devcd->failing_dev = get_device(dev);
> +
> +       device_initialize(&devcd->devcd_dev);
> +
> +       dev_set_name(&devcd->devcd_dev, "devcd%d",
> +                    atomic_inc_return(&devcd_count));
> +       devcd->devcd_dev.class = &devcd_class;
> +
> +       if (device_add(&devcd->devcd_dev))
> +               goto put_device;
> +
> +       if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> +                             "failing_device"))
> +               /* nothing - symlink will be missing */;
> +
> +       if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> +                             "dev_coredump"))
> +               /* nothing - symlink will be missing */;

Aren't you confusing the compiler here and above, doing the
INIT_DELAYED_WORK only when link creation failed? I would have thought
some braces are necessary..

> +
> +       INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> +       schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> +
> +       return;
> + put_device:
> +       put_device(&devcd->devcd_dev);
> + put_module:
> +       module_put(owner);
> +}
> +EXPORT_SYMBOL(dev_coredumpm);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/