On Thu, Jul 01, 2021 at 05:17:22PM +0800, Ming Lei wrote:
> On Thu, Jul 01, 2021 at 11:02:32AM +0200, Christoph Hellwig wrote:
> > On Thu, Jul 01, 2021 at 04:54:05PM +0800, Ming Lei wrote:
> > > On Thu, Jul 01, 2021 at 10:16:37AM +0200, Christoph Hellwig wrote:
> > > > Sending uevents requires the struct device to be alive. To
> > > > ensure that grab the device refcount instead of just an inode
> > > > reference.
> > > >
> > > > Fixes: bc359d03c7ec ("block: add a disk_uevent helper")
> > > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > > > ---
> > > > block/genhd.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/block/genhd.c b/block/genhd.c
> > > > index 79aa40b4c39c..af4d2ab4a633 100644
> > > > --- a/block/genhd.c
> > > > +++ b/block/genhd.c
> > > > @@ -365,12 +365,12 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action)
> > > > xa_for_each(&disk->part_tbl, idx, part) {
> > > > if (bdev_is_partition(part) && !bdev_nr_sectors(part))
> > > > continue;
> > > > - if (!bdgrab(part))
> > > > + if (!kobject_get_unless_zero(&part->bd_device.kobj))
> > > > continue;
> > >
> > > ->bd_device is embedded in the block device, and it has same lifetime
> > > with the block device, even part_release() calls bdput() to release this
> > > device, so why doesn't work by holding a inode reference?
> >
> > Because sending a uevent on a device that has device_del called on it
> > is going to blow up.
>
> But grabbing one reference can't prevent device_del() from being called.
>
> IMO, if driver core doesn't allow to sending uevent on one deleted device,
> it should return a failure instead of kernel panic.
Maybe the reason is that sending uevent needs the device/kobject not
'released' in viewpoint of driver core world, other device resource(such
as device name) may be referred but have been freed.
If that is the reason, this patch looks fine, and
kobject_get_unless_zero() may be replaced with get_device() since the
referred memory isn't freed yet.
Thanks,
Ming
