Hi Mathieu,
On 8/22/07, Mathieu Desnoyers <[EMAIL PROTECTED]> wrote:
> Cons:
> - Does not help code readability, i.e.:
>
> if (have_arch_cmpxchg())
> preempt_disable();
> else
> local_irq_save(flags);
Heh, that's an understatement, as now slub is starting to look a bit
like slab... ;-) We need to hide that if-else maze into helper
functions for sure.
On 8/22/07, Mathieu Desnoyers <[EMAIL PROTECTED]> wrote:
> --- slab.orig/mm/slub.c 2007-08-22 10:28:22.000000000 -0400
> +++ slab/mm/slub.c 2007-08-22 10:51:53.000000000 -0400
> @@ -1450,7 +1450,8 @@ static void *__slab_alloc(struct kmem_ca
> void **freelist = NULL;
> unsigned long flags;
>
> - local_irq_save(flags);
> + if (have_arch_cmpxchg())
> + local_irq_save(flags);
I haven't been following on the cmpxchg_local() discussion too much so
the obvious question is: why do we do local_irq_save() for the _has_
cmpxchg() case here...
On 8/22/07, Mathieu Desnoyers <[EMAIL PROTECTED]> wrote:
> @@ -1553,8 +1556,12 @@ static void __always_inline *slab_alloc(
> {
> void **object;
> struct kmem_cache_cpu *c;
> + unsigned long flags;
>
> - preempt_disable();
> + if (have_arch_cmpxchg())
> + preempt_disable();
> + else
> + local_irq_save(flags);
...and preempt_disable() here?
On 8/22/07, Mathieu Desnoyers <[EMAIL PROTECTED]> wrote:
> + if (have_arch_cmpxchg()) {
> + if (unlikely(cmpxchg_local(&c->freelist, object,
> + object[c->offset]) != object))
> + goto redo;
> + preempt_enable();
> + } else {
> + c->freelist = object[c->offset];
> + local_irq_restore(flags);
> + }
If you move the preempt_enable/local_irq_restore pair outside of the
if-else block, you can make a static inline function slob_get_object()
that does:
static inline bool slob_get_object(struct kmem_cache *c, void **object)
{
if (have_arch_cmpxchg()) {
if (unlikely(cmpxchg_local(&c->freelist, object,
object[c->offset]) != object))
return false;
} else {
c->freelist = object[c->offset];
}
return true;
}
And let the compiler optimize out the branch for the non-cmpxchg case.
Same for the reverse case too (i.e. slob_put_object).
Pekka
-
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/
