Re: [PATCH] percpu data: only iterate over possible CPUs

[Nathan Lynch <ntl@pobox.com>]

Eric Dumazet wrote:
>
> [PATCH] HOTPLUG_CPU : avoid hitting too many cachelines in recalc_bh_state()
> 
> Instead of using for_each_cpu(i), we can use for_each_online_cpu(i) : The 
> difference matters if HOTPUG_CPU=y
> 
> When a CPU goes offline (ie removed from online map), it might have a non 
> null bh_accounting.nr, so this patch adds a transfert of this counter to an 
> online CPU counter.
> 
> We already have a hotcpu_notifier, (function buffer_cpu_notify()), where we 
> can do this bh_accounting.nr transfert.
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

> --- a/fs/buffer.c	2006-02-10 15:08:21.000000000 +0100
> +++ b/fs/buffer.c	2006-02-10 15:47:55.000000000 +0100
> @@ -3138,7 +3138,7 @@
>  	if (__get_cpu_var(bh_accounting).ratelimit++ < 4096)
>  		return;
>  	__get_cpu_var(bh_accounting).ratelimit = 0;
> -	for_each_cpu(i)
> +	for_each_online_cpu(i)
>  		tot += per_cpu(bh_accounting, i).nr;
>  	buffer_heads_over_limit = (tot > max_buffer_heads);
>  }
> @@ -3187,6 +3187,9 @@
>  		brelse(b->bhs[i]);
>  		b->bhs[i] = NULL;
>  	}
> +	get_cpu_var(bh_accounting).nr += per_cpu(bh_accounting, cpu).nr ;
> +	per_cpu(bh_accounting, cpu).nr = 0;
> +	put_cpu_var(bh_accounting);
>  }

But now there is a window between the time the cpu is marked offline
and the time its bh_accounting.nr is moved to another cpu.  So
recalc_bh_state could fail to set buffer_heads_over_limit when it
should.

Maybe it's not worth worrying about?  I don't really know this code,
just thought I would point it out.