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.
