On Tue 14-03-23 18:18:20, Ming Lei wrote:
> On Mon, Mar 13, 2023 at 10:30:02AM +0100, Jan Kara wrote:
> > Commit 26fed4ac4eab ("block: flush plug based on hardware and software
> > queue order") changed flushing of plug list to submit requests one
> > device at a time. However while doing that it also started using
> > list_add_tail() instead of list_add() used previously thus effectively
> > submitting requests in reverse order. Also when forming a rq_list with
> > remaining requests (in case two or more devices are used), we
> > effectively reverse the ordering of the plug list for each device we
> > process. Submitting requests in reverse order has negative impact on
> > performance for rotational disks (when BFQ is not in use). We observe
> > 10-25% regression in random 4k write throughput, as well as ~20%
> > regression in MariaDB OLTP benchmark on rotational storage on btrfs
> > filesystem.
> >
> > Fix the problem by preserving ordering of the plug list when inserting
> > requests into the queuelist as well as by appending to requeue_list
> > instead of prepending to it.
>
> Also in case of !plug->multiple_queues && !plug->has_elevator, requests
> are still sent to device in reverse order, do we need to cover that case?
That's an interesting question. I suppose reversing the order in this case
could be suprising e.g. for shingled storage. I don't think it matters that
much for normal rotational storage as there you presumably run with at
least some IO scheduler (we were using mq-deadline in our testing).
Avoiding the reversal will require small changes to plug handling (so that
we append the plug list) but it shouldn't be too bad. Still I'd do it in a
separate patch.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
