linux-ext4 - Re: [PATCH 5/7] fs/jbd2: use sleeping version of __find_get

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <6tqny4muvkyejvokjkx6gh53ihc2duxleonzwqlkl6hn7y6w7n@r3x6dns3q6pn>
Date: Wed, 16 Apr 2025 11:38:47 +0200
From: Jan Kara <jack@...e.cz>
To: Davidlohr Bueso <dave@...olabs.net>
Cc: jack@...e.cz, tytso@....edu, adilger.kernel@...ger.ca, 
	brauner@...nel.org, mcgrof@...nel.org, willy@...radead.org, hare@...e.de, 
	djwong@...nel.org, linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-mm@...ck.org
Subject: Re: [PATCH 5/7] fs/jbd2: use sleeping version of __find_get_block()

On Tue 15-04-25 16:16:33, Davidlohr Bueso wrote:
> Convert to the new nonatomic flavor to benefit from potential
> performance benefits and adapt in the future vs migration such
> that semantics are kept.
> 
> - jbd2_journal_revoke(): can sleep (has might_sleep() in the beginning)
> 
> - jbd2_journal_cancel_revoke(): only used from do_get_write_access() and
>     do_get_create_access() which do sleep. So can sleep.
> 
> - jbd2_clear_buffer_revoked_flags() - only called from journal commit code
>     which sleeps. So can sleep.
> 
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Davidlohr Bueso <dave@...olabs.net>

Looks good. Feel free to add:

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

								Honza


> ---
>  fs/jbd2/revoke.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index 0cf0fddbee81..1467f6790747 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -345,7 +345,8 @@ int jbd2_journal_revoke(handle_t *handle, unsigned long long blocknr,
>  	bh = bh_in;
>  
>  	if (!bh) {
> -		bh = __find_get_block(bdev, blocknr, journal->j_blocksize);
> +		bh = __find_get_block_nonatomic(bdev, blocknr,
> +						journal->j_blocksize);
>  		if (bh)
>  			BUFFER_TRACE(bh, "found on hash");
>  	}
> @@ -355,7 +356,8 @@ int jbd2_journal_revoke(handle_t *handle, unsigned long long blocknr,
>  
>  		/* If there is a different buffer_head lying around in
>  		 * memory anywhere... */
> -		bh2 = __find_get_block(bdev, blocknr, journal->j_blocksize);
> +		bh2 = __find_get_block_nonatomic(bdev, blocknr,
> +						 journal->j_blocksize);
>  		if (bh2) {
>  			/* ... and it has RevokeValid status... */
>  			if (bh2 != bh && buffer_revokevalid(bh2))
> @@ -464,7 +466,8 @@ void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  	 * state machine will get very upset later on. */
>  	if (need_cancel) {
>  		struct buffer_head *bh2;
> -		bh2 = __find_get_block(bh->b_bdev, bh->b_blocknr, bh->b_size);
> +		bh2 = __find_get_block_nonatomic(bh->b_bdev, bh->b_blocknr,
> +						 bh->b_size);
>  		if (bh2) {
>  			if (bh2 != bh)
>  				clear_buffer_revoked(bh2);
> @@ -492,9 +495,9 @@ void jbd2_clear_buffer_revoked_flags(journal_t *journal)
>  			struct jbd2_revoke_record_s *record;
>  			struct buffer_head *bh;
>  			record = (struct jbd2_revoke_record_s *)list_entry;
> -			bh = __find_get_block(journal->j_fs_dev,
> -					      record->blocknr,
> -					      journal->j_blocksize);
> +			bh = __find_get_block_nonatomic(journal->j_fs_dev,
> +							record->blocknr,
> +							journal->j_blocksize);
>  			if (bh) {
>  				clear_buffer_revoked(bh);
>  				__brelse(bh);
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR