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 seven patches in the series.
The 1st 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 ->show_limit() method.
The 2nd patch is preparation to further simplify and group sysfs attributes
in subsequent patches. So in this patch we move acquire/release of q->sysfs
_lock as well as queue freeze/unfreeze under each attributes' respective
->show()/->store() method.
The 3rd 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 removed use of q->sysfs_lock for these attributes.
Subsequent patches address remaining attributes individually which require
some form of locking other than q->limits_lock or q->sysfs_lock.
The 4th patch introduce a new dedicated lock for elevator switch/update
and thus eliminates the dependency of sched update on q->sysfs_lock.
The 5th 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 6th 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 7th patch protects read_ahead_kb using q->limits_lock instead of
q->sysfs_lock as update to ->ra_pages is protected by ->limits_lock.
The ->ra_pages is usually calculated from the queue limits by queue_
limits_commit_update.
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/
---
Changes from v2:
- Add a prep patch (second patch in the series) to first move q->sysfs_
lock and queue freeze from queue_attr_show/queue_attr_store to show/
store method of the respective attribute; then in subsequent patches
we remove q->sysfs_lock from attributes that don't require any from of
locking or replace q->sysfs_lock with appropriate lock for attributes
that require some form of locking; this way we can get away with using
->show_nolock()/->store_nolock() methods (hch)
- Few other misc updates to commit message and code comments (hch)
- Rearrange the patchset to improve organization and clarity. Start with
a patch that groups all attributes requiring protection under ->limits_
lock. Follow this with a preparatory patch, then introduce a patch that
consolidates attributes that do not require any form of locking.
Finally, address the remaining attributes individually in subsequent
patches, which require some form of locking other than q->limits_lock
or q->sysfs_lock.
Link to v2: https://lore.kernel.org/all/20250218082908.265283-1-nilay@xxxxxxxxxxxxx/
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/
---
Nilay Shroff (7):
block: acquire q->limits_lock while reading sysfs attributes
block: move q->sysfs_lock and queue-freeze under show/store method
block: remove q->sysfs_lock for attributes which don't need it
block: Introduce a dedicated lock for protecting queue elevator
updates
block: protect nr_requests update using q->elevator_lock
block: protect wbt_lat_usec using q->elevator_lock
block: protect read_ahead_kb using q->limits_lock
block/blk-core.c | 1 +
block/blk-iocost.c | 2 +
block/blk-mq.c | 15 +-
block/blk-settings.c | 2 +-
block/blk-sysfs.c | 309 +++++++++++++++++++++++++++--------------
block/elevator.c | 43 ++++--
block/elevator.h | 2 -
block/genhd.c | 9 +-
include/linux/blkdev.h | 5 +
9 files changed, 251 insertions(+), 137 deletions(-)
--
2.47.1
