[PATCH V2 0/6] Some cleanups for blk-core.c and blk-flush.c

[PATCH V2 0/6] Some cleanups for blk-core.c and blk-flush.c

[Guoqing Jiang]

V2:

* collected Reviewed-by tags for some patches.
* remove __blk_rq_prep_clone and move the impletation to the only caller.

V1:

Hi,

This patchset updates code and comment in blk-core.c and blk-flush.c.
The first patch had been sent before, now update it with the Reviewed-by
tag from Bart.

Thanks,
Guoqing

Guoqing Jiang (6):
  block: fix comment for blk_cloned_rq_check_limits
  block: use bio_{wouldblock,io}_error in direct_make_request
  block: remove redundant setting of QUEUE_FLAG_DYING
  block: cleanup for _blk/blk_rq_prep_clone
  block: remove unneeded argument from blk_alloc_flush_queue
  block: cleanup comment for blk_flush_complete_seq

 block/blk-core.c  | 38 +++++++++++++-------------------------
 block/blk-flush.c |  7 ++-----
 block/blk-mq.c    |  3 +--
 block/blk.h       |  4 ++--
 4 files changed, 18 insertions(+), 34 deletions(-)

-- 
2.17.1

[PATCH V2 1/6] block: fix comment for blk_cloned_rq_check_limits

[Guoqing Jiang]

Since the later description mentioned "checked against the new queue
limits", so make the change to avoid confusion.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 089e890ab208..fd43266029be 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1203,7 +1203,7 @@ EXPORT_SYMBOL(submit_bio);
 
 /**
  * blk_cloned_rq_check_limits - Helper function to check a cloned request
- *                              for new the queue limits
+ *                              for the new queue limits
  * @q:  the queue
  * @rq: the request being checked
  *
-- 
2.17.1

[PATCH V2 2/6] block: use bio_{wouldblock,io}_error in direct_make_request

[Guoqing Jiang]

Use the two functions to simplify code.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index fd43266029be..6d36c2ad40ba 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1121,10 +1121,9 @@ blk_qc_t direct_make_request(struct bio *bio)
 
 	if (unlikely(blk_queue_enter(q, nowait ? BLK_MQ_REQ_NOWAIT : 0))) {
 		if (nowait && !blk_queue_dying(q))
-			bio->bi_status = BLK_STS_AGAIN;
+			bio_wouldblock_error(bio);
 		else
-			bio->bi_status = BLK_STS_IOERR;
-		bio_endio(bio);
+			bio_io_error(bio);
 		return BLK_QC_T_NONE;
 	}
 
-- 
2.17.1

[PATCH V2 3/6] block: remove redundant setting of QUEUE_FLAG_DYING

[Guoqing Jiang]

Previously, blk_cleanup_queue has called blk_set_queue_dying to set the
flag, no need to do it again.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 6d36c2ad40ba..883ffda216e4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -346,7 +346,6 @@ void blk_cleanup_queue(struct request_queue *q)
 
 	blk_queue_flag_set(QUEUE_FLAG_NOMERGES, q);
 	blk_queue_flag_set(QUEUE_FLAG_NOXMERGES, q);
-	blk_queue_flag_set(QUEUE_FLAG_DYING, q);
 
 	/*
 	 * Drain all requests queued before DYING marking. Set DEAD flag to
-- 
2.17.1

[PATCH V2 4/6] block: cleanup for _blk/blk_rq_prep_clone

[Guoqing Jiang]

Both cmd and sense had been moved to scsi_request, so remove
the related comments to avoid confusion.

And as Bart suggested, move _blk_rq_prep_clone into the only
caller (blk_rq_prep_clone).

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-core.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 883ffda216e4..4433f5276250 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1581,23 +1581,6 @@ void blk_rq_unprep_clone(struct request *rq)
 }
 EXPORT_SYMBOL_GPL(blk_rq_unprep_clone);
 
-/*
- * Copy attributes of the original request to the clone request.
- * The actual data parts (e.g. ->cmd, ->sense) are not copied.
- */
-static void __blk_rq_prep_clone(struct request *dst, struct request *src)
-{
-	dst->__sector = blk_rq_pos(src);
-	dst->__data_len = blk_rq_bytes(src);
-	if (src->rq_flags & RQF_SPECIAL_PAYLOAD) {
-		dst->rq_flags |= RQF_SPECIAL_PAYLOAD;
-		dst->special_vec = src->special_vec;
-	}
-	dst->nr_phys_segments = src->nr_phys_segments;
-	dst->ioprio = src->ioprio;
-	dst->extra_len = src->extra_len;
-}
-
 /**
  * blk_rq_prep_clone - Helper function to setup clone request
  * @rq: the request to be setup
@@ -1610,8 +1593,6 @@ static void __blk_rq_prep_clone(struct request *dst, struct request *src)
  *
  * Description:
  *     Clones bios in @rq_src to @rq, and copies attributes of @rq_src to @rq.
- *     The actual data parts of @rq_src (e.g. ->cmd, ->sense)
- *     are not copied, and copying such parts is the caller's responsibility.
  *     Also, pages which the original bios are pointing to are not copied
  *     and the cloned bios just point same pages.
  *     So cloned bios must be completed before original bios, which means
@@ -1642,7 +1623,16 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 			rq->bio = rq->biotail = bio;
 	}
 
-	__blk_rq_prep_clone(rq, rq_src);
+	/* Copy attributes of the original request to the clone request. */
+	rq->__sector = blk_rq_pos(rq_src);
+	rq->__data_len = blk_rq_bytes(rq_src);
+	if (rq_src->rq_flags & RQF_SPECIAL_PAYLOAD) {
+		rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
+		rq->special_vec = rq_src->special_vec;
+	}
+	rq->nr_phys_segments = rq_src->nr_phys_segments;
+	rq->ioprio = rq_src->ioprio;
+	rq->extra_len = rq_src->extra_len;
 
 	return 0;
 
-- 
2.17.1

[PATCH V2 5/6] block: remove unneeded argument from blk_alloc_flush_queue

[Guoqing Jiang]

Remove 'q' from arguments since it is not used anymore after
commit 7e992f847a08e ("block: remove non mq parts from the
flush code").

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-flush.c | 4 ++--
 block/blk-mq.c    | 3 +--
 block/blk.h       | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 5cc775bdb06a..7f7f98305115 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -485,8 +485,8 @@ int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask,
 }
 EXPORT_SYMBOL(blkdev_issue_flush);
 
-struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
-		int node, int cmd_size, gfp_t flags)
+struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
+					      gfp_t flags)
 {
 	struct blk_flush_queue *fq;
 	int rq_sz = sizeof(struct request);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d92088dec6c3..d855cf860682 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2409,8 +2409,7 @@ blk_mq_alloc_hctx(struct request_queue *q, struct blk_mq_tag_set *set,
 	init_waitqueue_func_entry(&hctx->dispatch_wait, blk_mq_dispatch_wake);
 	INIT_LIST_HEAD(&hctx->dispatch_wait.entry);
 
-	hctx->fq = blk_alloc_flush_queue(q, hctx->numa_node, set->cmd_size,
-			gfp);
+	hctx->fq = blk_alloc_flush_queue(hctx->numa_node, set->cmd_size, gfp);
 	if (!hctx->fq)
 		goto free_bitmap;
 
diff --git a/block/blk.h b/block/blk.h
index 0b8884353f6b..670337b7cfa0 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -55,8 +55,8 @@ is_flush_rq(struct request *req, struct blk_mq_hw_ctx *hctx)
 	return hctx->fq->flush_rq == req;
 }
 
-struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
-		int node, int cmd_size, gfp_t flags);
+struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
+					      gfp_t flags);
 void blk_free_flush_queue(struct blk_flush_queue *q);
 
 void blk_freeze_queue(struct request_queue *q);
-- 
2.17.1

[PATCH V2 6/6] block: cleanup comment for blk_flush_complete_seq

[Guoqing Jiang]

Remove the comment about return value, since it is not valid after
commit 404b8f5a03d84 ("block: cleanup kick/queued handling").

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-flush.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 7f7f98305115..843d25683691 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -160,9 +160,6 @@ static void blk_account_io_flush(struct request *rq)
  *
  * CONTEXT:
  * spin_lock_irq(fq->mq_flush_lock)
- *
- * RETURNS:
- * %true if requests were added to the dispatch queue, %false otherwise.
  */
 static void blk_flush_complete_seq(struct request *rq,
 				   struct blk_flush_queue *fq,
-- 
2.17.1

Re: [PATCH V2 0/6] Some cleanups for blk-core.c and blk-flush.c

[Jens Axboe]

On 3/9/20 3:41 PM, Guoqing Jiang wrote:
> V2:
> 
> * collected Reviewed-by tags for some patches.
> * remove __blk_rq_prep_clone and move the impletation to the only caller.
> 
> V1:
> 
> Hi,
> 
> This patchset updates code and comment in blk-core.c and blk-flush.c.
> The first patch had been sent before, now update it with the Reviewed-by
> tag from Bart.

Applied for 5.7, thanks.

-- 
Jens Axboe

Re: [PATCH V2 2/6] block: use bio_{wouldblock,io}_error in direct_make_request

[Christoph Hellwig]

On Mon, Mar 09, 2020 at 10:41:34PM +0100, Guoqing Jiang wrote:
> Use the two functions to simplify code.

I'd much rather kill these helpers rather than adding new users.