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 :)