[PATCH] block: add rationale for not using blk_mq_plug() when applicable

[PATCH] block: add rationale for not using blk_mq_plug() when applicable

[Pankaj Raghav]

There are two places in the block layer at the moment where
blk_mq_plug() helper could be used instead of directly accessing the
plug from struct current. In both these cases, directly accessing the plug
should not have any consequences for zoned devices.

Make the intent explicit by adding comments instead of introducing unwanted
checks with blk_mq_plug() helper.[1]

[1] https://lore.kernel.org/linux-block/f6e54907-1035-2b2c-6387-ed178be05ccb@kernel.dk/

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Suggested-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-core.c | 5 +++++
 block/blk-mq.c   | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 203be672da52..c19d084b2a74 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -850,6 +850,11 @@ int bio_poll(struct bio *bio, struct io_comp_batch *iob, unsigned int flags)
 	    !test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
 		return 0;
 
+	/* As the requests that require a zone lock are not plugged in the
+	 * first place, directly accessing the plug instead of using
+	 * blk_mq_plug() should not have any consequences during flushing for
+	 * zoned devices.
+	 */
 	blk_flush_plug(current->plug, false);
 
 	if (bio_queue_enter(bio))
diff --git a/block/blk-mq.c b/block/blk-mq.c
index c11949d66163..c0b5b2c027f7 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1213,6 +1213,11 @@ void blk_execute_rq_nowait(struct request *rq, bool at_head)
 	WARN_ON(!blk_rq_is_passthrough(rq));
 
 	blk_account_io_start(rq);
+
+	/* As plugging can be enabled for passthrough requests on a zoned
+	 * device, directly accessing the plug instead of using blk_mq_plug()
+	 * should not have any consequences.
+	 */
 	if (current->plug)
 		blk_add_rq_to_plug(current->plug, rq);
 	else
-- 
2.25.1

Re: [PATCH] block: add rationale for not using blk_mq_plug() when applicable

[Jens Axboe]

On 9/29/22 8:41 AM, Pankaj Raghav wrote:
> There are two places in the block layer at the moment where
> blk_mq_plug() helper could be used instead of directly accessing the
> plug from struct current. In both these cases, directly accessing the plug
> should not have any consequences for zoned devices.
> 
> Make the intent explicit by adding comments instead of introducing unwanted
> checks with blk_mq_plug() helper.[1]
> 
> [1] https://lore.kernel.org/linux-block/f6e54907-1035-2b2c-6387-ed178be05ccb@kernel.dk/
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> Suggested-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/blk-core.c | 5 +++++
>  block/blk-mq.c   | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 203be672da52..c19d084b2a74 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -850,6 +850,11 @@ int bio_poll(struct bio *bio, struct io_comp_batch *iob, unsigned int flags)
>  	    !test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
>  		return 0;
>  
> +	/* As the requests that require a zone lock are not plugged in the
> +	 * first place, directly accessing the plug instead of using
> +	 * blk_mq_plug() should not have any consequences during flushing for
> +	 * zoned devices.
> +	 */
>  	blk_flush_plug(current->plug, false);

Multi-line comments should follow the style of:

/*
 * This is a multi
 * line comment
 */

I can fix that up while applying.

-- 
Jens Axboe

Re: [PATCH] block: add rationale for not using blk_mq_plug() when applicable

[Jens Axboe]

On Thu, 29 Sep 2022 16:41:41 +0200, Pankaj Raghav wrote:
> There are two places in the block layer at the moment where
> blk_mq_plug() helper could be used instead of directly accessing the
> plug from struct current. In both these cases, directly accessing the plug
> should not have any consequences for zoned devices.
> 
> Make the intent explicit by adding comments instead of introducing unwanted
> checks with blk_mq_plug() helper.[1]
> 
> [...]

Applied, thanks!

[1/1] block: add rationale for not using blk_mq_plug() when applicable
      commit: 110fdb4486d3a5e67d55bc6866d6426e6d49ccfc

Best regards,
-- 
Jens Axboe