linux-ext4 - Re: [PATCH 5/7] jbd2: Don't call _

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: <20191030114939.GH28525@quack2.suse.cz>
Date:   Wed, 30 Oct 2019 12:49:39 +0100
From:   Jan Kara <jack@...e.cz>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Julia Cartwright <julia@...com>
Subject: Re: [PATCH 5/7] jbd2: Don't call __bforget() unnecessarily

On Mon 28-10-19 12:01:36, Theodore Y. Ts'o wrote:
> On Mon, Oct 28, 2019 at 11:28:08AM -0400, Theodore Y. Ts'o wrote:
> > drop:
> > 	jbd_unlock_bh_state(bh);
> > 	__brelse(bh);
> > 	if (drop_reserve) {
> > 		/* no need to reserve log space for this block -bzzz */
> > 		handle->h_buffer_credits++;
> > 	}
> > 	return err;
> > 
> > not_jbd:
> > 	jbd_unlock_bh_state(bh);
> > 	__bforget(bh);
> > 	goto drop;
> > ----
> > 
> > And we still have a case we jump to not_jbd, at which point hilarity
> > will ensue.
> > 
> > This is cleaned up in the following patch in this sequence, but this
> > leaves us in a not-great state if we are ever bisecting.
> 
> Proposed fixup: I'll apply the following on top of this commit, and
> then fix the merge conflicts in 6/7 so that the end result looks the
> same as before.
> 
> Jan, any objections?  I figure this way it'll save you from resending
> the patch series, since the rest of it looks fine to me.

Yeah, looks good to me. Thanks for the fixup!

								Honza

> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index f2af4afc690a..c7c9a42451c7 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1541,8 +1541,11 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh)
>  
>  	jbd_lock_bh_state(bh);
>  
> -	if (!buffer_jbd(bh))
> -		goto not_jbd;
> +	if (!buffer_jbd(bh)) {
> +		jbd_unlock_bh_state(bh);
> +		__bforget(bh);
> +		return 0;
> +	}
>  	jh = bh2jh(bh);
>  
>  	/* Critical error: attempting to delete a bitmap buffer, maybe?
> @@ -1671,11 +1674,6 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh)
>  		handle->h_buffer_credits++;
>  	}
>  	return err;
> -
> -not_jbd:
> -	jbd_unlock_bh_state(bh);
> -	__bforget(bh);
> -	goto drop;
>  }
>  
>  /**
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR