Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

block: get rid of request queue ->sysfs_dir_lock

The request queue uses ->sysfs_dir_lock for protecting the addition/
deletion of kobject entries under sysfs while we register/unregister
blk-mq. However kobject addition/deletion is already protected with
kernfs/sysfs internal synchronization primitives. So use of q->sysfs_
dir_lock seems redundant.

Moreover, q->sysfs_dir_lock is also used at few other callsites along
with q->sysfs_lock for protecting the addition/deletion of kojects.
One such example is when we register with sysfs a set of independent
access ranges for a disk. Here as well we could get rid off q->sysfs_
dir_lock and only use q->sysfs_lock.

The only variable which q->sysfs_dir_lock appears to protect is q->
mq_sysfs_init_done which is set/unset while registering/unregistering
blk-mq with sysfs. But use of q->mq_sysfs_init_done could be easily
replaced using queue registered bit QUEUE_FLAG_REGISTERED.

So with this patch we remove q->sysfs_dir_lock from each callsite
and replace q->mq_sysfs_init_done using QUEUE_FLAG_REGISTERED.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Link: https://lore.kernel.org/r/20250128143436.874357-2-nilay@linux.ibm.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Nilay Shroff and committed by
Jens Axboe
fe662860 5aa21b04

+5 -31
-1
block/blk-core.c
··· 430 430 refcount_set(&q->refs, 1); 431 431 mutex_init(&q->debugfs_mutex); 432 432 mutex_init(&q->sysfs_lock); 433 - mutex_init(&q->sysfs_dir_lock); 434 433 mutex_init(&q->limits_lock); 435 434 mutex_init(&q->rq_qos_mutex); 436 435 spin_lock_init(&q->queue_lock);
-4
block/blk-ia-ranges.c
··· 111 111 struct request_queue *q = disk->queue; 112 112 int i, ret; 113 113 114 - lockdep_assert_held(&q->sysfs_dir_lock); 115 114 lockdep_assert_held(&q->sysfs_lock); 116 115 117 116 if (!iars) ··· 154 155 struct blk_independent_access_ranges *iars = disk->ia_ranges; 155 156 int i; 156 157 157 - lockdep_assert_held(&q->sysfs_dir_lock); 158 158 lockdep_assert_held(&q->sysfs_lock); 159 159 160 160 if (!iars) ··· 287 289 { 288 290 struct request_queue *q = disk->queue; 289 291 290 - mutex_lock(&q->sysfs_dir_lock); 291 292 mutex_lock(&q->sysfs_lock); 292 293 if (iars && !disk_check_ia_ranges(disk, iars)) { 293 294 kfree(iars); ··· 310 313 disk_register_independent_access_ranges(disk); 311 314 unlock: 312 315 mutex_unlock(&q->sysfs_lock); 313 - mutex_unlock(&q->sysfs_dir_lock); 314 316 } 315 317 EXPORT_SYMBOL_GPL(disk_set_independent_access_ranges);
+5 -18
block/blk-mq-sysfs.c
··· 223 223 unsigned long i, j; 224 224 int ret; 225 225 226 - lockdep_assert_held(&q->sysfs_dir_lock); 227 - 228 226 ret = kobject_add(q->mq_kobj, &disk_to_dev(disk)->kobj, "mq"); 229 227 if (ret < 0) 230 228 goto out; ··· 235 237 goto unreg; 236 238 } 237 239 238 - q->mq_sysfs_init_done = true; 239 240 240 241 out: 241 242 return ret; ··· 256 259 struct blk_mq_hw_ctx *hctx; 257 260 unsigned long i; 258 261 259 - lockdep_assert_held(&q->sysfs_dir_lock); 260 262 261 263 queue_for_each_hw_ctx(q, hctx, i) 262 264 blk_mq_unregister_hctx(hctx); 263 265 264 266 kobject_uevent(q->mq_kobj, KOBJ_REMOVE); 265 267 kobject_del(q->mq_kobj); 266 - 267 - q->mq_sysfs_init_done = false; 268 268 } 269 269 270 270 void blk_mq_sysfs_unregister_hctxs(struct request_queue *q) ··· 269 275 struct blk_mq_hw_ctx *hctx; 270 276 unsigned long i; 271 277 272 - mutex_lock(&q->sysfs_dir_lock); 273 - if (!q->mq_sysfs_init_done) 274 - goto unlock; 278 + if (!blk_queue_registered(q)) 279 + return; 275 280 276 281 queue_for_each_hw_ctx(q, hctx, i) 277 282 blk_mq_unregister_hctx(hctx); 278 - 279 - unlock: 280 - mutex_unlock(&q->sysfs_dir_lock); 281 283 } 282 284 283 285 int blk_mq_sysfs_register_hctxs(struct request_queue *q) ··· 282 292 unsigned long i; 283 293 int ret = 0; 284 294 285 - mutex_lock(&q->sysfs_dir_lock); 286 - if (!q->mq_sysfs_init_done) 287 - goto unlock; 295 + if (!blk_queue_registered(q)) 296 + goto out; 288 297 289 298 queue_for_each_hw_ctx(q, hctx, i) { 290 299 ret = blk_mq_register_hctx(hctx); ··· 291 302 break; 292 303 } 293 304 294 - unlock: 295 - mutex_unlock(&q->sysfs_dir_lock); 296 - 305 + out: 297 306 return ret; 298 307 }
-5
block/blk-sysfs.c
··· 764 764 struct request_queue *q = disk->queue; 765 765 int ret; 766 766 767 - mutex_lock(&q->sysfs_dir_lock); 768 767 kobject_init(&disk->queue_kobj, &blk_queue_ktype); 769 768 ret = kobject_add(&disk->queue_kobj, &disk_to_dev(disk)->kobj, "queue"); 770 769 if (ret < 0) ··· 804 805 if (q->elevator) 805 806 kobject_uevent(&q->elevator->kobj, KOBJ_ADD); 806 807 mutex_unlock(&q->sysfs_lock); 807 - mutex_unlock(&q->sysfs_dir_lock); 808 808 809 809 /* 810 810 * SCSI probing may synchronously create and destroy a lot of ··· 828 830 mutex_unlock(&q->sysfs_lock); 829 831 out_put_queue_kobj: 830 832 kobject_put(&disk->queue_kobj); 831 - mutex_unlock(&q->sysfs_dir_lock); 832 833 return ret; 833 834 } 834 835 ··· 858 861 blk_queue_flag_clear(QUEUE_FLAG_REGISTERED, q); 859 862 mutex_unlock(&q->sysfs_lock); 860 863 861 - mutex_lock(&q->sysfs_dir_lock); 862 864 /* 863 865 * Remove the sysfs attributes before unregistering the queue data 864 866 * structures that can be modified through sysfs. ··· 874 878 /* Now that we've deleted all child objects, we can delete the queue. */ 875 879 kobject_uevent(&disk->queue_kobj, KOBJ_REMOVE); 876 880 kobject_del(&disk->queue_kobj); 877 - mutex_unlock(&q->sysfs_dir_lock); 878 881 879 882 blk_debugfs_remove(disk); 880 883 }
-3
include/linux/blkdev.h
··· 561 561 struct list_head flush_list; 562 562 563 563 struct mutex sysfs_lock; 564 - struct mutex sysfs_dir_lock; 565 564 struct mutex limits_lock; 566 565 567 566 /* ··· 604 605 * Serializes all debugfs metadata operations using the above dentries. 605 606 */ 606 607 struct mutex debugfs_mutex; 607 - 608 - bool mq_sysfs_init_done; 609 608 }; 610 609 611 610 /* Keep blk_queue_flag_name[] in sync with the definitions below */