Hello! On 2023/11/28 0:11, Jan Kara wrote: > Hello! > > On Sat 25-11-23 20:17:39, Zhang Yi wrote: >> From: Zhang Yi <yi.zhang@xxxxxxxxxx> >> >> Current jbd2 only add REQ_SYNC for descriptor block, metadata log >> buffer, commit buffer and superblock buffer, the submitted IO could be >> throttled by writeback throttle in block layer, that could lead to >> priority inversion in some cases. The log IO looks like a kind of high >> priority metadata IO, so it should not be throttled by WBT like QOS >> policies in block layer, let's add REQ_SYNC | REQ_IDLE to exempt from >> writeback throttle, and also add REQ_META together indicates it's a >> metadata IO. > > So I agree about REQ_META but with REQ_IDLE I'm not so sure. __REQ_IDLE is > documented as "anticipate more IO after this one" so that is a good fit for > normal transaction writes but not so much for journal superblock writes. > OTOH as far as I'm checking XFS uses REQ_IDLE as well for its log IO and > the only places where REQ_IDLE is really checked is in blk-wbt... So I > guess this makes sense. > Thanks a lot for your review. Yeah, We've checked the block cgroup related qos policies and blk-wbt, block cgroup based policies does not throttle IO with REQ_META, and blk-wbt exempt IO with both REQ_IDLE and REQ_SYNC. But submit_bh() can make sure the journal IO is always bind to the root cgroup, so only blk-wbt is left for us to deal with, so I add REQ_IDLE like xfs does. >> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h >> index 52772c826c86..f7e8274b46ae 100644 >> --- a/include/linux/jbd2.h >> +++ b/include/linux/jbd2.h >> @@ -1374,6 +1374,9 @@ JBD2_FEATURE_INCOMPAT_FUNCS(csum2, CSUM_V2) >> JBD2_FEATURE_INCOMPAT_FUNCS(csum3, CSUM_V3) >> JBD2_FEATURE_INCOMPAT_FUNCS(fast_commit, FAST_COMMIT) >> >> +/* Journal high priority write IO operation flags */ >> +#define JBD2_REQ_HIPRIO (REQ_META | REQ_SYNC | REQ_IDLE) > > Since all journal IO is submitted with these flags I'd maybe name this > JBD2_JOURNAL_REQ_FLAGS? Otherwise the patch looks good to me so feel free > to add: > Sure, JBD2_JOURNAL_REQ_FLAGS looks fine, I will use it in v2. Thanks, Yi.
