On Monday 12 June 2006 18:37, Christoph Lameter wrote:
> On Mon, 12 Jun 2006, Ingo Molnar wrote:
> > below is an updated patch that includes fixups for i386 - but the real
> > fix should be to properly reduce the per-arch local.h footprint to the
> > bare minimum possible, and to do this fix on the asm-generic headers.
> Nak. This is simply evidence of local.t breakage on i386. One cannot
> calculate the address of a per cpu area and then increment without
> disabling preemption. The process may have been moved to another processor
> between those two operations and will then increment the event counter of
> the former processor (which in turn may at the same time increment the
> same counter). The inc is not atomic in the sense that it syncs multiple
> processors. So we will have the race back.
True - i forgot that race.
> The increment must occur directly through the atomic-vs-interrupt dec/inc
> on the local per cpu area *without* any use of *_smp_processor_id().
> As far as I can see x86_64 does the right thing and it increments on the
> local per cpu area. The definition of __get_cpu_var is:
> #define __get_cpu_var(var) (*RELOC_HIDE(&per_cpu__##var, __my_cpu_offset()))
It is also affected by your race. The inc would only be atomic if the counter
was in the PDA, but standard per cpu data isn't. So it has to follow
a pointer and then it could already have switched.
[I think Ingo stated this earlier, but I didn't get -- sorry]
Fix would be to disable preemption. I don't think it needs cli/sti
on non preemptible kernels.
> i386 uses the asm-generic/percpu.h but provides its own
> implementation of local.t. That simply cannot work. i386
> must provide a definition of __get_per_var that increments directly
> on the variable in the local per cpu area.
> The definition for __get_cpu_var in asm-generic/percpu.h is:
> #define __get_cpu_var(var) per_cpu(var, smp_processor_id())
> This breaks the cpu_* operations for local.t on i386.
> Also we have various forms of __raw_get_cpu_var() around. Is there any
> reason for their existence. The presence of these shows the assumption
> that one can determine the current processor id and then index into an
> array of per cpu areas. That is not possible with preemption enabled.
> In the absence of a race free __get_cpu_var() i386 would need to fall
> back to atomic ops by using asm-generic/local.t.
Or just disable preemption?
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Video 4 Linux]
[Linux for the blind]