mbox series

[0/9,v5] block: Fix IO priority mess

Message ID 20220623074450.30550-1-jack@suse.cz (mailing list archive)
Headers show
Series block: Fix IO priority mess | expand

Message

Jan Kara June 23, 2022, 7:48 a.m. UTC
Hello,

This is the fifth revision of my patches fixing IO priority handling in the
block layer. Damien has reviewed all the patches and tested them as well so
I think patches are ready to be merged.

Changes since v4:
* Added Reviewed-by and Tested-by tags
* Fixed prototype of stub function for !CONFIG_BLOCK

Changes since v3:
* Added Reviewed-by tags from Damien
* Fixed build failure without CONFIG_BLOCK
* Separated refactoring of get_current_ioprio() into a separate patch

Changes since v2:
* added some comments to better explain things
* changed handling of ioprio_get(2)
* a few small tweaks based on Damien's feedback

Original cover letter:
Recently, I've been looking into 10% regression reported by our performance
measurement infrastructure in reaim benchmark that was bisected down to
5a9d041ba2f6 ("block: move io_context creation into where it's needed"). This
didn't really make much sense and it took me a while to understand this but the
culprit is actually in even older commit e70344c05995 ("block: fix default IO
priority handling") and 5a9d041ba2f6 just made the breakage visible.
Essentially the problem was that after these commits some IO was queued with IO
priority class IOPRIO_CLASS_BE while other IO was queued with IOPRIO_CLASS_NONE
and as a result they could not be merged together resulting in performance
regression. I think what commit e70344c05995 ("block: fix default IO priority
handling") did is actually broken not only because of this performance
regression but because of other reasons as well (see changelog of patch 3/8 for
details). Besides this problem, there are multiple other inconsistencies in the
IO priority handling throughout the block stack we have identified when
discussing this with Damien Le Moal. So this patch set aims at fixing all these
various issues.

Note that there are a few choices I've made I'm not 100% sold on. In particular
the conversion of blk-ioprio from rqos is somewhat disputable since it now
incurs a cost similar to blk-throttle in the bio submission fast path (need to
load bio->bi_blkg->pd[ioprio_policy.plid]).  If people think the removed
boilerplate code is not worth the cost, I can certainly go via the "additional
rqos hook" path.

								Honza
Previous versions:
Link: http://lore.kernel.org/r/20220601132347.13543-1-jack@suse.cz # v1
Link: http://lore.kernel.org/r/20220615160437.5478-1-jack@suse.cz # v2
Link: http://lore.kernel.org/r/20220620160726.19798-1-jack@suse.cz # v3
Link: http://lore.kernel.org/r/20220621102201.26337-1-jack@suse.cz # v4