Message ID | 20250402043851.946498-4-ming.lei@redhat.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | block: fix lock dependency between freeze and elevator lock | expand |
On 4/2/25 10:08 AM, Ming Lei wrote: > Use blk_mq_no_io() to prevent IO from entering queue for avoiding lock > dependency between freeze lock and elevator lock, and we have got many > such reports: > > Reported-by: syzbot+4c7e0f9b94ad65811efb@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-block/67e6b425.050a0220.2f068f.007b.GAE@google.com/ > Reported-by: Valdis Klētnieks <valdis.kletnieks@vt.edu> > Closes: https://lore.kernel.org/linux-block/7755.1743228130@turing-police/#t > Signed-off-by: Ming Lei <ming.lei@redhat.com> I tested this series on my system and this works well as we cut dependency between ->elevator_lock and ->freeze_lock. However don't we plan to now model blk_mq_enter_no_io and blk_mq_exit_no_io as lock/unlock for supporting lockdep? Maybe we don't. Overall changes looks good to me: Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
On Wed, Apr 02, 2025 at 07:13:56PM +0530, Nilay Shroff wrote: > > > On 4/2/25 10:08 AM, Ming Lei wrote: > > Use blk_mq_no_io() to prevent IO from entering queue for avoiding lock > > dependency between freeze lock and elevator lock, and we have got many > > such reports: > > > > Reported-by: syzbot+4c7e0f9b94ad65811efb@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/linux-block/67e6b425.050a0220.2f068f.007b.GAE@google.com/ > > Reported-by: Valdis Klētnieks <valdis.kletnieks@vt.edu> > > Closes: https://lore.kernel.org/linux-block/7755.1743228130@turing-police/#t > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > I tested this series on my system and this works well as we cut dependency > between ->elevator_lock and ->freeze_lock. However don't we plan to now > model blk_mq_enter_no_io and blk_mq_exit_no_io as lock/unlock for supporting > lockdep? Maybe we don't. Good point! > > Overall changes looks good to me: > Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> Thanks for the review! lockdep modeling for blk_mq_enter_no_io and blk_mq_exit_no_io has been added in V2. Thanks, Ming
diff --git a/block/blk-mq.c b/block/blk-mq.c index 075ee51066b3..022d8139910d 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -4882,9 +4882,6 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr) int ret; unsigned long i; - if (WARN_ON_ONCE(!q->mq_freeze_depth)) - return -EINVAL; - if (!set) return -EINVAL; @@ -5025,7 +5022,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, memflags = memalloc_noio_save(); list_for_each_entry(q, &set->tag_list, tag_set_list) - blk_mq_freeze_queue_nomemsave(q); + blk_mq_enter_no_io(q); /* * Switch IO scheduler to 'none', cleaning up the data associated @@ -5074,7 +5071,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, blk_mq_elv_switch_back(&head, q); list_for_each_entry(q, &set->tag_list, tag_set_list) - blk_mq_unfreeze_queue_nomemrestore(q); + blk_mq_exit_no_io(q); memalloc_noio_restore(memflags); /* Free the excess tags when nr_hw_queues shrink. */ diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index a2882751f0d2..e866875c17be 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -76,7 +76,7 @@ queue_requests_store(struct gendisk *disk, const char *page, size_t count) if (ret < 0) return ret; - memflags = blk_mq_freeze_queue(q); + memflags = blk_mq_enter_no_io_memsave(q); mutex_lock(&q->elevator_lock); if (nr < BLKDEV_MIN_RQ) nr = BLKDEV_MIN_RQ; @@ -85,7 +85,7 @@ queue_requests_store(struct gendisk *disk, const char *page, size_t count) if (err) ret = err; mutex_unlock(&q->elevator_lock); - blk_mq_unfreeze_queue(q, memflags); + blk_mq_exit_no_io_memrestore(q, memflags); return ret; } @@ -592,7 +592,7 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page, if (val < -1) return -EINVAL; - memflags = blk_mq_freeze_queue(q); + memflags = blk_mq_enter_no_io_memsave(q); mutex_lock(&q->elevator_lock); rqos = wbt_rq_qos(q); @@ -623,7 +623,7 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page, blk_mq_unquiesce_queue(q); out: mutex_unlock(&q->elevator_lock); - blk_mq_unfreeze_queue(q, memflags); + blk_mq_exit_no_io_memrestore(q, memflags); return ret; } diff --git a/block/elevator.c b/block/elevator.c index 4d3a8f996c91..c9cb8386bf5e 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -724,13 +724,13 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf, elv_iosched_load_module(name); - memflags = blk_mq_freeze_queue(q); + memflags = blk_mq_enter_no_io_memsave(q); mutex_lock(&q->elevator_lock); ret = elevator_change(q, name); if (!ret) ret = count; mutex_unlock(&q->elevator_lock); - blk_mq_unfreeze_queue(q, memflags); + blk_mq_exit_no_io_memrestore(q, memflags); return ret; }
Use blk_mq_no_io() to prevent IO from entering queue for avoiding lock dependency between freeze lock and elevator lock, and we have got many such reports: Reported-by: syzbot+4c7e0f9b94ad65811efb@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-block/67e6b425.050a0220.2f068f.007b.GAE@google.com/ Reported-by: Valdis Klētnieks <valdis.kletnieks@vt.edu> Closes: https://lore.kernel.org/linux-block/7755.1743228130@turing-police/#t Signed-off-by: Ming Lei <ming.lei@redhat.com> --- block/blk-mq.c | 7 ++----- block/blk-sysfs.c | 8 ++++---- block/elevator.c | 4 ++-- 3 files changed, 8 insertions(+), 11 deletions(-)