On Tue, 20 Nov 2007, Mathieu Desnoyers wrote:
> > {
> > -#ifdef CONFIG_SMP
> > on_each_cpu(flush_cpu_slab, s, 1, 1);
> > -#else
> > - unsigned long flags;
> > -
> > - local_irq_save(flags);
> > - flush_cpu_slab(s);
> > - local_irq_restore(flags);
> > -#endif
> > }
> >
>
> Normally :
>
> You can't use on_each_cpu if interrupts are already disabled. Therefore,
> the implementation using "local_irq_disable/enable" in smp.h for the UP
> case is semantically correct and there is no need to use a save/restore.
> So just using on_each_cpu should be enough here.
The on_each_cpu is only used in the smp case.
> I also wonder about flush_cpu_slab execution on _other_ cpus. I am not
> convinced interrupts are disabled when it executes.. have I missing
> something ?
flush_cpu_slab only flushes the local cpu slab. keventd threads are pinned
to each processor so it should be safe.
> > -#ifdef CONFIG_FAST_CMPXCHG_LOCAL
> > - c = get_cpu_slab(s, get_cpu());
> > +#ifdef CONFIG_FAST_CPU_OPS
>
> I wonder.. are there some architectures that would provide fast
> cmpxchg_local but not fast cpu ops ?
If you have a fast cmpxchg_local then you can use that to implement fast
cpu ops.
> > + } while (CPU_CMPXCHG(c->freelist, object,
> > + object[__CPU_READ(c->offset)]) != object);
>
> Hrm. What happens here if we call __slab_alloc, get a valid object, then
> have a CPU_CMPXCHG that fails, restart the loop.. is this case taken
> care of or do we end up having an unreferenced object ? Maybe there is
> some logic in freelist that takes care of it ?
If we fail then we have done nothing....
> Also, we have to be aware that we can now change CPU between the
>
> __CPU_READ and the CPU_CMPXCHG. (also : should it be a __CPU_CMPXCHG ?)
>
> But since "object" contains information specific to the local CPU, the
> cmpxchg should fail if we are migrated and everything should be ok.
Right.
> Hrm, actually, the
>
> c = s->cpu_slab;
>
> should probably be after the object = __CPU_READ(c->freelist); ?
No. c is invariant in respect to cpus.
> The cpu_read acts as a safeguard checking that we do not change CPU
> between the read and the cmpxchg. If we are preempted between the "c"
> read and the cpu_read, we could do a !cpu_node_match(c, node) check that
> would apply to the wrong cpu.
C is not pointing to a specific cpu. It can only be used in CPU_xx ops to
address the currrent cpu.
> > @@ -1800,19 +1792,19 @@ static void __always_inline slab_free(st
> > * then any change of cpu_slab will cause the cmpxchg to fail
> > * since the freelist pointers are unique per slab.
> > */
> > - if (unlikely(page != c->page || c->node < 0)) {
> > - __slab_free(s, page, x, addr, c->offset);
> > + if (unlikely(page != __CPU_READ(c->page) ||
> > + __CPU_READ(c->node) < 0)) {
> > + __slab_free(s, page, x, addr, __CPU_READ(c->offset));
>
> And same question as above : what happens if we fail after executing the
> __slab_free.. is it valid to do it twice ?
__slab_free is always successful and will never cause a repeat of the
loop.
-
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/
