Re: [PATCH] fs/buffer: publish max_buffer_heads with release semantics

[Jan Kara]

On Mon 21-09-26 21:15:46, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri@xxxxxx>
> 
> buffer_init() creates bh_cachep and then computes max_buffer_heads, both
> with plain stores. recalc_bh_state() reads max_buffer_heads with a plain
> load from alloc_buffer_head() and free_buffer_head(), on any CPU.
> 
> Store it with smp_store_release() and read it with smp_load_acquire().
> 
> Found with MBCheck, a static herd7-based memory consistency checker.
> 
> Signed-off-by: Jaidev Shastri <jaidevshastri@xxxxxx>

And how exactly could these race? If alloc_buffer_head() could run so that
it won't see full results from buffer_init(), we'd be in big trouble. Hint:
buffer_init() is marked as __init and thus is run very early during kernel
bring up when we still run in single-cpu mode.

								Honza

> ---
>  fs/buffer.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index ed966fa73..bf5a674c1 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2864,7 +2864,8 @@ static void recalc_bh_state(void)
>  	__this_cpu_write(bh_accounting.ratelimit, 0);
>  	for_each_online_cpu(i)
>  		tot += per_cpu(bh_accounting, i).nr;
> -	buffer_heads_over_limit = (tot > max_buffer_heads);
> +	/* Pairs with the smp_store_release() in buffer_init(). */
> +	buffer_heads_over_limit = (tot > smp_load_acquire(&max_buffer_heads));
>  }
>  
>  struct buffer_head *alloc_buffer_head(gfp_t gfp_flags)
> @@ -2998,7 +2999,9 @@ void __init buffer_init(void)
>  	 * Limit the bh occupancy to 10% of ZONE_NORMAL
>  	 */
>  	nrpages = (nr_free_buffer_pages() * 10) / 100;
> -	max_buffer_heads = nrpages * (PAGE_SIZE / sizeof(struct buffer_head));
> +	/* Pairs with the smp_load_acquire() in recalc_bh_state(). */
> +	smp_store_release(&max_buffer_heads,
> +			  nrpages * (PAGE_SIZE / sizeof(struct buffer_head)));
>  	ret = cpuhp_setup_state_nocalls(CPUHP_FS_BUFF_DEAD, "fs/buffer:dead",
>  					NULL, buffer_exit_cpu_dead);
>  	WARN_ON(ret < 0);
> 
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> change-id: 20260921-mb-fs-buffer-0a91680176e5
> 
> Best regards,
> --  
> Jaidev Shastri <jaidevshastri@xxxxxx>
> 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR