* [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c
@ 2020-02-28 15:05 Guoqing Jiang
2020-02-28 15:05 ` [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits Guoqing Jiang
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
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: remove obsolete comments 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 | 11 +++--------
block/blk-flush.c | 7 ++-----
block/blk-mq.c | 3 +--
block/blk.h | 4 ++--
4 files changed, 8 insertions(+), 17 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-29 23:06 ` Chaitanya Kulkarni
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
` (4 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, 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>
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
2020-02-28 15:05 ` [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-28 22:37 ` Nikolay Borisov
` (2 more replies)
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
` (3 subsequent siblings)
5 siblings, 3 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
Use the two functions to simplify code.
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
2020-02-28 15:05 ` [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits Guoqing Jiang
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-28 22:38 ` Nikolay Borisov
` (2 more replies)
2020-02-28 15:05 ` [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone Guoqing Jiang
` (2 subsequent siblings)
5 siblings, 3 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
Previously, blk_cleanup_queue has called blk_set_queue_dying to set the
flag, no need to do it again.
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
` (2 preceding siblings ...)
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-29 2:34 ` Bart Van Assche
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
5 siblings, 1 reply; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
Both cmd and sense had been moved to scsi_request, so remove
the related comments to avoid confusion.
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
block/blk-core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 883ffda216e4..9094fd7d1b01 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1583,7 +1583,6 @@ 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)
{
@@ -1610,8 +1609,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
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
` (3 preceding siblings ...)
2020-02-28 15:05 ` [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-28 22:40 ` Nikolay Borisov
` (2 more replies)
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
5 siblings, 3 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
Remove 'q' from arguments since it is not used anymore after
commit 7e992f847a08e ("block: remove non mq parts from the
flush code").
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 3f977c517960..963ae56d5aae 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 a12b1763508d..9684ea9a9e1f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2405,8 +2405,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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
` (4 preceding siblings ...)
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
@ 2020-02-28 15:05 ` Guoqing Jiang
2020-02-28 22:41 ` Nikolay Borisov
` (2 more replies)
5 siblings, 3 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-02-28 15:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Guoqing Jiang
Remove the comment about return value, since it is not valid after
commit 404b8f5a03d84 ("block: cleanup kick/queued handling").
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 963ae56d5aae..46fd6e83dd79 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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
@ 2020-02-28 22:37 ` Nikolay Borisov
2020-02-29 2:26 ` Bart Van Assche
2020-02-29 23:11 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2020-02-28 22:37 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 28.02.20 г. 17:05 ч., Guoqing Jiang wrote:
> Use the two functions to simplify code.
>
> 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;
> }
>
>
The code is functionally identical so:
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
@ 2020-02-28 22:38 ` Nikolay Borisov
2020-02-29 2:27 ` Bart Van Assche
2020-02-29 23:12 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2020-02-28 22:38 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 28.02.20 г. 17:05 ч., Guoqing Jiang wrote:
> Previously, blk_cleanup_queue has called blk_set_queue_dying to set the
> flag, no need to do it again.
>
> 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
>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
@ 2020-02-28 22:40 ` Nikolay Borisov
2020-02-29 2:35 ` Bart Van Assche
2020-02-29 23:19 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2020-02-28 22:40 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 28.02.20 г. 17:05 ч., Guoqing Jiang wrote:
> Remove 'q' from arguments since it is not used anymore after
> commit 7e992f847a08e ("block: remove non mq parts from the
> flush code").
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
@ 2020-02-28 22:41 ` Nikolay Borisov
2020-02-29 2:36 ` Bart Van Assche
2020-02-29 23:58 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2020-02-28 22:41 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 28.02.20 г. 17:05 ч., Guoqing Jiang wrote:
> Remove the comment about return value, since it is not valid after
> commit 404b8f5a03d84 ("block: cleanup kick/queued handling").
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
Obviously correct, given the function is void.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
2020-02-28 22:37 ` Nikolay Borisov
@ 2020-02-29 2:26 ` Bart Van Assche
2020-02-29 23:11 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2020-02-29 2:26 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 2020-02-28 07:05, Guoqing Jiang wrote:
> Use the two functions to simplify code.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
2020-02-28 22:38 ` Nikolay Borisov
@ 2020-02-29 2:27 ` Bart Van Assche
2020-02-29 23:12 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2020-02-29 2:27 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 2020-02-28 07:05, Guoqing Jiang wrote:
> Previously, blk_cleanup_queue has called blk_set_queue_dying to set the
> flag, no need to do it again.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone
2020-02-28 15:05 ` [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone Guoqing Jiang
@ 2020-02-29 2:34 ` Bart Van Assche
2020-03-01 16:41 ` Guoqing Jiang
0 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2020-02-29 2:34 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 2020-02-28 07:05, Guoqing Jiang wrote:
> Both cmd and sense had been moved to scsi_request, so remove
> the related comments to avoid confusion.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> ---
> block/blk-core.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 883ffda216e4..9094fd7d1b01 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1583,7 +1583,6 @@ 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)
> {
Although the removed comment is outdated, the new comment is not useful.
How about inlining __blk_rq_prep_clone() into its only caller and
removing the comment above that function entirely? It's not clear to me
why the code inside __blk_rq_prep_clone() ever was put into a separate
function.
> *
> * 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
Adding a comment that explains that some but not all struct request
members are copied and also why would be welcome.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
2020-02-28 22:40 ` Nikolay Borisov
@ 2020-02-29 2:35 ` Bart Van Assche
2020-02-29 23:19 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2020-02-29 2:35 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 2020-02-28 07:05, Guoqing Jiang wrote:
> Remove 'q' from arguments since it is not used anymore after
> commit 7e992f847a08e ("block: remove non mq parts from the
> flush code").
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
2020-02-28 22:41 ` Nikolay Borisov
@ 2020-02-29 2:36 ` Bart Van Assche
2020-02-29 23:58 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2020-02-29 2:36 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
On 2020-02-28 07:05, Guoqing Jiang wrote:
> Remove the comment about return value, since it is not valid after
> commit 404b8f5a03d84 ("block: cleanup kick/queued handling").
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits
2020-02-28 15:05 ` [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits Guoqing Jiang
@ 2020-02-29 23:06 ` Chaitanya Kulkarni
0 siblings, 0 replies; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-29 23:06 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On 02/28/2020 07:06 AM, Guoqing Jiang wrote:
> 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>
> 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
> *
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
2020-02-28 22:37 ` Nikolay Borisov
2020-02-29 2:26 ` Bart Van Assche
@ 2020-02-29 23:11 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-29 23:11 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
Neat, looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On 02/28/2020 07:06 AM, Guoqing Jiang wrote:
> Use the two functions to simplify code.
>
> Signed-off-by: Guoqing Jiang<guoqing.jiang@cloud.ionos.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
2020-02-28 22:38 ` Nikolay Borisov
2020-02-29 2:27 ` Bart Van Assche
@ 2020-02-29 23:12 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-29 23:12 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On 02/28/2020 07:06 AM, Guoqing Jiang wrote:
> Previously, blk_cleanup_queue has called blk_set_queue_dying to set the
> flag, no need to do it again.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
2020-02-28 22:40 ` Nikolay Borisov
2020-02-29 2:35 ` Bart Van Assche
@ 2020-02-29 23:19 ` Chaitanya Kulkarni
2020-03-01 16:47 ` Guoqing Jiang
2 siblings, 1 reply; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-29 23:19 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
Looks good, except I did not count but please verify the
patch subject length, otherwise looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On 02/28/2020 07:07 AM, Guoqing Jiang wrote:
> Remove 'q' from arguments since it is not used anymore after
> commit 7e992f847a08e ("block: remove non mq parts from the
> flush code").
>
> Signed-off-by: Guoqing Jiang<guoqing.jiang@cloud.ionos.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
2020-02-28 22:41 ` Nikolay Borisov
2020-02-29 2:36 ` Bart Van Assche
@ 2020-02-29 23:58 ` Chaitanya Kulkarni
2 siblings, 0 replies; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-29 23:58 UTC (permalink / raw)
To: Guoqing Jiang, axboe; +Cc: linux-block
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On 02/28/2020 07:06 AM, Guoqing Jiang wrote:
> Remove the comment about return value, since it is not valid after
> commit 404b8f5a03d84 ("block: cleanup kick/queued handling").
>
> Signed-off-by: Guoqing Jiang<guoqing.jiang@cloud.ionos.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone
2020-02-29 2:34 ` Bart Van Assche
@ 2020-03-01 16:41 ` Guoqing Jiang
0 siblings, 0 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-03-01 16:41 UTC (permalink / raw)
To: Bart Van Assche, axboe; +Cc: linux-block
Hi Bart,
Thanks for your review.
On 2/29/20 3:34 AM, Bart Van Assche wrote:
> On 2020-02-28 07:05, Guoqing Jiang wrote:
>> Both cmd and sense had been moved to scsi_request, so remove
>> the related comments to avoid confusion.
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
>> ---
>> block/blk-core.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 883ffda216e4..9094fd7d1b01 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1583,7 +1583,6 @@ 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)
>> {
>
> Although the removed comment is outdated, the new comment is not useful.
>
> How about inlining __blk_rq_prep_clone() into its only caller and
> removing the comment above that function entirely?It's not clear to me
> why the code inside __blk_rq_prep_clone() ever was put into a separate
> function.
Not sure about it, the original code was introduced in commit b0fd271d5fba0
("block: add request clone interface (v2)"), maybe author preferred to use
a function to copy attributes from src request to dst request.
I will make the change based on your suggestion if no one objects it.
>
>> *
>> * 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
>
> Adding a comment that explains that some but not all struct request
> members are copied and also why would be welcome.
I think we need care about the actual data parts of request, I guess all the
actual data parts had been moved into scsi_request, but my understanding could
probably be wrong.
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue
2020-02-29 23:19 ` Chaitanya Kulkarni
@ 2020-03-01 16:47 ` Guoqing Jiang
0 siblings, 0 replies; 23+ messages in thread
From: Guoqing Jiang @ 2020-03-01 16:47 UTC (permalink / raw)
To: Chaitanya Kulkarni, axboe; +Cc: linux-block
On 3/1/20 12:19 AM, Chaitanya Kulkarni wrote:
> Looks good, except I did not count but please verify the
> patch subject length, otherwise looks good.
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>
Thanks for your review, I assume it is fine since checkpatch didn't complain.
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-03-01 16:48 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 15:05 [PATCH 0/6] Some cleanups for blk-core.c and blk-flush.c Guoqing Jiang
2020-02-28 15:05 ` [PATCH 1/6] block: fix comment for blk_cloned_rq_check_limits Guoqing Jiang
2020-02-29 23:06 ` Chaitanya Kulkarni
2020-02-28 15:05 ` [PATCH 2/6] block: use bio_{wouldblock,io}_error in direct_make_request Guoqing Jiang
2020-02-28 22:37 ` Nikolay Borisov
2020-02-29 2:26 ` Bart Van Assche
2020-02-29 23:11 ` Chaitanya Kulkarni
2020-02-28 15:05 ` [PATCH 3/6] block: remove redundant setting of QUEUE_FLAG_DYING Guoqing Jiang
2020-02-28 22:38 ` Nikolay Borisov
2020-02-29 2:27 ` Bart Van Assche
2020-02-29 23:12 ` Chaitanya Kulkarni
2020-02-28 15:05 ` [PATCH 4/6] block: remove obsolete comments for _blk/blk_rq_prep_clone Guoqing Jiang
2020-02-29 2:34 ` Bart Van Assche
2020-03-01 16:41 ` Guoqing Jiang
2020-02-28 15:05 ` [PATCH 5/6] block: remove unneeded argument from blk_alloc_flush_queue Guoqing Jiang
2020-02-28 22:40 ` Nikolay Borisov
2020-02-29 2:35 ` Bart Van Assche
2020-02-29 23:19 ` Chaitanya Kulkarni
2020-03-01 16:47 ` Guoqing Jiang
2020-02-28 15:05 ` [PATCH 6/6] block: cleanup comment for blk_flush_complete_seq Guoqing Jiang
2020-02-28 22:41 ` Nikolay Borisov
2020-02-29 2:36 ` Bart Van Assche
2020-02-29 23:58 ` Chaitanya Kulkarni
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).