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 559dc11143eb
("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 370ac285f23a
("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: 559dc11143eb ("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>

diff --git a/block/elevator.c b/block/elevator.c
index 3bcd37c2aa34012c9c60ef0167c56fe9b82d7a2c..2161b6eea680ca28f396788d0fb7f142144b67e3 100644 (file)
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -812,8 +812,13 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
         * reference during concurrent disk deletion:
         *   update_nr_hwq_lock -> kn->active (via del_gendisk -> kobject_del)
         *   kn->active -> update_nr_hwq_lock (via this sysfs write path)
+        *
+        * Use the writer lock instead of the reader lock of update_nr_hwq_lock
+        * to serialize the two-stage elevator switch steps in
+        * elevator_change(): the core switch step under the elevator lock and
+        * the elevator_change_done() step outside the elevator lock.
         */
-       if (!down_read_trylock(&set->update_nr_hwq_lock)) {
+       if (!down_write_trylock(&set->update_nr_hwq_lock)) {
                ret = -EBUSY;
                goto out;
        }
@@ -824,7 +829,7 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
        } else {
                ret = -ENOENT;
        }
-       up_read(&set->update_nr_hwq_lock);
+       up_write(&set->update_nr_hwq_lock);
 
 out:
        if (ctx.type)