Messages in this thread | | | From | Andi Kleen <> | Subject | Re: broken local_t on i386 | Date | Mon, 12 Jun 2006 18:48:05 +0200 |
| |
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?
-Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |