> 2021年06月09日 14:12，Ming Lei <ming.lei@xxxxxxxxxx> 写道：
>
> On Wed, Jun 09, 2021 at 11:05:59AM +0800, Wang Shanker wrote:
>>
>>
>>> 2021年06月09日 08:45，Ming Lei <ming.lei@xxxxxxxxxx> 写道：
>>>
>>> So far multi-range discard treats each bio as one segment(range) of single
>>> discard request. This way becomes not efficient if lots of small sized
>>> discard bios are submitted, and one example is raid456.
>>>
>>> Support bio merge for multi-range discard for improving lots of small
>>> sized discard bios.
>>>
>>> Turns out it is easy to support it:
>>>
>>> 1) always try to merge bio first
>>>
>>> 2) run into multi-range discard only if bio merge can't be done
>>>
>>> 3) add rq_for_each_discard_range() for retrieving each range(segment)
>>> of discard request
>>>
>>> Reported-by: Wang Shanker <shankerwangmiao@xxxxxxxxx>
>>> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
>>> ---
>>> block/blk-merge.c | 12 ++++-----
>>> drivers/block/virtio_blk.c | 9 ++++---
>>> drivers/nvme/host/core.c | 8 +++---
>>> include/linux/blkdev.h | 51 ++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 66 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>>> index bcdff1879c34..65210e9a8efa 100644
>>> --- a/block/blk-merge.c
>>> +++ b/block/blk-merge.c
>>> @@ -724,10 +724,10 @@ static inline bool blk_discard_mergable(struct request *req)
>>> static enum elv_merge blk_try_req_merge(struct request *req,
>>> struct request *next)
>>> {
>>> - if (blk_discard_mergable(req))
>>> - return ELEVATOR_DISCARD_MERGE;
>>> - else if (blk_rq_pos(req) + blk_rq_sectors(req) == blk_rq_pos(next))
>>> + if (blk_rq_pos(req) + blk_rq_sectors(req) == blk_rq_pos(next))
>>> return ELEVATOR_BACK_MERGE;
>>> + else if (blk_discard_mergable(req))
>>
>> Shall we adjust how req->nr_phys_segments is calculated in
>> bio_attempt_discard_merge() so that multiple contiguous bio's can
>> be seen as one segment?
>
> I think it isn't necessary, because we try to merge discard IOs first
> just like plain IO. So when bio_attempt_discard_merge() is reached, it
> means that IOs can't be merged, so req->nr_phys_segments should be
> increased by 1.
You are right. And by applying the series, I can confirm contiguous bio's
are getting merged into ~20M requests. It's much better than before. I
guess it might be the scheduler that prevents further merging. So it
seems that the root solution for raid456 is required since bio merging cannot
solve all the problem.
Thanks again for your work.
Cheers,
Miao Wang
