[PATCH] Revert "block: don't reorder requests in blk_add_rq_to_plug"

[Mohamed Abuelfotoh, Hazem]

On 11/06/2025 13:56, Jens Axboe wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 6/11/25 6:14 AM, Hazem Mohamed Abuelfotoh wrote:
>> This reverts commit e70c301faece15b618e54b613b1fd6ece3dd05b4.
>>
>> Commit <e70c301faece> ("block: don't reorder requests in
>> blk_add_rq_to_plug") reversed how requests are stored in the blk_plug
>> list, this had significant impact on bio merging with requests exist on
>> the plug list. This impact has been reported in [1] and could easily be
>> reproducible using 4k randwrite fio benchmark on an NVME based SSD without
>> having any filesystem on the disk.
> 
> Rather than revert this commit, why not just attempt a tail merge?
> Something ala this, totally untested.
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 3af1d284add5..708ded67d52a 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -998,6 +998,10 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
>          if (!plug || rq_list_empty(&plug->mq_list))
>                  return false;
> 
> +       rq = plug->mq_list.tail;
> +       if (rq->q == q)
> +               return blk_attempt_bio_merge(q, rq, bio, nr_segs, false) == BIO_MERGE_OK;
> +
>          rq_list_for_each(&plug->mq_list, rq) {
>                  if (rq->q == q) {
>                          if (blk_attempt_bio_merge(q, rq, bio, nr_segs, false) ==
> 
> --
> Jens Axboe

I thought about that solution before submitting the revert and I believe 
it will help with the case we are discussing here  but what about the 
case where we have raid disks for which we need to iterate the plug 
list? In this case we will iterate the plug list from head to tail while 
the most recent requests (where merging will likely happen) will be 
closer to that tail so there will be additional overhead to iterate 
through the whole plug list and I believe the revert will also be better 
for this specific case unless I am missing something here :)