linux-ext4 - Re: [PATCH 2/2] jbd2: increase the journal IO's priority

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 28 Nov 2023 11:32:21 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca,
 yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com
Subject: Re: [PATCH 2/2] jbd2: increase the journal IO's priority

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@...wei.com>
>>
>> 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.