Re: [PATCH v2] block: remove plug based merging

[Ming Lei <ming.lei@redhat.com>]

Hello Jens,

On Wed, Oct 13, 2021 at 02:00:35PM -0600, Jens Axboe wrote:
> It's expensive to browse the whole plug list for merge opportunities at
> the IOPS rates that modern storage can do. For sequential IO, the one-hit
> cached merge should suffice on fast drives, and for rotational storage the
> IO scheduler will do a more exhaustive lookup based merge anyway.
> 
> Just remove the plug based O(N) traversal merging.

This way looks too aggressive, is there any performance data with the plug
merge which is supposed to be fast since it is totally lockless?

Maybe we can simply check if the incoming bio can be merged to the last or 1st
request in the plug list?

> 
> With that, there's really no difference between the two plug cases that
> we have. Unify them.

IMO, there are differences between the two:

1) blk_attempt_plug_merge() is lockless, and no request allocation is
involved for merging incoming sequential bio.

2) after plug merge is killed, the merge is delayed until request is
allocated & added to plug list: a) for elevator, the merge requires lock
when running blk_mq_sched_insert_requests() b) for none, there isn't
merge any more, and small IO is always sent to driver because blk_mq_insert_requests
doesn't merge requests.


Thanks, 
Ming