Re: [PATCH 2/2] fs: jbd2: use clear_and_wake_up_bit() in journal_end_buffer_io_sync()

[Jan Kara]

On Wed 20-05-26 17:05:46, Agatha Isabelle Moreira wrote:
> Use `clear_and_wake_up_bit()` in `journal_end_buffer_io_sync()`, since
> the helper was introduced in 'commit 8236b0ae31c83 ("bdi: wake up
> concurrent wb_shutdown() callers.")' as a generic way of doing the same
> sequence of operations:
> 	clear_bit_unlock();
> 	smp_mb__after_atomic();
> 	wake_up_bit();
> 
> The helper was first implemented to avoid bugs caused by forgetting to
> call `wake_up_bit()` after `clear_bit_unlock()`.
> 
> Since `journal_end_buffer_io_sync()` was first introduced by 'commit
> 470decc613ab2 ("jbd2: initial copy of files from jbd")' and last
> modified in this operation by 'commit 4e857c58efeb9 ("arch: Mass
> conversion of smp_mb__*()")', years before `clear_and_wake_up_bit()`, it
> still uses the open-coded sequence.
> 
> Replace the open-coded sequence with the helper to avoid duplicate code
> and reduce code paths to maintain.
> 
> Suggested-by: shuo chen <1289151713@xxxxxx>
> Link: https://lore.kernel.org/kernelnewbies/agzoqV835-co4kAN@guidai/T/#t
> Signed-off-by: Agatha Isabelle Moreira <code@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/jbd2/commit.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 8cf61e7185c4..b647fde76e49 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -39,9 +39,7 @@ static void journal_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
>  	else
>  		clear_buffer_uptodate(bh);
>  	if (orig_bh) {
> -		clear_bit_unlock(BH_Shadow, &orig_bh->b_state);
> -		smp_mb__after_atomic();
> -		wake_up_bit(&orig_bh->b_state, BH_Shadow);
> +		clear_and_wake_up_bit(BH_Shadow, &orig_bh->b_state);
>  	}
>  	unlock_buffer(bh);
>  }
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR