linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Simplify the code modifying queue attributes
@ 2025-07-02 20:38 Bart Van Assche
  2025-07-02 20:38 ` [PATCH 1/8] block: Introduce QUEUE_FLAG_FROZEN Bart Van Assche
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

Hi Jens,

The blk_mq_quiesce_queue() / blk_mq_unquiesce_queue() calls on frozen request
queues cause confusion and also make the code more verbose than necessary.
Hence this patch series that removes these quiesce / unquiesce calls. This
patch series should have no performance impact on the hot path and should not
modify the behavior of the block layer.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (8):
  block: Introduce QUEUE_FLAG_FROZEN
  block: Do not run frozen queues
  block: Remove the quiesce/unquiesce calls on frozen queues
  aoe: Remove the quiesce/unquiesce calls on frozen queues
  ataflop: Remove the quiesce/unquiesce calls on frozen queues
  sunvdc: Remove the quiesce/unquiesce calls on frozen queues
  swim3: Remove the quiesce/unquiesce calls on frozen queues
  mtd_blkdevs: Remove the quiesce/unquiesce calls on frozen queues

 block/blk-core.c           |  1 +
 block/blk-iocost.c         |  8 --------
 block/blk-mq.c             | 21 +++++++++------------
 block/blk-sysfs.c          | 13 +++++--------
 block/blk-throttle.c       |  2 --
 block/elevator.c           |  8 ++------
 drivers/block/aoe/aoedev.c |  5 +----
 drivers/block/ataflop.c    |  2 --
 drivers/block/sunvdc.c     |  5 +----
 drivers/block/swim3.c      |  2 --
 drivers/mtd/mtd_blkdevs.c  |  4 +---
 include/linux/blkdev.h     |  2 ++
 12 files changed, 22 insertions(+), 51 deletions(-)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/8] block: Introduce QUEUE_FLAG_FROZEN
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-02 20:38 ` [PATCH 2/8] block: Do not run frozen queues Bart Van Assche
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

Prepare for checking whether or not a request queue is frozen from the hot
path with a minimal performance overhead.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-core.c       | 1 +
 block/blk-mq.c         | 1 +
 include/linux/blkdev.h | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index fdac48aec5ef..001a4ac997ae 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -376,6 +376,7 @@ static void blk_queue_usage_counter_release(struct percpu_ref *ref)
 	struct request_queue *q =
 		container_of(ref, struct request_queue, q_usage_counter);
 
+	blk_queue_flag_set(QUEUE_FLAG_FROZEN, q);
 	wake_up_all(&q->mq_freeze_wq);
 }
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 0c61492724d2..7919607c1aeb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -218,6 +218,7 @@ bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic)
 	WARN_ON_ONCE(q->mq_freeze_depth < 0);
 	if (!q->mq_freeze_depth) {
 		percpu_ref_resurrect(&q->q_usage_counter);
+		blk_queue_flag_clear(QUEUE_FLAG_FROZEN, q);
 		wake_up_all(&q->mq_freeze_wq);
 	}
 	unfreeze = blk_unfreeze_check_owner(q);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index a51f92b6c340..77b9067d621a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -641,6 +641,7 @@ enum {
 	QUEUE_FLAG_INIT_DONE,		/* queue is initialized */
 	QUEUE_FLAG_STATS,		/* track IO start and completion times */
 	QUEUE_FLAG_REGISTERED,		/* queue has been registered to a disk */
+	QUEUE_FLAG_FROZEN,		/* queue has been frozen */
 	QUEUE_FLAG_QUIESCED,		/* queue has been quiesced */
 	QUEUE_FLAG_RQ_ALLOC_TIME,	/* record rq->alloc_time_ns */
 	QUEUE_FLAG_HCTX_ACTIVE,		/* at least one blk-mq hctx is active */
@@ -676,6 +677,7 @@ void blk_queue_flag_clear(unsigned int flag, struct request_queue *q);
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
 			     REQ_FAILFAST_DRIVER))
+#define blk_queue_frozen(q)	test_bit(QUEUE_FLAG_FROZEN, &(q)->queue_flags)
 #define blk_queue_quiesced(q)	test_bit(QUEUE_FLAG_QUIESCED, &(q)->queue_flags)
 #define blk_queue_pm_only(q)	atomic_read(&(q)->pm_only)
 #define blk_queue_registered(q)	test_bit(QUEUE_FLAG_REGISTERED, &(q)->queue_flags)

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/8] block: Do not run frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
  2025-07-02 20:38 ` [PATCH 1/8] block: Introduce QUEUE_FLAG_FROZEN Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-03  1:15   ` Yu Kuai
  2025-07-02 20:38 ` [PATCH 3/8] block: Remove the quiesce/unquiesce calls on " Bart Van Assche
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

If a request queue is frozen there are no requests pending and hence it
is not necessary to run a request queue.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7919607c1aeb..91b9fc1a7ddb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2298,16 +2298,16 @@ static inline bool blk_mq_hw_queue_need_run(struct blk_mq_hw_ctx *hctx)
 	bool need_run;
 
 	/*
-	 * When queue is quiesced, we may be switching io scheduler, or
-	 * updating nr_hw_queues, or other things, and we can't run queue
-	 * any more, even blk_mq_hctx_has_pending() can't be called safely.
-	 *
-	 * And queue will be rerun in blk_mq_unquiesce_queue() if it is
-	 * quiesced.
+	 * The request queue is frozen when the e.g. the I/O scheduler is
+	 * changed and also when nr_hw_queues is updated. Neither running the
+	 * queue nor calling blk_mq_hctx_has_pending() is safe during these
+	 * operations. Hence, check whether the queue is frozen before checking
+	 * whether any requests are pending.
 	 */
 	__blk_mq_run_dispatch_ops(hctx->queue, false,
-		need_run = !blk_queue_quiesced(hctx->queue) &&
-		blk_mq_hctx_has_pending(hctx));
+		need_run = !blk_queue_frozen(hctx->queue) &&
+			   !blk_queue_quiesced(hctx->queue) &&
+			   blk_mq_hctx_has_pending(hctx));
 	return need_run;
 }
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/8] block: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
  2025-07-02 20:38 ` [PATCH 1/8] block: Introduce QUEUE_FLAG_FROZEN Bart Van Assche
  2025-07-02 20:38 ` [PATCH 2/8] block: Do not run frozen queues Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-02 20:38 ` [PATCH 4/8] aoe: " Bart Van Assche
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche,
	Tejun Heo, Josef Bacik

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-iocost.c   |  8 --------
 block/blk-mq.c       |  4 ----
 block/blk-sysfs.c    | 13 +++++--------
 block/blk-throttle.c |  2 --
 block/elevator.c     |  8 ++------
 5 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 5bfd70311359..e567d8480569 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3248,8 +3248,6 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
 		ioc = q_to_ioc(disk->queue);
 	}
 
-	blk_mq_quiesce_queue(disk->queue);
-
 	spin_lock_irq(&ioc->lock);
 	memcpy(qos, ioc->params.qos, sizeof(qos));
 	enable = ioc->enabled;
@@ -3346,13 +3344,10 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
 	else
 		wbt_enable_default(disk);
 
-	blk_mq_unquiesce_queue(disk->queue);
-
 	blkg_conf_exit_frozen(&ctx, memflags);
 	return nbytes;
 einval:
 	spin_unlock_irq(&ioc->lock);
-	blk_mq_unquiesce_queue(disk->queue);
 	ret = -EINVAL;
 err:
 	blkg_conf_exit_frozen(&ctx, memflags);
@@ -3439,7 +3434,6 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 	}
 
 	memflags = blk_mq_freeze_queue(q);
-	blk_mq_quiesce_queue(q);
 
 	spin_lock_irq(&ioc->lock);
 	memcpy(u, ioc->params.i_lcoefs, sizeof(u));
@@ -3489,7 +3483,6 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 	ioc_refresh_params(ioc, true);
 	spin_unlock_irq(&ioc->lock);
 
-	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 
 	blkg_conf_exit(&ctx);
@@ -3498,7 +3491,6 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 einval:
 	spin_unlock_irq(&ioc->lock);
 
-	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 
 	ret = -EINVAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 91b9fc1a7ddb..b84ff8e4e17e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4932,8 +4932,6 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 	if (q->nr_requests == nr)
 		return 0;
 
-	blk_mq_quiesce_queue(q);
-
 	ret = 0;
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (!hctx->tags)
@@ -4964,8 +4962,6 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 		}
 	}
 
-	blk_mq_unquiesce_queue(q);
-
 	return ret;
 }
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 1f63b184c6e9..dd11d7f1c600 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -593,6 +593,11 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page,
 	if (val < -1)
 		return -EINVAL;
 
+	/*
+	 * Ensure that the queue is idled, in case the latency update
+	 * ends up either enabling or disabling wbt completely. We can't
+	 * have IO inflight if that happens.
+	 */
 	memflags = blk_mq_freeze_queue(q);
 
 	rqos = wbt_rq_qos(q);
@@ -611,18 +616,10 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page,
 	if (wbt_get_min_lat(q) == val)
 		goto out;
 
-	/*
-	 * Ensure that the queue is idled, in case the latency update
-	 * ends up either enabling or disabling wbt completely. We can't
-	 * have IO inflight if that happens.
-	 */
-	blk_mq_quiesce_queue(q);
-
 	mutex_lock(&disk->rqos_state_mutex);
 	wbt_set_min_lat(q, val);
 	mutex_unlock(&disk->rqos_state_mutex);
 
-	blk_mq_unquiesce_queue(q);
 out:
 	blk_mq_unfreeze_queue(q, memflags);
 
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 397b6a410f9e..9abcdbfe91e1 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1332,7 +1332,6 @@ static int blk_throtl_init(struct gendisk *disk)
 	 * which is protected by 'q_usage_counter'.
 	 */
 	memflags = blk_mq_freeze_queue(disk->queue);
-	blk_mq_quiesce_queue(disk->queue);
 
 	q->td = td;
 	td->queue = q;
@@ -1354,7 +1353,6 @@ static int blk_throtl_init(struct gendisk *disk)
 		blk_stat_enable_accounting(q);
 
 out:
-	blk_mq_unquiesce_queue(disk->queue);
 	blk_mq_unfreeze_queue(disk->queue, memflags);
 
 	return ret;
diff --git a/block/elevator.c b/block/elevator.c
index ab22542e6cf0..574b35f8c01c 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -584,8 +584,6 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
 			return -EINVAL;
 	}
 
-	blk_mq_quiesce_queue(q);
-
 	if (q->elevator) {
 		ctx->old = q->elevator;
 		elevator_exit(q);
@@ -594,7 +592,7 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
 	if (new_e) {
 		ret = blk_mq_init_sched(q, new_e);
 		if (ret)
-			goto out_unfreeze;
+			goto out;
 		ctx->new = q->elevator;
 	} else {
 		blk_queue_flag_clear(QUEUE_FLAG_SQ_SCHED, q);
@@ -603,9 +601,7 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
 	}
 	blk_add_trace_msg(q, "elv switch: %s", ctx->name);
 
-out_unfreeze:
-	blk_mq_unquiesce_queue(q);
-
+out:
 	if (ret) {
 		pr_warn("elv: switch to \"%s\" failed, falling back to \"none\"\n",
 			new_e->elevator_name);

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/8] aoe: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
                   ` (2 preceding siblings ...)
  2025-07-02 20:38 ` [PATCH 3/8] block: Remove the quiesce/unquiesce calls on " Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-02 20:38 ` [PATCH 5/8] ataflop: " Bart Van Assche
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche,
	Justin Sanders

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/aoe/aoedev.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 3a240755045b..b765be7765e7 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -236,11 +236,8 @@ aoedev_downdev(struct aoedev *d)
 
 	/* fast fail all pending I/O */
 	if (d->blkq) {
-		/* UP is cleared, freeze+quiesce to insure all are errored */
+		/* UP is cleared, freeze to ensure all are errored */
 		unsigned int memflags = blk_mq_freeze_queue(d->blkq);
-
-		blk_mq_quiesce_queue(d->blkq);
-		blk_mq_unquiesce_queue(d->blkq);
 		blk_mq_unfreeze_queue(d->blkq, memflags);
 	}
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/8] ataflop: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
                   ` (3 preceding siblings ...)
  2025-07-02 20:38 ` [PATCH 4/8] aoe: " Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-02 20:38 ` [PATCH 6/8] sunvdc: " Bart Van Assche
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/ataflop.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 7fe14266c12c..0475f3bc6fbd 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -760,7 +760,6 @@ static int do_format(int drive, int type, struct atari_format_descr *desc)
 
 	q = unit[drive].disk[type]->queue;
 	memflags = blk_mq_freeze_queue(q);
-	blk_mq_quiesce_queue(q);
 
 	local_irq_save(flags);
 	stdma_lock(floppy_irq, NULL);
@@ -817,7 +816,6 @@ static int do_format(int drive, int type, struct atari_format_descr *desc)
 	finish_fdc();
 	ret = FormatError ? -EIO : 0;
 out:
-	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 	return ret;
 }

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/8] sunvdc: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
                   ` (4 preceding siblings ...)
  2025-07-02 20:38 ` [PATCH 5/8] ataflop: " Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-02 20:38 ` [PATCH 7/8] swim3: " Bart Van Assche
  2025-07-02 20:38 ` [PATCH 8/8] mtd_blkdevs: " Bart Van Assche
  7 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/sunvdc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c
index b5727dea15bd..8e3b32bcfd8b 100644
--- a/drivers/block/sunvdc.c
+++ b/drivers/block/sunvdc.c
@@ -1116,18 +1116,15 @@ static void vdc_queue_drain(struct vdc_port *port)
 	unsigned int memflags;
 
 	/*
-	 * Mark the queue as draining, then freeze/quiesce to ensure
+	 * Mark the queue as draining, then freeze the request queue to ensure
 	 * that all existing requests are seen in ->queue_rq() and killed
 	 */
 	port->drain = 1;
 	spin_unlock_irq(&port->vio.lock);
 
 	memflags = blk_mq_freeze_queue(q);
-	blk_mq_quiesce_queue(q);
-
 	spin_lock_irq(&port->vio.lock);
 	port->drain = 0;
-	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 }
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/8] swim3: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
                   ` (5 preceding siblings ...)
  2025-07-02 20:38 ` [PATCH 6/8] sunvdc: " Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-04  2:01   ` Damien Le Moal
  2025-07-02 20:38 ` [PATCH 8/8] mtd_blkdevs: " Bart Van Assche
  7 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/swim3.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 01f7aef3fcfb..8e209804ceaf 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -850,8 +850,6 @@ static void release_drive(struct floppy_state *fs)
 	spin_unlock_irqrestore(&swim3_lock, flags);
 
 	memflags = blk_mq_freeze_queue(q);
-	blk_mq_quiesce_queue(q);
-	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 }
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 8/8] mtd_blkdevs: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
                   ` (6 preceding siblings ...)
  2025-07-02 20:38 ` [PATCH 7/8] swim3: " Bart Van Assche
@ 2025-07-02 20:38 ` Bart Van Assche
  2025-07-28 10:14   ` Miquel Raynal
  7 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-07-02 20:38 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Ming Lei, Bart Van Assche,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra

Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
protecting request queue changes with blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
Hence this patch that removes quiesce/unquiesce calls on frozen queues.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/mtd/mtd_blkdevs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 847c11542f02..8131b17a980e 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -420,10 +420,8 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 	old->rq->queuedata = NULL;
 	spin_unlock_irqrestore(&old->queue_lock, flags);
 
-	/* freeze+quiesce queue to ensure all requests are flushed */
+	/* freeze the request queue to ensure all requests are flushed */
 	memflags = blk_mq_freeze_queue(old->rq);
-	blk_mq_quiesce_queue(old->rq);
-	blk_mq_unquiesce_queue(old->rq);
 	blk_mq_unfreeze_queue(old->rq, memflags);
 
 	/* If the device is currently open, tell trans driver to close it,

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-02 20:38 ` [PATCH 2/8] block: Do not run frozen queues Bart Van Assche
@ 2025-07-03  1:15   ` Yu Kuai
  2025-07-03  1:30     ` Bart Van Assche
  0 siblings, 1 reply; 17+ messages in thread
From: Yu Kuai @ 2025-07-03  1:15 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: linux-block, Christoph Hellwig, Ming Lei, yukuai (C)

Hi,

在 2025/07/03 4:38, Bart Van Assche 写道:
> If a request queue is frozen there are no requests pending and hence it
> is not necessary to run a request queue.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   block/blk-mq.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 7919607c1aeb..91b9fc1a7ddb 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2298,16 +2298,16 @@ static inline bool blk_mq_hw_queue_need_run(struct blk_mq_hw_ctx *hctx)
>   	bool need_run;
>   
>   	/*
> -	 * When queue is quiesced, we may be switching io scheduler, or
> -	 * updating nr_hw_queues, or other things, and we can't run queue
> -	 * any more, even blk_mq_hctx_has_pending() can't be called safely.
> -	 *
> -	 * And queue will be rerun in blk_mq_unquiesce_queue() if it is
> -	 * quiesced.
> +	 * The request queue is frozen when the e.g. the I/O scheduler is
> +	 * changed and also when nr_hw_queues is updated. Neither running the
> +	 * queue nor calling blk_mq_hctx_has_pending() is safe during these
> +	 * operations. Hence, check whether the queue is frozen before checking
> +	 * whether any requests are pending.
>   	 */
>   	__blk_mq_run_dispatch_ops(hctx->queue, false,
> -		need_run = !blk_queue_quiesced(hctx->queue) &&
> -		blk_mq_hctx_has_pending(hctx));
> +		need_run = !blk_queue_frozen(hctx->queue) &&
> +			   !blk_queue_quiesced(hctx->queue) &&
> +			   blk_mq_hctx_has_pending(hctx));

Just a question, blk_mq_quiesce_queue also calls synchronize_rcu() to
wait for inflight dispatch work to be done, is it safe in following
patches? I think it's not, dispatch work can be running while there is
no request pending, menas queue can be frozen with active dispatch work.

Thanks,
Kuai

>   	return need_run;
>   }
>   
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-03  1:15   ` Yu Kuai
@ 2025-07-03  1:30     ` Bart Van Assche
  2025-07-03  1:51       ` Ming Lei
  0 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-07-03  1:30 UTC (permalink / raw)
  To: Yu Kuai, Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei, yukuai (C)

On 7/2/25 6:15 PM, Yu Kuai wrote:
> Just a question, blk_mq_quiesce_queue also calls synchronize_rcu() to
> wait for inflight dispatch work to be done, is it safe in following
> patches? I think it's not, dispatch work can be running while there is
> no request pending, menas queue can be frozen with active dispatch work.

No work is dispatched if a queue is frozen because freezing a queue only
finishes after q_usage_counter drops to zero. queue_rq() and queue_rqs()
are only called if one or more requests are being executed.
q_usage_counter is increased when a request is allocated and decreased
when a request is freed. Hence, q_usage_counter cannot be zero while
queue_rq() or queue_rqs() is in progress. Hence, neither queue_rq() nor
queue_rqs() are called while q_usage_counter is zero.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-03  1:30     ` Bart Van Assche
@ 2025-07-03  1:51       ` Ming Lei
  2025-07-03  2:19         ` Yu Kuai
  2025-07-07 18:22         ` Bart Van Assche
  0 siblings, 2 replies; 17+ messages in thread
From: Ming Lei @ 2025-07-03  1:51 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Yu Kuai, Jens Axboe, linux-block, Christoph Hellwig, yukuai (C)

On Wed, Jul 02, 2025 at 06:30:34PM -0700, Bart Van Assche wrote:
> On 7/2/25 6:15 PM, Yu Kuai wrote:
> > Just a question, blk_mq_quiesce_queue also calls synchronize_rcu() to
> > wait for inflight dispatch work to be done, is it safe in following
> > patches? I think it's not, dispatch work can be running while there is
> > no request pending, menas queue can be frozen with active dispatch work.
> 
> No work is dispatched if a queue is frozen because freezing a queue only
> finishes after q_usage_counter drops to zero. queue_rq() and queue_rqs()
> are only called if one or more requests are being executed.
> q_usage_counter is increased when a request is allocated and decreased
> when a request is freed. Hence, q_usage_counter cannot be zero while
> queue_rq() or queue_rqs() is in progress. Hence, neither queue_rq() nor
> queue_rqs() are called while q_usage_counter is zero.

It isn't related with queue_rq() or queue_rqs() only.

The dispatch work is still in-progress when all requests are
completed(.q_usage_counter becomes zero), because request can complete any
time, even before queue_rq()/queue_rqs() returns.

The dispatch critical area is much _longer_ than queue_rq()/queue_rqs(),
block layer data structure may still be accessed after .q_usage_counter drops
to zero.

Please run 'git grep', and we did fix such kind of issues several times, such as:

662156641bc4 ("block: don't drain in-progress dispatch in blk_cleanup_queue()")


Thanks,
Ming


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-03  1:51       ` Ming Lei
@ 2025-07-03  2:19         ` Yu Kuai
  2025-07-07 18:22         ` Bart Van Assche
  1 sibling, 0 replies; 17+ messages in thread
From: Yu Kuai @ 2025-07-03  2:19 UTC (permalink / raw)
  To: Ming Lei, Bart Van Assche
  Cc: Yu Kuai, Jens Axboe, linux-block, Christoph Hellwig, yukuai (C)

Hi,

在 2025/07/03 9:51, Ming Lei 写道:
> On Wed, Jul 02, 2025 at 06:30:34PM -0700, Bart Van Assche wrote:
>> On 7/2/25 6:15 PM, Yu Kuai wrote:
>>> Just a question, blk_mq_quiesce_queue also calls synchronize_rcu() to
>>> wait for inflight dispatch work to be done, is it safe in following
>>> patches? I think it's not, dispatch work can be running while there is
>>> no request pending, menas queue can be frozen with active dispatch work.
>>
>> No work is dispatched if a queue is frozen because freezing a queue only
>> finishes after q_usage_counter drops to zero. queue_rq() and queue_rqs()
>> are only called if one or more requests are being executed.
>> q_usage_counter is increased when a request is allocated and decreased
>> when a request is freed. Hence, q_usage_counter cannot be zero while
>> queue_rq() or queue_rqs() is in progress. Hence, neither queue_rq() nor
>> queue_rqs() are called while q_usage_counter is zero.
> 
> It isn't related with queue_rq() or queue_rqs() only.
> 
> The dispatch work is still in-progress when all requests are
> completed(.q_usage_counter becomes zero), because request can complete any
> time, even before queue_rq()/queue_rqs() returns.
> 
> The dispatch critical area is much _longer_ than queue_rq()/queue_rqs(),
> block layer data structure may still be accessed after .q_usage_counter drops
> to zero.

Yes, the critical area is protected by rcu/srcu, you can see this in
__blk_mq_run_dispatch_ops.

There can be request pending while blk_mq_hw_queue_need_run() is
checking, and request can complete before __blk_mq_run_dispatch_ops().

Thanks,
Kuai

> 
> Please run 'git grep', and we did fix such kind of issues several times, such as:
> 
> 662156641bc4 ("block: don't drain in-progress dispatch in blk_cleanup_queue()")
> 
> 
> Thanks,
> Ming
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/8] swim3: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 ` [PATCH 7/8] swim3: " Bart Van Assche
@ 2025-07-04  2:01   ` Damien Le Moal
  0 siblings, 0 replies; 17+ messages in thread
From: Damien Le Moal @ 2025-07-04  2:01 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe; +Cc: linux-block, Christoph Hellwig, Ming Lei

On 7/3/25 05:38, Bart Van Assche wrote:
> Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
> protecting request queue changes with blk_mq_quiesce_queue() and
> blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
> Hence this patch that removes quiesce/unquiesce calls on frozen queues.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/block/swim3.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
> index 01f7aef3fcfb..8e209804ceaf 100644
> --- a/drivers/block/swim3.c
> +++ b/drivers/block/swim3.c
> @@ -850,8 +850,6 @@ static void release_drive(struct floppy_state *fs)
>  	spin_unlock_irqrestore(&swim3_lock, flags);
>  
>  	memflags = blk_mq_freeze_queue(q);
> -	blk_mq_quiesce_queue(q);
> -	blk_mq_unquiesce_queue(q);
>  	blk_mq_unfreeze_queue(q, memflags);
>  }

With your changes, the pattern:

	memflags = blk_mq_freeze_queue(q);
	blk_mq_unfreeze_queue(q, memflags);

is repeated several times. So maybe make that a helper ? Naming it is tricky
though. Maybe something like:

static inline void blk_mq_drain_queue(q)
{
	unsigned int memflags = blk_mq_freeze_queue(q);

	blk_mq_unfreeze_queue(q, memflags);
}


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-03  1:51       ` Ming Lei
  2025-07-03  2:19         ` Yu Kuai
@ 2025-07-07 18:22         ` Bart Van Assche
  2025-07-08  0:50           ` Ming Lei
  1 sibling, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2025-07-07 18:22 UTC (permalink / raw)
  To: Ming Lei; +Cc: Yu Kuai, Jens Axboe, linux-block, Christoph Hellwig, yukuai (C)

On 7/2/25 6:51 PM, Ming Lei wrote:
> The dispatch critical area is much _longer_ than queue_rq()/queue_rqs(),
> block layer data structure may still be accessed after .q_usage_counter drops
> to zero.

I think the above is only correct for block drivers that set the
BLK_MQ_F_BLOCKING flag. If BLK_MQ_F_BLOCKING is not set,
__blk_mq_run_dispatch_ops() uses rcu_read_lock() and rcu_read_unlock().
After q_usage_counter drops to zero, the percpu_refcount implementation
uses call_rcu_hurry() to invoke blk_queue_usage_counter_release().
Hence, when blk_queue_usage_counter_release() is called, a grace period 
has occurred since q_usage_counter dropped to zero. Hence, all request
processing and all dispatch activity has finished for non-blocking block
drivers. For BLK_MQ_F_BLOCKING drivers a synchronize_srcu() call could
be added in blk_mq_freeze_queue_wait() and also in
blk_mq_freeze_queue_wait_timeout(). However, I'm not sure this change
would be acceptable because it would slow down queue freezing even if it
is not necessary to wait for dispatch activity.

Thanks,

Bart.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/8] block: Do not run frozen queues
  2025-07-07 18:22         ` Bart Van Assche
@ 2025-07-08  0:50           ` Ming Lei
  0 siblings, 0 replies; 17+ messages in thread
From: Ming Lei @ 2025-07-08  0:50 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Yu Kuai, Jens Axboe, linux-block, Christoph Hellwig, yukuai (C)

On Mon, Jul 07, 2025 at 11:22:07AM -0700, Bart Van Assche wrote:
> On 7/2/25 6:51 PM, Ming Lei wrote:
> > The dispatch critical area is much _longer_ than queue_rq()/queue_rqs(),
> > block layer data structure may still be accessed after .q_usage_counter drops
> > to zero.
> 
> I think the above is only correct for block drivers that set the
> BLK_MQ_F_BLOCKING flag. If BLK_MQ_F_BLOCKING is not set,

No, please see my comment:

https://lore.kernel.org/linux-block/aGXiH1HqSlLk-QSI@fedora/

thanks,
Ming


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 8/8] mtd_blkdevs: Remove the quiesce/unquiesce calls on frozen queues
  2025-07-02 20:38 ` [PATCH 8/8] mtd_blkdevs: " Bart Van Assche
@ 2025-07-28 10:14   ` Miquel Raynal
  0 siblings, 0 replies; 17+ messages in thread
From: Miquel Raynal @ 2025-07-28 10:14 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, linux-block, Christoph Hellwig, Ming Lei,
	Richard Weinberger, Vignesh Raghavendra

Hello Bart,

On 02/07/2025 at 13:38:43 -07, Bart Van Assche <bvanassche@acm.org> wrote:

> Because blk_mq_hw_queue_need_run() now returns false if a queue is frozen,
> protecting request queue changes with blk_mq_quiesce_queue() and
> blk_mq_unquiesce_queue() while a queue is frozen is no longer necessary.
> Hence this patch that removes quiesce/unquiesce calls on frozen queues.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Sorry for the long delay, I initially marked this patch for applying it
on my side, but it looks like it depends on the first patches of the
series, so I'm not taking it.

Here is a formal A-b in case this patch must be carried through another
tree:

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Otherwise let me know, I can pick it up at -rc1.

Sorry again for the delay.
Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-07-28 10:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 20:38 [PATCH 0/8] Simplify the code modifying queue attributes Bart Van Assche
2025-07-02 20:38 ` [PATCH 1/8] block: Introduce QUEUE_FLAG_FROZEN Bart Van Assche
2025-07-02 20:38 ` [PATCH 2/8] block: Do not run frozen queues Bart Van Assche
2025-07-03  1:15   ` Yu Kuai
2025-07-03  1:30     ` Bart Van Assche
2025-07-03  1:51       ` Ming Lei
2025-07-03  2:19         ` Yu Kuai
2025-07-07 18:22         ` Bart Van Assche
2025-07-08  0:50           ` Ming Lei
2025-07-02 20:38 ` [PATCH 3/8] block: Remove the quiesce/unquiesce calls on " Bart Van Assche
2025-07-02 20:38 ` [PATCH 4/8] aoe: " Bart Van Assche
2025-07-02 20:38 ` [PATCH 5/8] ataflop: " Bart Van Assche
2025-07-02 20:38 ` [PATCH 6/8] sunvdc: " Bart Van Assche
2025-07-02 20:38 ` [PATCH 7/8] swim3: " Bart Van Assche
2025-07-04  2:01   ` Damien Le Moal
2025-07-02 20:38 ` [PATCH 8/8] mtd_blkdevs: " Bart Van Assche
2025-07-28 10:14   ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).