| Message ID | 20250128143436.874357-3-nilay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: remove q->sysfs_dir_lock and fix race updating nr_hw_queue | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 1/28/25 15:34, Nilay Shroff wrote: > The nr_hw_queue update could potentially race with disk addtion/removal > while registering/unregistering hctx sysfs files. The __blk_mq_update_ > nr_hw_queues() runs with q->tag_list_lock held and so to avoid it racing > with disk addition/removal we should acquire q->tag_list_lock while > registering/unregistering hctx sysfs files. > > With this patch, blk_mq_sysfs_register() (called during disk addition) > and blk_mq_sysfs_unregister() (called during disk removal) now runs > with q->tag_list_lock held so that it avoids racing with __blk_mq_update > _nr_hw_queues(). > > Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> > --- > block/blk-mq-sysfs.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c > index 6113328abd70..3feeeccf8a99 100644 > --- a/block/blk-mq-sysfs.c > +++ b/block/blk-mq-sysfs.c > @@ -225,25 +225,25 @@ int blk_mq_sysfs_register(struct gendisk *disk) > > ret = kobject_add(q->mq_kobj, &disk_to_dev(disk)->kobj, "mq"); > if (ret < 0) > - goto out; > + return ret; > > kobject_uevent(q->mq_kobj, KOBJ_ADD); > > + mutex_lock(&q->tag_set->tag_list_lock); Maybe a comment here to indicate that it prevents a race? > queue_for_each_hw_ctx(q, hctx, i) { > ret = blk_mq_register_hctx(hctx); > if (ret) > - goto unreg; > + goto out_unreg; > } > + mutex_unlock(&q->tag_set->tag_list_lock); > + return 0; > > - > -out: > - return ret; > - > -unreg: > +out_unreg: > queue_for_each_hw_ctx(q, hctx, j) { > if (j < i) > blk_mq_unregister_hctx(hctx); > } > + mutex_unlock(&q->tag_set->tag_list_lock); > > kobject_uevent(q->mq_kobj, KOBJ_REMOVE); > kobject_del(q->mq_kobj); > @@ -256,9 +256,10 @@ void blk_mq_sysfs_unregister(struct gendisk *disk) > struct blk_mq_hw_ctx *hctx; > unsigned long i; > > - > + mutex_lock(&q->tag_set->tag_list_lock); Similar here. > queue_for_each_hw_ctx(q, hctx, i) > blk_mq_unregister_hctx(hctx); > + mutex_unlock(&q->tag_set->tag_list_lock); > > kobject_uevent(q->mq_kobj, KOBJ_REMOVE); > kobject_del(q->mq_kobj); Otherwise looks good. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c index 6113328abd70..3feeeccf8a99 100644 --- a/block/blk-mq-sysfs.c +++ b/block/blk-mq-sysfs.c @@ -225,25 +225,25 @@ int blk_mq_sysfs_register(struct gendisk *disk) ret = kobject_add(q->mq_kobj, &disk_to_dev(disk)->kobj, "mq"); if (ret < 0) - goto out; + return ret; kobject_uevent(q->mq_kobj, KOBJ_ADD); + mutex_lock(&q->tag_set->tag_list_lock); queue_for_each_hw_ctx(q, hctx, i) { ret = blk_mq_register_hctx(hctx); if (ret) - goto unreg; + goto out_unreg; } + mutex_unlock(&q->tag_set->tag_list_lock); + return 0; - -out: - return ret; - -unreg: +out_unreg: queue_for_each_hw_ctx(q, hctx, j) { if (j < i) blk_mq_unregister_hctx(hctx); } + mutex_unlock(&q->tag_set->tag_list_lock); kobject_uevent(q->mq_kobj, KOBJ_REMOVE); kobject_del(q->mq_kobj); @@ -256,9 +256,10 @@ void blk_mq_sysfs_unregister(struct gendisk *disk) struct blk_mq_hw_ctx *hctx; unsigned long i; - + mutex_lock(&q->tag_set->tag_list_lock); queue_for_each_hw_ctx(q, hctx, i) blk_mq_unregister_hctx(hctx); + mutex_unlock(&q->tag_set->tag_list_lock); kobject_uevent(q->mq_kobj, KOBJ_REMOVE); kobject_del(q->mq_kobj);
The nr_hw_queue update could potentially race with disk addtion/removal while registering/unregistering hctx sysfs files. The __blk_mq_update_ nr_hw_queues() runs with q->tag_list_lock held and so to avoid it racing with disk addition/removal we should acquire q->tag_list_lock while registering/unregistering hctx sysfs files. With this patch, blk_mq_sysfs_register() (called during disk addition) and blk_mq_sysfs_unregister() (called during disk removal) now runs with q->tag_list_lock held so that it avoids racing with __blk_mq_update _nr_hw_queues(). Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> --- block/blk-mq-sysfs.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)