Hi,
After we modeled the freeze & enter queue as lock for supporting lockdep
under commit f1be1788a32e ("block: model freeze & enter queue as lock
for supporting lockdep"), we received numerous lockdep splats. And one
of those splats[1] reported the potential deadlock due to incorrect lock
ordering issue between q->sysfs-lock and q->q_usage_counter. So some of
the patches in this series are aimed to cut the dependency between q->
sysfs-lock and q->q_usage_counter.
This patchset contains six patches in the series.
The 1st patch removes the q->sysfs_lock for all sysfs attributes which
don't need it. We identified all sysfs attributes which don't need any
locking and all such attributes have been now grouped in queue_attr_show
/queue_attr_store under entry->show_nolock/entry->store_nolock methods.
The 2nd patch helps acquire q->limits_lock instead of q->sysfs_lock while
reading a set of attributes whose write method is protected with atomic
limit update APIs or updates to these attributes could occur under atomic
limit update APIs such as queue_limit_start_update() and queue_limits_
commit_update(). So all such attributes have been now grouped in queue_
attr_show under entry->show_limit method.
Subsequent patches address remaining attributes individually and group
them in queue_attr_show/queue_attr_store under entry->show/entry->store
method which require some form of locking other than q->limits_lock or
q->sysfs_lock.
The 3rd patch introduce a new dedicated lock for elevator switch/update
and thus eliminates the dependecy of sched update on q->sysfs_lock.
The 4th patch protects sysfs attribute nr_requests using q->elevator_lock
instead of q->sysfs_lock as the update to q->nr_requests now happen under
q->elevator_lock.
Similarly, the 5th patch protects sysfs attribute wbt_lat_usec using
q->elevator_lock instead of q->sysfs_lock as the update to wbt state and
latency now happen under q->elevator_lock.
The 6th patch protects read_ahead_kb using q->limits_lock instead of
q->sysfs_lock as update to bdi->ra_pages could happen using atomic limit
update APIs. Ideally we should have grouped this attribute in queue_attr_
show/queue_attr_store under entry->show_limit/entry->store_limit method.
However we don't use atomic update helper APIs queue_limits_start_update()
and queue_limits_commit_update() here bacause blk_apply_bdi_limits() which
is invoked from queue_limits_commit_update() can overwrite the bdi->ra_
pages value which user actaully wants to store using this attribute. The
blk_apply_bdi_limits() sets value of bdi->ra_pages based on the optimal
I/O size(io_opt). So we choose instead to update this attribute value
outside of using atomic limit update APIs.
Please note that above changes were unit tested against blktests and
quick xfstests with lockdep enabled.
[1] https://lore.kernel.org/all/67637e70.050a0220.3157ee.000c.GAE@xxxxxxxxxx/
Nilay Shroff (6):
blk-sysfs: remove q->sysfs_lock for attributes which don't need it
blk-sysfs: acquire q->limits_lock while reading attributes
block: Introduce a dedicated lock for protecting queue elevator
updates
blk-sysfs: protect nr_requests update using q->elevator_lock
blk-sysfs: protect wbt_lat_usec using q->elevator_lock
blk-sysfs: protect read_ahead_kb using q->limits_lock
---
Changes from v1:
- Audit all sysfs attributes in block layer and find attributes which
don't need any locking as well as attributes which needs some form of
locking; then remove locking from queue_attr_store/queue_attr_show and
move it into the attributes that still need it in some form, followed
by replacing it with the more suitable locks (hch)
- Use dedicated lock for elevator switch/update (Ming Lei)
- Re-arrange patchset to first segregate and group together all
attributes which don't need locking followed by grouping attributes
which need some form of locking.
Link to v1: https://lore.kernel.org/all/20250205144506.663819-1-nilay@xxxxxxxxxxxxx/
---
block/blk-core.c | 1 +
block/blk-mq.c | 12 +-
block/blk-settings.c | 2 +-
block/blk-sysfs.c | 324 ++++++++++++++++++++++++++++-------------
block/elevator.c | 18 ++-
block/genhd.c | 9 +-
include/linux/blkdev.h | 1 +
7 files changed, 254 insertions(+), 113 deletions(-)
--
2.47.1
