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

On Fri, Sep 5, 2014 at 11:50 AM, Johannes Berg johannes@sipsolutions.net wrote:

From: Johannes Berg johannes.berg@intel.com

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);