diff options
| author | Nilay Shroff <nilay@linux.ibm.com> | 2025-03-04 15:52:33 +0530 |
|---|---|---|
| committer | Jens Axboe <axboe@kernel.dk> | 2025-03-10 07:30:18 -0600 |
| commit | 1bf70d08cc3b55abd1763e6dff5855cb8dd8318b (patch) | |
| tree | 11ec9cc9c02828e4b9416163102eff1eee5b5ae3 /block/blk-sysfs.c | |
| parent | d23977fee1ee838316fb1b00945064a146460843 (diff) | |
| download | linux-1bf70d08cc3b55abd1763e6dff5855cb8dd8318b.tar.gz linux-1bf70d08cc3b55abd1763e6dff5855cb8dd8318b.tar.bz2 linux-1bf70d08cc3b55abd1763e6dff5855cb8dd8318b.zip | |
block: introduce a dedicated lock for protecting queue elevator updates
A queue's elevator can be updated either when modifying nr_hw_queues
or through the sysfs scheduler attribute. Currently, elevator switching/
updating is protected using q->sysfs_lock, but this has led to lockdep
splats[1] due to inconsistent lock ordering between q->sysfs_lock and
the freeze-lock in multiple block layer call sites.
As the scope of q->sysfs_lock is not well-defined, its (mis)use has
resulted in numerous lockdep warnings. To address this, introduce a new
q->elevator_lock, dedicated specifically for protecting elevator
switches/updates. And we'd now use this new q->elevator_lock instead of
q->sysfs_lock for protecting elevator switches/updates.
While at it, make elv_iosched_load_module() a static function, as it is
only called from elv_iosched_store(). Also, remove redundant parameters
from elv_iosched_load_module() function signature.
[1] https://lore.kernel.org/all/67637e70.050a0220.3157ee.000c.GAE@google.com/
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://lore.kernel.org/r/20250304102551.2533767-5-nilay@linux.ibm.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Diffstat (limited to 'block/blk-sysfs.c')
| -rw-r--r-- | block/blk-sysfs.c | 32 |
1 files changed, 22 insertions, 10 deletions
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index bc641ac71cde..1562e22877e1 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -693,11 +693,16 @@ static struct attribute *blk_mq_queue_attrs[] = { * Attributes which are protected with q->sysfs_lock. */ &queue_requests_entry.attr, - &elv_iosched_entry.attr, #ifdef CONFIG_BLK_WBT &queue_wb_lat_entry.attr, #endif /* + * Attributes which require some form of locking other than + * q->sysfs_lock. + */ + &elv_iosched_entry.attr, + + /* * Attributes which don't require locking. */ &queue_rq_affinity_entry.attr, @@ -865,15 +870,19 @@ int blk_register_queue(struct gendisk *disk) if (ret) goto out_debugfs_remove; + ret = blk_crypto_sysfs_register(disk); + if (ret) + goto out_unregister_ia_ranges; + + mutex_lock(&q->elevator_lock); if (q->elevator) { ret = elv_register_queue(q, false); - if (ret) - goto out_unregister_ia_ranges; + if (ret) { + mutex_unlock(&q->elevator_lock); + goto out_crypto_sysfs_unregister; + } } - - ret = blk_crypto_sysfs_register(disk); - if (ret) - goto out_elv_unregister; + mutex_unlock(&q->elevator_lock); blk_queue_flag_set(QUEUE_FLAG_REGISTERED, q); wbt_enable_default(disk); @@ -898,8 +907,8 @@ int blk_register_queue(struct gendisk *disk) return ret; -out_elv_unregister: - elv_unregister_queue(q); +out_crypto_sysfs_unregister: + blk_crypto_sysfs_unregister(disk); out_unregister_ia_ranges: disk_unregister_independent_access_ranges(disk); out_debugfs_remove: @@ -945,8 +954,11 @@ void blk_unregister_queue(struct gendisk *disk) blk_mq_sysfs_unregister(disk); blk_crypto_sysfs_unregister(disk); - mutex_lock(&q->sysfs_lock); + mutex_lock(&q->elevator_lock); elv_unregister_queue(q); + mutex_unlock(&q->elevator_lock); + + mutex_lock(&q->sysfs_lock); disk_unregister_independent_access_ranges(disk); mutex_unlock(&q->sysfs_lock); |