On Sat, 2020-04-18 at 12:13 -0700, Matthew Wilcox wrote: > > > > > > if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, > > > > > "failing_device")) > > > > > - /* nothing - symlink will be missing */; > > > > > + do_empty(); /* nothing - symlink will be missing */ > > > > > > > > > > if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, > > > > > "devcoredump")) > > > > > - /* nothing - symlink will be missing */; > > > > > + do_empty(); /* nothing - symlink will be missing */ > > > > > > > > > > INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); > > > > > schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT); > > > > > > > > Could just remove the 'if's? > > > > > > > > + sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, > > > > + "failing_device"); > > > > > > > > > > OK. > > > > sysfs_create_link is __must_check > > Oh, I missed the declaration -- I just saw the definition. This is a > situation where __must_check hurts us and it should be removed. > > Or this code is wrong and it should be > > WARN(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, > "failing_device"); Perhaps it should be. I didn't think it really mattered _that_ much if the symlink suddenly went missing, but OTOH I don't even know how it could possibly fail. johannes
