Skip to content

Commit 4ff58d6

Browse files
kawasakiaxboe
authored andcommitted
block: serialize elevator changes for the same queue using a writer lock
When elevator_change() is called concurrently for the same queue, the elevator_change_done() function runs concurrently as well. This function adds or deletes kobjects for the debugfs entry of the queue. Then the concurrent calls cause memory corruption of the kobjects and result in a process hang. The core part of the elevator switch is protected by queue freeze and q->elevator_lock. However, since the commit 559dc11 ("block: move elv_register[unregister]_queue out of elevator_lock"), the elevator_change_done() is not serialized. Hence the memory corruption and the hang. The failures are observed when udev-worker writes to a sysfs queue/scheduler attribute file while the blktests test case block/005 writes to the same attribute file. The failure also can be recreated by running two processes that write to the same queue/scheduler file concurrently. The failure is observed since another commit 370ac28 ("block: avoid cpu_hotplug_lock depedency on freeze_lock"). This commit changed the behavior of queue freeze and it unveiled the failure. Fix the failure by changing elv_iosched_store() to acquire update_nr_hwq_lock as the writer lock instead of the reader lock. This serializes the whole elevator switch steps, including the elevator_change_done() call. Fixes: 559dc11 ("block: move elv_register[unregister]_queue out of elevator_lock") Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> Reviewed-by: Ming Lei <tom.leiming@gmail.com> Link: https://patch.msgid.link/20260716092237.1305030-1-shinichiro.kawasaki@wdc.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent bd2df8d commit 4ff58d6

1 file changed

Lines changed: 7 additions & 2 deletions

File tree

block/elevator.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -812,8 +812,13 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
812812
* reference during concurrent disk deletion:
813813
* update_nr_hwq_lock -> kn->active (via del_gendisk -> kobject_del)
814814
* kn->active -> update_nr_hwq_lock (via this sysfs write path)
815+
*
816+
* Use the writer lock instead of the reader lock of update_nr_hwq_lock
817+
* to serialize the two-stage elevator switch steps in
818+
* elevator_change(): the core switch step under the elevator lock and
819+
* the elevator_change_done() step outside the elevator lock.
815820
*/
816-
if (!down_read_trylock(&set->update_nr_hwq_lock)) {
821+
if (!down_write_trylock(&set->update_nr_hwq_lock)) {
817822
ret = -EBUSY;
818823
goto out;
819824
}
@@ -824,7 +829,7 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
824829
} else {
825830
ret = -ENOENT;
826831
}
827-
up_read(&set->update_nr_hwq_lock);
832+
up_write(&set->update_nr_hwq_lock);
828833

829834
out:
830835
if (ctx.type)

0 commit comments

Comments
 (0)