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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231127161149.2s7yizu72gw332s2@quack3>
Date: Mon, 27 Nov 2023 17:11:49 +0100
From: Jan Kara <jack@...e.cz>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca,
	jack@...e.cz, yi.zhang@...wei.com, chengzhihao1@...wei.com,
	yukuai3@...wei.com
Subject: Re: [PATCH 2/2] jbd2: increase the journal IO's priority

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.

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

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR