lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <Pine.LNX.4.64.0710281502480.4207@sbz-30.cs.Helsinki.FI> Date: Sun, 28 Oct 2007 15:05:50 +0200 (EET) From: Pekka J Enberg <penberg@...helsinki.fi> To: Christoph Lameter <clameter@....com> cc: Matthew Wilcox <matthew@....cx>, akpm@...ux-foundation.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org Subject: Re: [patch 08/10] SLUB: Optional fast path using cmpxchg_local On Sat, 27 Oct 2007, Christoph Lameter wrote: > The alternate path is realized using #ifdef's. Several attempts to do the > same with macros and in line functions resulted in a mess (in particular due > to the strange way that local_interrupt_save() handles its argument and due > to the need to define macros/functions that sometimes disable interrupts > and sometimes do something else. The macro based approaches made it also > difficult to preserve the optimizations for the non cmpxchg paths). I think at least slub_alloc() and slub_free() can be made simpler. See the included patch below. > @@ -1496,7 +1496,12 @@ static void *__slab_alloc(struct kmem_ca > { > void **object; > struct page *new; > +#ifdef CONFIG_FAST_CMPXCHG_LOCAL > + unsigned long flags; > > + local_irq_save(flags); > + preempt_enable_no_resched(); > +#endif > if (!c->page) > goto new_slab; > > @@ -1518,6 +1523,10 @@ load_freelist: > unlock_out: > slab_unlock(c->page); > out: > +#ifdef CONFIG_FAST_CMPXCHG_LOCAL > + preempt_disable(); > + local_irq_restore(flags); > +#endif > return object; Can you please write a comment of the locking rules when cmpxchg_local() is used? Looks as if we could push that local_irq_save() to slub_lock() and local_irq_restore() to slub_unlock() and deal with the unused flags variable for the non-CONFIG_FAST_CMPXCHG_LOCAL case with a macro, no? Pekka Signed-off-by: Christoph Lameter <clameter@....com> Signed-off-by: Pekka Enberg <penberg@...helsinki.fi> --- arch/x86/Kconfig.i386 | 4 + arch/x86/Kconfig.x86_64 | 4 + mm/slub.c | 140 +++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 122 insertions(+), 26 deletions(-) Index: linux-2.6/mm/slub.c =================================================================== --- linux-2.6.orig/mm/slub.c +++ linux-2.6/mm/slub.c @@ -1496,7 +1496,12 @@ static void *__slab_alloc(struct kmem_ca { void **object; struct page *new; +#ifdef CONFIG_FAST_CMPXCHG_LOCAL + unsigned long flags; + local_irq_save(flags); + preempt_enable_no_resched(); +#endif if (!c->page) goto new_slab; @@ -1518,6 +1523,10 @@ load_freelist: unlock_out: slab_unlock(c->page); out: +#ifdef CONFIG_FAST_CMPXCHG_LOCAL + preempt_disable(); + local_irq_restore(flags); +#endif return object; another_slab: @@ -1578,6 +1587,45 @@ debug: goto unlock_out; } +#ifdef CONFIG_FAST_CMPXHG_LOCAL +static __always_inline void *do_slab_alloc(struct kmem_cache *s, + struct kmem_cache_cpu *c, gfp_t gfpflags, int node, void *addr) +{ + unsigned long flags; + void **object; + + do { + object = c->freelist; + if (unlikely(is_end(object) || !node_match(c, node))) { + object = __slab_alloc(s, gfpflags, node, addr, c); + break; + } + } while (cmpxchg_local(&c->freelist, object, object[c->offset]) + != object); + put_cpu(); + + return object; +} +#else + +static __always_inline void *do_slab_alloc(struct kmem_cache *s, + struct kmem_cache_cpu *c, gfp_t gfpflags, int node, void *addr) +{ + unsigned long flags; + void **object; + + local_irq_save(flags); + if (unlikely((is_end(c->freelist)) || !node_match(c, node))) { + object = __slab_alloc(s, gfpflags, node, addr, c); + } else { + object = c->freelist; + c->freelist = object[c->offset]; + } + local_irq_restore(flags); + return object; +} +#endif + /* * Inlined fastpath so that allocation functions (kmalloc, kmem_cache_alloc) * have the fastpath folded into their functions. So no function call @@ -1591,24 +1639,13 @@ debug: static void __always_inline *slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, void *addr) { - void **object; - unsigned long flags; struct kmem_cache_cpu *c; + void **object; - local_irq_save(flags); c = get_cpu_slab(s, smp_processor_id()); - if (unlikely((is_end(c->freelist)) || !node_match(c, node))) { - - object = __slab_alloc(s, gfpflags, node, addr, c); - if (unlikely(!object)) { - local_irq_restore(flags); - goto out; - } - } else { - object = c->freelist; - c->freelist = object[c->offset]; - } - local_irq_restore(flags); + object = do_slab_alloc(s, c, gfpflags, node, addr); + if (unlikely(!object)) + goto out; if (unlikely((gfpflags & __GFP_ZERO))) memset(object, 0, c->objsize); @@ -1644,6 +1681,11 @@ static void __slab_free(struct kmem_cach void *prior; void **object = (void *)x; +#ifdef CONFIG_FAST_CMPXCHG_LOCAL + unsigned long flags; + + local_irq_save(flags); +#endif slab_lock(page); if (unlikely(SlabDebug(page))) @@ -1669,6 +1711,9 @@ checks_ok: out_unlock: slab_unlock(page); +#ifdef CONFIG_FAST_CMPXCHG_LOCAL + local_irq_restore(flags); +#endif return; slab_empty: @@ -1679,6 +1724,9 @@ slab_empty: remove_partial(s, page); slab_unlock(page); +#ifdef CONFIG_FAST_CMPXCHG_LOCAL + local_irq_restore(flags); +#endif discard_slab(s, page); return; @@ -1688,6 +1736,56 @@ debug: goto checks_ok; } +#ifdef CONFIG_FAST_CMPXCHG_LOCAL +static __always_inline void do_slab_free(struct kmem_cache *s, + struct page *page, void **object, void *addr) +{ + struct kmem_cache_cpu *c; + void **freelist; + + c = get_cpu_slab(s, get_cpu()); + do { + freelist = c->freelist; + barrier(); + /* + * If the compiler would reorder the retrieval of c->page to + * come before c->freelist then an interrupt could + * change the cpu slab before we retrieve c->freelist. We + * could be matching on a page no longer active and put the + * object onto the freelist of the wrong slab. + * + * On the other hand: If we already have the freelist pointer + * 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, object, addr, c->offset); + break; + } + object[c->offset] = freelist; + } while (cmpxchg_local(&c->freelist, freelist, object) != freelist); + put_cpu(); +} +#else + +static __always_inline void do_slab_free(struct kmem_cache *s, + struct page *page, void **object, void *addr) +{ + struct kmem_cache_cpu *c; + unsigned long flags; + + c = get_cpu_slab(s, smp_processor_id()); + local_irq_save(flags); + if (likely(page == c->page && c->node >= 0)) { + object[c->offset] = c->freelist; + c->freelist = object; + } else + __slab_free(s, page, object, addr, c->offset); + + local_irq_restore(flags); +} +#endif + /* * Fastpath with forced inlining to produce a kfree and kmem_cache_free that * can perform fastpath freeing without additional function calls. @@ -1703,19 +1801,9 @@ static void __always_inline slab_free(st struct page *page, void *x, void *addr) { void **object = (void *)x; - unsigned long flags; - struct kmem_cache_cpu *c; - local_irq_save(flags); debug_check_no_locks_freed(object, s->objsize); - c = get_cpu_slab(s, smp_processor_id()); - if (likely(page == c->page && c->node >= 0)) { - object[c->offset] = c->freelist; - c->freelist = object; - } else - __slab_free(s, page, x, addr, c->offset); - - local_irq_restore(flags); + do_slab_free(s, page, object, addr); } void kmem_cache_free(struct kmem_cache *s, void *x) Index: linux-2.6/arch/x86/Kconfig.i386 =================================================================== --- linux-2.6.orig/arch/x86/Kconfig.i386 +++ linux-2.6/arch/x86/Kconfig.i386 @@ -51,6 +51,10 @@ config X86 bool default y +config FAST_CMPXCHG_LOCAL + bool + default y + config MMU bool default y Index: linux-2.6/arch/x86/Kconfig.x86_64 =================================================================== --- linux-2.6.orig/arch/x86/Kconfig.x86_64 +++ linux-2.6/arch/x86/Kconfig.x86_64 @@ -97,6 +97,10 @@ config X86_CMPXCHG bool default y +config FAST_CMPXCHG_LOCAL + bool + default y + config EARLY_PRINTK bool default y - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists