diff mbox series

[3/3] block: use blk_mq_no_io() for avoiding lock dependency

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

Commit Message

Ming Lei April 2, 2025, 4:38 a.m. UTC
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(-)

Comments

Nilay Shroff April 2, 2025, 1:43 p.m. UTC | #1
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>
Ming Lei April 3, 2025, 2:54 a.m. UTC | #2
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 mbox series

Patch

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;
 }