Re: [PATCH] jbd2: make sure jh have b_transaction set in refile/unlink_buffer

[Jan Kara <jack@suse.cz>]

On Wed 17-06-20 11:10:31, Lukas Czerner wrote:
> Callers of __jbd2_journal_unfile_buffer() and
> __jbd2_journal_refile_buffer() assume that the b_transaction is set. In
> fact if it's not, we can end up with journal_head refcounting errors
> leading to crash much later that might be very hard to track down. Add
> asserts to make sure that is the case.
> 
> We also make sure that b_next_transaction is NULL in
> __jbd2_journal_unfile_buffer() since the callers expect that as well and
> we should not get into that stage in this state anyway, leading to
> problems later on if we do.
> 
> Tested with fstests.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks! The patch looks good to me. You can add:

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

								Honza

> ---
>  fs/jbd2/transaction.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index e91aad3637a2..e65e0aca2826 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2026,6 +2026,9 @@ static void __jbd2_journal_temp_unlink_buffer(struct journal_head *jh)
>   */
>  static void __jbd2_journal_unfile_buffer(struct journal_head *jh)
>  {
> +	J_ASSERT_JH(jh, jh->b_transaction != NULL);
> +	J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
> +
>  	__jbd2_journal_temp_unlink_buffer(jh);
>  	jh->b_transaction = NULL;
>  }
> @@ -2572,6 +2575,13 @@ bool __jbd2_journal_refile_buffer(struct journal_head *jh)
>  
>  	was_dirty = test_clear_buffer_jbddirty(bh);
>  	__jbd2_journal_temp_unlink_buffer(jh);
> +
> +	/*
> +	 * b_transaction must be set, otherwise the new b_transaction won't
> +	 * be holding jh reference
> +	 */
> +	J_ASSERT_JH(jh, jh->b_transaction != NULL);
> +
>  	/*
>  	 * We set b_transaction here because b_next_transaction will inherit
>  	 * our jh reference and thus __jbd2_journal_file_buffer() must not
> -- 
> 2.21.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR