Re: [PATCH] block: streamline merge possibility checks

[Christoph Hellwig <hch@infradead.org>]

On Fri, Dec 20, 2019 at 02:50:05PM +0800, Bob Liu wrote:
> 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;

This looks sensible, but we might have to be a bit more careful.
rq_mergeable checks for RQF_NOMERGE_FLAGS and various ops, while
bio_mergeable is missing those.  So I think you need to go through
carefully if we need to keep any extra checks, but otherwise using
blk_rq_merge_ok looks sensible.