On 12/19/19 3:41 AM, Dmitry Fomichev wrote:
> Checks for data direction in attempt_merge() and blk_rq_merge_ok()
Speak about these two functions, do you think attempt_merge() can be built on blk_rq_merge_ok()?
Things like..
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 48e6725..2a00c4c 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -724,28 +724,7 @@ static enum elv_merge blk_try_req_merge(struct request *req,
static struct request *attempt_merge(struct request_queue *q,
struct request *req, struct request *next)
{
- if (!rq_mergeable(req) || !rq_mergeable(next))
- return NULL;
-
- if (req_op(req) != req_op(next))
- return NULL;
-
- if (rq_data_dir(req) != rq_data_dir(next)
- || req->rq_disk != next->rq_disk)
- return NULL;
-
- if (req_op(req) == REQ_OP_WRITE_SAME &&
- !blk_write_same_mergeable(req->bio, next->bio))
- return NULL;
-
- /*
- * Don't allow merge of different write hints, or for a hint with
- * non-hint IO.
- */
- if (req->write_hint != next->write_hint)
- return NULL;
-
- if (req->ioprio != next->ioprio)
+ if (!blk_rq_merge_ok(req, next->bio))
return NULL;
> are redundant and will always succeed when the both I/O request
> operations are equal. Therefore, remove them.
>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@xxxxxxx>
> ---
> block/blk-merge.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index d783bdc4559b..796451aed7d6 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -745,8 +745,7 @@ static struct request *attempt_merge(struct request_queue *q,
> if (req_op(req) != req_op(next))
> return NULL;
>
> - if (rq_data_dir(req) != rq_data_dir(next)
> - || req->rq_disk != next->rq_disk)
> + if (req->rq_disk != next->rq_disk)
> return NULL;
>
> if (req_op(req) == REQ_OP_WRITE_SAME &&
> @@ -868,10 +867,6 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
> if (req_op(rq) != bio_op(bio))
> return false;
>
> - /* different data direction or already started, don't merge */
> - if (bio_data_dir(bio) != rq_data_dir(rq))
> - return false;
> -
> /* must be same device */
> if (rq->rq_disk != bio->bi_disk)
> return false;
>
