* two fixup for the block_device / hd_struct merge @ 2021-07-01 8:16 Christoph Hellwig 2021-07-01 8:16 ` [PATCH 1/2] block: grab a device refcount in disk_uevent Christoph Hellwig ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Christoph Hellwig @ 2021-07-01 8:16 UTC (permalink / raw) To: axboe; +Cc: linux-block Hi Jens, find two fixes for the block_device / hd_struct merge series attached. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 8:16 two fixup for the block_device / hd_struct merge Christoph Hellwig @ 2021-07-01 8:16 ` Christoph Hellwig 2021-07-01 8:54 ` Ming Lei 2021-07-01 8:16 ` [PATCH 2/2] block: remove the bdgrab in blk_drop_partitions Christoph Hellwig 2021-07-01 16:21 ` two fixup for the block_device / hd_struct merge Jens Axboe 2 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2021-07-01 8:16 UTC (permalink / raw) To: axboe; +Cc: linux-block 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@lst.de> --- 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; rcu_read_unlock(); kobject_uevent(bdev_kobj(part), action); - bdput(part); + put_device(&part->bd_device); rcu_read_lock(); } rcu_read_unlock(); -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 8:16 ` [PATCH 1/2] block: grab a device refcount in disk_uevent Christoph Hellwig @ 2021-07-01 8:54 ` Ming Lei 2021-07-01 9:02 ` Christoph Hellwig 0 siblings, 1 reply; 9+ messages in thread From: Ming Lei @ 2021-07-01 8:54 UTC (permalink / raw) To: Christoph Hellwig; +Cc: axboe, linux-block 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@lst.de> > --- > 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? Thanks, Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 8:54 ` Ming Lei @ 2021-07-01 9:02 ` Christoph Hellwig 2021-07-01 9:17 ` Ming Lei 0 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2021-07-01 9:02 UTC (permalink / raw) To: Ming Lei; +Cc: Christoph Hellwig, axboe, linux-block 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@lst.de> > > --- > > 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 9:02 ` Christoph Hellwig @ 2021-07-01 9:17 ` Ming Lei 2021-07-01 9:27 ` Ming Lei 0 siblings, 1 reply; 9+ messages in thread From: Ming Lei @ 2021-07-01 9:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: axboe, linux-block 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@lst.de> > > > --- > > > 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. Thanks, Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 9:17 ` Ming Lei @ 2021-07-01 9:27 ` Ming Lei 2021-07-01 9:49 ` Christoph Hellwig 0 siblings, 1 reply; 9+ messages in thread From: Ming Lei @ 2021-07-01 9:27 UTC (permalink / raw) To: Christoph Hellwig; +Cc: axboe, linux-block 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@lst.de> > > > > --- > > > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: grab a device refcount in disk_uevent 2021-07-01 9:27 ` Ming Lei @ 2021-07-01 9:49 ` Christoph Hellwig 0 siblings, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2021-07-01 9:49 UTC (permalink / raw) To: Ming Lei; +Cc: Christoph Hellwig, axboe, linux-block On Thu, Jul 01, 2021 at 05:27:37PM +0800, Ming Lei wrote: > > 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. Yes, sorry. Braino while looking up the cause. But there isn't much of a point in sending a uevent to a partition already deleted, so I think the unless_zero here still makes sense. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] block: remove the bdgrab in blk_drop_partitions 2021-07-01 8:16 two fixup for the block_device / hd_struct merge Christoph Hellwig 2021-07-01 8:16 ` [PATCH 1/2] block: grab a device refcount in disk_uevent Christoph Hellwig @ 2021-07-01 8:16 ` Christoph Hellwig 2021-07-01 16:21 ` two fixup for the block_device / hd_struct merge Jens Axboe 2 siblings, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2021-07-01 8:16 UTC (permalink / raw) To: axboe; +Cc: linux-block There is no need to hold a bdev reference when removing the partition. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/partitions/core.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/block/partitions/core.c b/block/partitions/core.c index 347c56a51d87..3907cebd2be9 100644 --- a/block/partitions/core.c +++ b/block/partitions/core.c @@ -537,12 +537,8 @@ void blk_drop_partitions(struct gendisk *disk) lockdep_assert_held(&disk->open_mutex); - xa_for_each_start(&disk->part_tbl, idx, part, 1) { - if (!bdgrab(part)) - continue; + xa_for_each_start(&disk->part_tbl, idx, part, 1) delete_partition(part); - bdput(part); - } } static bool blk_add_partition(struct gendisk *disk, -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: two fixup for the block_device / hd_struct merge 2021-07-01 8:16 two fixup for the block_device / hd_struct merge Christoph Hellwig 2021-07-01 8:16 ` [PATCH 1/2] block: grab a device refcount in disk_uevent Christoph Hellwig 2021-07-01 8:16 ` [PATCH 2/2] block: remove the bdgrab in blk_drop_partitions Christoph Hellwig @ 2021-07-01 16:21 ` Jens Axboe 2 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2021-07-01 16:21 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-block On 7/1/21 2:16 AM, Christoph Hellwig wrote: > Hi Jens, > > find two fixes for the block_device / hd_struct merge series attached. Applied, thanks. -- Jens Axboe ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-01 16:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-01 8:16 two fixup for the block_device / hd_struct merge Christoph Hellwig 2021-07-01 8:16 ` [PATCH 1/2] block: grab a device refcount in disk_uevent Christoph Hellwig 2021-07-01 8:54 ` Ming Lei 2021-07-01 9:02 ` Christoph Hellwig 2021-07-01 9:17 ` Ming Lei 2021-07-01 9:27 ` Ming Lei 2021-07-01 9:49 ` Christoph Hellwig 2021-07-01 8:16 ` [PATCH 2/2] block: remove the bdgrab in blk_drop_partitions Christoph Hellwig 2021-07-01 16:21 ` two fixup for the block_device / hd_struct merge Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.