SLQB slab allocator
Subsystems (7)
PATCH 1/1 SLQB slab allocator
Hi,
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with), and improved performance and reduced locking somewhat for node-specific and interleaved allocations.
There are a few TODOs remaining (see "TODO"). Most are hopefully obscure or relatively unimportant cases. The biggest thing really is to test and tune on a wider range of workloads, so I'll ask to merge it in the slab tree and from there linux-next to see what comes up. I'll work on tuning things and the TODO items before a possible mainline merge. Actually it would be kind of instructive if people run into issues on the TODO list because it would help guide improvements...
BTW, if anybody wants explicit copyright attribution on the files, that's fine just send patches. I just dislike big header buildups, which is why I make a broader acknowledgement. In fact, the other allocators don't even explictly acknowledge SLAB, so I didn't think it would be a problem. I don't really know the legal issues, but we've set plenty of precendent...
---
Introducing the SLQB slab allocator.
SLQB takes code and ideas from all other slab allocators in the tree.
The primary method for keeping lists of free objects within the allocator is a singly-linked list, storing a pointer within the object memory itself (or a small additional space in the case of RCU destroyed slabs). This is like SLOB and SLUB, and opposed to SLAB, which uses arrays of objects, and metadata. This reduces memory consumption and makes smaller sized objects more realistic as there is less overhead.
Using lists rather than arrays can reduce the cacheline footprint. When moving objects around, SLQB can move a list of objects from one CPU to another by simply manipulating a head pointer, wheras SLAB needs to memcpy arrays. Some SLAB per-CPU arrays can be up to 1K in size, which is a lot of cachelines that can be touched during alloc/free. Newly freed objects tend to be cache hot, and newly allocated ones tend to soon be touched anyway, so often there is little cost to using metadata in the objects.
SLQB has a per-CPU LIFO freelist of objects like SLAB (but using lists rather than arrays). Freed objects are returned to this freelist if they belong to the node which our CPU belongs to. So objects allocated on one CPU can be added to the freelist of another CPU on the same node. When LIFO freelists need to be refilled or trimmed, SLQB takes or returns objects from a list of slabs.
SLQB has per-CPU lists of slabs (which use struct page as their metadata including list head for this list). Each slab contains a singly-linked list of objects that are free in that slab (free, and not on a LIFO freelist). Slabs are freed as soon as all their objects are freed, and only allocated when there are no slabs remaining. They are taken off this slab list when if there are no free objects left. So the slab lists always only contain "partial" slabs; those slabs which are not completely full and not completely empty. SLQB slabs can be manipulated with no locking unlike other allocators which tend to use per-node locks. As the number of threads per socket increases, this should help improve the scalability of slab operations.
Freeing objects to remote slab lists first batches up the objects on the freeing CPU, then moves them over at once to a list on the allocating CPU. The allocating CPU will then notice those objects and pull them onto the end of its freelist. This remote freeing scheme is designed to minimise the number of cross CPU cachelines touched, short of going to a "crossbar" arrangement like SLAB has. SLAB has "crossbars" of arrays of objects. That is, NR_CPUS*MAX_NUMNODES type arrays, which can become very bloated in huge systems (this could be hundreds of GBs for kmem caches for 4096 CPU, 1024 nodes systems).
SLQB also has similar freelist, slablist structures per-node, which are protected by a lock, and usable by any CPU in order to do node specific allocations. These allocations tend not to be too frequent (short lived allocations should be node local, long lived allocations should not be too frequent).
There is a good overview and illustration of the design here:
http://lwn.net/Articles/311502/
By using LIFO freelists like SLAB, SLQB tries to be very page-size agnostic. It tries very hard to use order-0 pages. This is good for both page allocator fragmentation, and slab fragmentation.
SLQB initialistaion code attempts to be as simple and un-clever as possible. There are no multiple phases where different things come up. There is no weird self bootstrapping stuff. It just statically allocates the structures required to create the slabs that allocate other slab structures.
SLQB uses much of the debugging infrastructure, and fine-grained sysfs statistics from SLUB. There is also a Documentation/vm/slqbinfo.c, derived from slabinfo.c, which can query the sysfs data.
---
include/linux/rcupdate.h +1 -10
@@ -33,6 +33,7 @@#ifndef __LINUX_RCUPDATE_H#define __LINUX_RCUPDATE_H +#include <linux/rcu_types.h> #include <linux/cache.h> #include <linux/spinlock.h> #include <linux/threads.h>
@@ -42,16 +43,6 @@#include <linux/lockdep.h> #include <linux/completion.h> -/** -* struct rcu_head - callback structure for use with RCU -* @next: next update requests in a list -* @func: actual update function to call after the grace period. -*/ -struct rcu_head { -struct rcu_head *next; -void (*func)(struct rcu_head *head); -}; -#if defined(CONFIG_CLASSIC_RCU )#include <linux/rcuclassic.h> #elif defined(CONFIG_TREE_RCU )
include/linux/slqb_def.h new file +283 -0
283 lines not on this page · raw message
init/Kconfig +7 -2
@@ -806,7 +806,7 @@ config SLUB_DEBUG choice prompt "Choose SLAB allocator" - default SLUB + default SLQB help This option allows to select a slab allocator.
@@ -827,6 +827,11 @@ config SLUB and has enhanced diagnostics. SLUB is the default choice for a slab allocator. +config SLQB + bool "SLQB (Qeued allocator)" + help + SLQB is a proposed new slab allocator. + config SLOB depends on EMBEDDED bool "SLOB (Simple Allocator)"
@@ -868,7 +873,7 @@ config HAVE_GENERIC_DMA_COHERENT config SLABINFO bool depends on PROC_FS - depends on SLAB || SLUB_DEBUG + depends on SLAB || SLUB_DEBUG || SLQB default y config RT_MUTEXES
lib/Kconfig.debug +20 -0
@@ -298,6 +298,26 @@ config SLUB_STATS out which slabs are relevant to a particular load. Try running: slabinfo -DA +config SLQB_DEBUG + default y + bool "Enable SLQB debugging support" + depends on SLQB + +config SLQB_DEBUG_ON + default n + bool "SLQB debugging on by default" + depends on SLQB_DEBUG + +config SLQB_SYSFS + bool "Create SYSFS entries for slab caches" + default n + depends on SLQB + +config SLQB_STATS + bool "Enable SLQB performance statistics" + default n + depends on SLQB_SYSFS + config DEBUG_PREEMPT bool "Debug preemptible kernel" depends on DEBUG_KERNEL && PREEMPT && (TRACE_IRQFLAGS_SUPPORT || PPC64)
mm/slqb.c new file +3436 -0
2,641 lines not on this page · raw message
Documentation/vm/slqbinfo.c new file +1054 -0
1,054 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Part of the diff is not shown here. View the raw message.
Quoted 8 lines · Nick Piggin
include/linux/slqb_def.h new file +8
-0+/* +* Management object for a slab cache. +*/ +struct kmem_cache { +unsigned long flags; +int hiwater;/* LIFO list high watermark */ +int freebatch;/* LIFO freelist batch flush size */ +int objsize;/* The size of an object without meta data */
include/linux/slqb_def.h line 134
Quoted 10 lines
include/linux/slqb_def.h new file +10
-0+int offset;/* Free pointer offset. */ +int objects;/* Number of objects in slab */ + +int size;/* The size of an object including meta data */ +int order;/* Allocation order */ +gfp_t allocflags;/* gfp flags to use on allocation */ +unsigned int colour_range;/* range of colour counter */ +unsigned int colour_off;/* offset per colour */ +void (*ctor)(void *); +
Mind if i nitpick a bit about minor style issues? Since this is going to be the next Linux SLAB allocator we might as well do it perfectly :-)
When intoducing new structures it makes sense to properly vertical align them, like:
include/linux/slqb_def.h line 139
Quoted 11 lines
include/linux/slqb_def.h new file +11
-0+unsigned long flags; +int hiwater;/* LIFO list high watermark */ +int freebatch;/* LIFO freelist batch flush size */ +int objsize;/* Object size without meta data */ +int offset;/* Free pointer offset */ +int objects;/* Number of objects in slab */ +const char *name;/* Name (only for display!) */ +struct list_head list;/* List of slab caches */ + +int align;/* Alignment */ +int inuse;/* Offset to metadata */
because proper vertical alignment/lineup can really help readability. Like you do it yourself here:
Quoted 14 lines
include/linux/slqb_def.h new file +14
-0+if (size <=8 )return 3 ; +if (size <=16 )return 4 ; +if (size <=32 )return 5 ; +if (size <=64 )return 6 ; +if (size <=128 )return 7 ; +if (size <=256 )return 8 ; +if (size <=512 )return 9 ; +if (size <=1024 )return 10 ; +if (size <=2 *1024 )return 11 ; +if (size <=4 *1024 )return 12 ; +if (size <=8 *1024 )return 13 ; +if (size <=16 *1024 )return 14 ; +if (size <=32 *1024 )return 15 ; +if (size <=64 *1024 )return 16 ;
include/linux/slqb_def.h
Quoted 10 lines
include/linux/slqb_def.h new file +9
-0+if (size <=128 *1024 )return 17 ; +if (size <=256 *1024 )return 18 ; +if (size <=512 *1024 )return 19 ; +if (size <=1024 *1024 )return 20 ; +if (size <=2 *1024 *1024 )return 21 ; +static void slab_err (struct kmem_cache *s,struct slqb_page *page,char *fmt, ...) +{ +va_list args; +char buf[100 ];
magic constant.
mm/slqb.c line 530
Quoted 4 lines
mm/slqb.c new file +4
-0+if (s->flags &SLAB_RED_ZONE ) +memset (p + s->objsize, + active ?SLUB_RED_ACTIVE :SLUB_RED_INACTIVE , + s->inuse - s->objsize);
We tend to add curly braces in such multi-line statement situations i guess.
mm/slqb.c line 722
Quoted 8 lines
mm/slqb.c new file +8
-0+static void trace (struct kmem_cache *s,struct slqb_page *page,void *object,int alloc) +{ +if (s->flags &SLAB_TRACE ) { +printk (KERN_INFO "TRACE %s %s 0x%p inuse=%d fp=0x%p\n" , + s->name, + alloc ?"alloc" :"free" , + object, page->inuse, + page->freelist);
Could use ftrace_printk() here i guess. That way it goes into a fast ringbuffer and not printk and it also gets embedded into whatever tracer plugin there is active. (for example kmemtrace)
mm/slqb.c line 732
Quoted 2 lines
mm/slqb.c new file +2
-0+static void setup_object_debug (struct kmem_cache *s,struct slqb_page *page, +void *object)
there's a trick that can be done here to avoid the col-80 artifact:
static voidsetup_object_debug(struct kmem_cache *s, struct slqb_page *page, void *object)
ditto all these prototypes:
Quoted 7 lines
mm/slqb.c new file +7
-0+static int alloc_debug_processing (struct kmem_cache *s,void *object,void *addr) +static int free_debug_processing(struct kmem_cache *s, void *object, void *addr) +static unsigned long kmem_cache_flags(unsigned long objsize, + unsigned long flags, const char *name, + void (*ctor)(void *)) +static inline void setup_object_debug(struct kmem_cache *s, + struct slqb_page *page, void *object) {}
mm/slqb.c line 891
Quoted 10 lines
mm/slqb.c new file +9
-0+static inline int alloc_debug_processing (struct kmem_cache *s, +void *object,void *addr) {return 0 ; } +static inline int free_debug_processing (struct kmem_cache *s, +void *object,void *addr) {return 0 ; } +static inline int check_object (struct kmem_cache *s,struct slqb_page *page, +void *object,int active) {return 1 ; } +static inline unsigned long kmem_cache_flags (unsigned long objsize, + unsigned long flags, const char *name, void (*ctor)(void *)) +#define slqb_debug 0
should be 'static const int slqb_debug;' i guess?
more function prototype inconsistencies:
Review
Quoted 9 lines
(quoted diff) +8
-0+static struct slqb_page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) +static void setup_object(struct kmem_cache *s, struct slqb_page *page, + void *object) +static struct slqb_page *new_slab_page(struct kmem_cache *s, gfp_t flags, int node, unsigned int colour) +static int free_object_to_page(struct kmem_cache *s, struct kmem_cache_list *l, struct slqb_page *page, void *object) +#ifdef CONFIG_SMP +static noinline void slab_free_to_remote(struct kmem_cache *s, struct slqb_page *page, void *object, struct kmem_cache_cpu *c); +#endif
does noline have to be declared?
i almost missed the lock taking here:
mm/slqb.c line 1134
Quoted 7 lines
mm/slqb.c new file +7
-0+spin_lock (&l->remote_free.lock); + l->remote_free.list.head =NULL ; + tail = l->remote_free.list.tail; + l->remote_free.list.tail =NULL ; + nr = l->remote_free.list.nr; + l->remote_free.list.nr =0 ; +spin_unlock (&l->remote_free.lock);
Putting an extra newline after the spin_lock() and one extra newline before the spin_unlock() really helps raise attention to critical sections.
various leftover bits:
mm/slqb.c line 1226
Quoted 10 lines
mm/slqb.c new file +7
-0+// if (next) +// prefetchw(next); +// if (next) +// prefetchw(next); +list_del (&page->lru); +/*XXX list_move(&page->lru, &l->full); */ +// VM_BUG_ON(node != -1 && node != slqb_page_to_nid(page));
overlong prototype:
Quoted 1 line
+static noinline void *__slab_alloc_page(struct
kmem_cache*s,gfp_tgfpflags, int node)
putting the newline elsewhere would improve this too:
Review
Quoted 2 lines
(quoted diff) +2
-0+static noinline void *__remote_slab_alloc(struct kmem_cache *s, + gfp_t gfpflags, int node)
leftover:
mm/slqb.c line 1341
Quoted 2 lines
mm/slqb.c new file +2
-0+// if (unlikely(!(l->freelist.nr | l->nr_partial | l->remote_free_check))) +// return NULL;
newline in wrong place:
mm/slqb.c line 1468
Quoted 13 lines
mm/slqb.c new file +10
-0+static __always_inline void *__slab_alloc(struct kmem_cache *s, + gfp_t gfpflags, int node) +static __always_inline void *slab_alloc(struct kmem_cache *s, + gfp_t gfpflags, int node, void *addr) +static __always_inline void *__kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags, void *caller) +#ifdef CONFIG_SLQB_STATS + { +struct kmem_cache_list *l = &c->list; +slqb_stat_inc (l,FLUSH_RFREE_LIST ); +slqb_stat_add (l,FLUSH_RFREE_LIST_OBJECTS , nr);
Please put a newline after local variable declarations.
newline in another place could improve this:
mm/slqb.c line 1572
Quoted 12 lines
mm/slqb.c new file +11
-0+static __always_inline void __slab_free(struct kmem_cache *s, +struct slqb_page *page,void *object) +#ifdef CONFIG_NUMA + } else { +/* +* Freeing an object that was allocated on a remote node. +*/ +slab_free_to_remote (s, page, object, c); +slqb_stat_inc (l,FREE_REMOTE ); +#endif + }
while it's correct code, the CONFIG_NUMA ifdef begs to be placed one line further down.
newline in another place could improve this:
mm/slqb.c line 1601
Quoted 8 lines
mm/slqb.c new file +7
-0+static __always_inline void slab_free(struct kmem_cache *s, +struct slqb_page *page,void *object) +void kmem_cache_free(struct kmem_cache *s, void *object) +{ +struct slqb_page *page =NULL ; +if (numa_platform) + page =virt_to_head_slqb_page (object);
newline after local variable definition please.
mm/slqb.c line 1621
Quoted 9 lines
mm/slqb.c new file +9
-0+static inline int slab_order (int size,int max_order,int frac) +{ +int order; + +if (fls (size -1 ) <=PAGE_SHIFT ) + order =0 ; +else + order =fls (size -1 ) -PAGE_SHIFT ; +while (order <= max_order) {
Please put a newline before loops, so that they stand out better.
Quoted 13 lines
mm/slqb.c new file +13
-0+static inline int calculate_order (int size) +{ +int order; + +/* +* Attempt to find best configuration for a slab. This +* works by first attempting to generate a layout with +* the best configuration and backing off gradually. +*/ + order =slab_order (size,1 ,4 ); +if (order <=1 ) +return order; +
mm/slqb.c line 1663
Quoted 10 lines
mm/slqb.c new file +10
-0+/* +* This size cannot fit in order-1. Allow bigger orders, but +* forget about trying to save space. +*/ + order =slab_order (size,MAX_ORDER ,0 ); +if (order <=MAX_ORDER ) +return order; + +return -ENOSYS ; +}
function with very nice typographics. All should be like this.
mm/slqb.c line 1681
Quoted 4 lines
mm/slqb.c new file +4
-0+if (flags &SLAB_HWCACHE_ALIGN ) { +unsigned long ralign =cache_line_size (); +while (size <= ralign /2 ) + ralign /=2 ;
newline after variables please.
mm/slqb.c line 1700
Quoted 10 lines
mm/slqb.c new file +10
-0+static void init_kmem_cache_list (struct kmem_cache *s,struct kmem_cache_list *l) +{ + l->cache = s; + l->freelist.nr =0 ; + l->freelist.head =NULL ; + l->freelist.tail =NULL ; + l->nr_partial =0 ; + l->nr_slabs =0 ; +INIT_LIST_HEAD (&l->partial); +// INIT_LIST_HEAD(&l->full);
leftover. Also, initializations tend to read nicer if they are aligned like this:
Quoted 4 lines
mm/slqb.c new file +4
-0+ l->cache = s; + l->freelist.nr =0 ; + l->freelist.head =NULL ; + l->freelist.tail =NULL ;
mm/slqb.c line 1708
Quoted 10 lines
mm/slqb.c new file +10
-0+ l->nr_partial =0 ; + l->nr_slabs =0 ; + +#ifdef CONFIG_SMP + l->remote_free_check =0 ; +spin_lock_init (&l->remote_free.lock); + l->remote_free.list.nr =0 ; + l->remote_free.list.head =NULL ; + l->remote_free.list.tail =NULL ; +#endif
As this way it really stands out that the only relevant non-zero initializations are l->cache and the spinlock init.
mm/slqb.c line 1716
Quoted 2 lines
mm/slqb.c new file +2
-0+static void init_kmem_cache_cpu (struct kmem_cache *s, +struct kmem_cache_cpu *c)
prototype newline.
dead code:
mm/slqb.c line 2484
Quoted 12 lines
mm/slqb.c new file +12
-0+#if 0 // XXX: see cpu offline comment +down_read (&slqb_lock); +list_for_each_entry (s, &slab_caches, list) { +struct kmem_cache_node *n; + n = s->node[nid]; +if (n) { + s->node[nid] =NULL ; +kmem_cache_free (&kmem_node_cache, n); + } + } +up_read (&slqb_lock); +#endif
... and many more similar instances are in the patch in other places.
Ingo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 16 lines · Ingo Molnar
Quoted 13 lines · Nick Piggin
include/linux/slqb_def.h new file +13
-0+/* +* Management object for a slab cache. +*/ +struct kmem_cache { +unsigned long flags; +int hiwater;/* LIFO list high watermark */ +int freebatch;/* LIFO freelist batch flush size */ +int objsize;/* The size of an object without meta data */ +int offset;/* Free pointer offset. */ +int objects;/* Number of objects in slab */ + +int size;/* The size of an object including meta data */ +int order;/* Allocation order */
Quoted 8 lines
include/linux/slqb_def.h line 134
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+gfp_t allocflags;/* gfp flags to use on allocation */ +unsigned int colour_range;/* range of colour counter */ +unsigned int colour_off;/* offset per colour */ +void (*ctor)(void *); +Mind if i nitpick a bit about minor style issues? Since this is going to be the next Linux SLAB allocator we might as well do it perfectly :-)
Well, let's not get ahead of ourselves :) But it's very appreciated.
I think most if not all of your suggestions are good ones, although I probably won't convert to ftrace just for the moment.
I'll come up with an incremental patch....
Thanks,
Nick
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 3 lines · Ingo Molnar
Mind if i nitpick a bit about minor style issues? Since this is going to be the next Linux SLAB allocator we might as well do it perfectly :-)
Well here is an incremental patch which should get most of the issues you pointed out, most of the sane ones that checkpatch pointed out, and a few of my own ;)
--- include/linux/slqb_def.h | 90 +++++----- mm/slqb.c | 386 +++++++++++++++++++++++++---------------------- 2 files changed, 261 insertions(+), 215 deletions(-)
include/linux/slqb_def.h +48 -42
@@ -37,8 +37,9 @@ enum stat_item {* Singly-linked list with head, tail, and nr */ struct kmlist { -unsigned long nr; -void **head, **tail; +unsigned long nr; +void **head; +void **tail; };/*
@@ -46,8 +47,8 @@ struct kmlist {* objects can be returned to the kmem_cache_list from remote CPUs. */ struct kmem_cache_remote_free { -spinlock_t lock; -struct kmlist list; +spinlock_t lock; +struct kmlist list; }____cacheline_aligned ;/*
@@ -56,18 +57,23 @@ struct kmem_cache_remote_free {* kmem_cache_lists allow off-node allocations (but require locking). */ struct kmem_cache_list { -struct kmlist freelist;/* Fastpath LIFO freelist of objects */ +/* Fastpath LIFO freelist of objects */ +struct kmlist freelist;#ifdef CONFIG_SMP -int remote_free_check;/* remote_free has reached a watermark */ +/* remote_free has reached a watermark */ +int remote_free_check;#endif -struct kmem_cache *cache;/* kmem_cache corresponding to this list */ +/* kmem_cache corresponding to this list */ +struct kmem_cache *cache; -unsigned long nr_partial;/* Number of partial slabs (pages) */ -struct list_head partial;/* Slabs which have some free objects */ +/* Number of partial slabs (pages) */ +unsigned long nr_partial; -unsigned long nr_slabs;/* Total number of slabs allocated */ +/* Slabs which have some free objects */ +struct list_head partial; -//struct list_head full; +/* Total number of slabs allocated */ +unsigned long nr_slabs;#ifdef CONFIG_SMP /*
@@ -79,7 +85,7 @@ struct kmem_cache_list {#endif #ifdef CONFIG_SLQB_STATS -unsigned long stats[NR_SLQB_STAT_ITEMS ]; +unsigned long stats[NR_SLQB_STAT_ITEMS ];#endif }____cacheline_aligned ;
@@ -87,9 +93,8 @@ struct kmem_cache_list {* Primary per-cpu, per-kmem_cache structure. */ struct kmem_cache_cpu { -struct kmem_cache_list list;/* List for node-local slabs. */ - -unsigned int colour_next; +struct kmem_cache_list list;/* List for node-local slabs */ +unsigned int colour_next;/* Next colour offset to use */ #ifdef CONFIG_SMP /*
@@ -101,53 +106,53 @@ struct kmem_cache_cpu {* An NR_CPUS or MAX_NUMNODES array would be nice here, but then we * get to O(NR_CPUS^2) memory consumption situation. */ -struct kmlist rlist; -struct kmem_cache_list *remote_cache_list; +struct kmlist rlist; +struct kmem_cache_list *remote_cache_list;#endif }____cacheline_aligned ;/* -* Per-node, per-kmem_cache structure. +* Per-node, per-kmem_cache structure. Used for node-specific allocations. */ struct kmem_cache_node { -struct kmem_cache_list list; -spinlock_t list_lock;/* protects access to list */ +struct kmem_cache_list list; +spinlock_t list_lock;/* protects access to list */ }____cacheline_aligned ;/* * Management object for a slab cache. */ struct kmem_cache { -unsigned long flags; -int hiwater;/* LIFO list high watermark */ -int freebatch;/* LIFO freelist batch flush size */ -int objsize;/* The size of an object without meta data */ -int offset;/* Free pointer offset. */ -int objects;/* Number of objects in slab */ - -int size;/* The size of an object including meta data */ -int order;/* Allocation order */ -gfp_t allocflags;/* gfp flags to use on allocation */ -unsigned int colour_range;/* range of colour counter */ -unsigned int colour_off;/* offset per colour */ -void (*ctor)(void *); +unsigned long flags; +int hiwater;/* LIFO list high watermark */ +int freebatch;/* LIFO freelist batch flush size */ +int objsize;/* Size of object without meta data */ +int offset;/* Free pointer offset. */ +int objects;/* Number of objects in slab */ + +int size;/* Size of object including meta data */ +int order;/* Allocation order */ +gfp_t allocflags;/* gfp flags to use on allocation */ +unsigned int colour_range;/* range of colour counter */ +unsigned int colour_off;/* offset per colour */ +void (*ctor)(void *); -const char *name;/* Name (only for display!) */ -struct list_head list;/* List of slab caches */ +const char *name;/* Name (only for display!) */ +struct list_head list;/* List of slab caches */ -int align;/* Alignment */ -int inuse;/* Offset to metadata */ +int align;/* Alignment */ +int inuse;/* Offset to metadata */ #ifdef CONFIG_SLQB_SYSFS -struct kobject kobj;/* For sysfs */ +struct kobject kobj;/* For sysfs */ #endif #ifdef CONFIG_NUMA -struct kmem_cache_node *node[MAX_NUMNODES ]; +struct kmem_cache_node *node[MAX_NUMNODES ];#endif #ifdef CONFIG_SMP -struct kmem_cache_cpu *cpu_slab[NR_CPUS ]; +struct kmem_cache_cpu *cpu_slab[NR_CPUS ];#else -struct kmem_cache_cpu cpu_slab; +struct kmem_cache_cpu cpu_slab;#endif };
@@ -245,7 +250,8 @@ void *__kmalloc(size_t size, gfp_t flags#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)#endif -#define KMALLOC_HEADER (ARCH_KMALLOC_MINALIGN < sizeof(void *) ? sizeof(void *) : ARCH_KMALLOC_MINALIGN) +#define KMALLOC_HEADER (ARCH_KMALLOC_MINALIGN < sizeof(void *) ? \ + sizeof(void *) : ARCH_KMALLOC_MINALIGN) static __always_inline void *kmalloc(size_t size, gfp_t flags) {
mm/slqb.c +213 -173
1,035 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Nick Piggin
Quoted 3 lines · Ingo Molnar
Mind if i nitpick a bit about minor style issues? Since this is going to be the next Linux SLAB allocator we might as well do it perfectly :-)
Well here is an incremental patch which should get most of the issues you pointed out, most of the sane ones that checkpatch pointed out, and a few of my own ;)
here's an incremental one ontop of your incremental patch, enhancing some more issues. I now find the code very readable! :-)
( in case you are wondering about the placement of bit_spinlock.h - that
file needs fixing, just move it to the top of the file and see the build break. But that's a separate patch.)
Ingo
------------------->
Subject: slbq: cleanup
From: Ingo Molnar <[email protected]>
Date: Wed Jan 21 18:10:20 CET 2009
mm/slqb.o:
text data bss dec hex filename
17655 54159 200456 272270 4278e slqb.o.before 17653 54159 200456 272268 4278c slqb.o.after
--- mm/slqb.c | 588 ++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 308 insertions(+), 280 deletions(-)
mm/slqb.c +308 -280
@@ -7,19 +7,20 @@* Using ideas and code from mm/slab.c, mm/slob.c, and mm/slub.c. */ -#include <linux/mm.h> -#include <linux/module.h> -#include <linux/bit_spinlock.h> #include <linux/interrupt.h> -#include <linux/bitops.h> -#include <linux/slab.h> -#include <linux/seq_file.h> -#include <linux/cpu.h> -#include <linux/cpuset.h> #include <linux/mempolicy.h> -#include <linux/ctype.h> #include <linux/kallsyms.h> +#include <linux/seq_file.h> +#include <linux/bitops.h> +#include <linux/cpuset.h> #include <linux/memory.h> +#include <linux/module.h> +#include <linux/ctype.h> +#include <linux/slab.h> +#include <linux/cpu.h> +#include <linux/mm.h> + +#include <linux/bit_spinlock.h> /* * TODO
@@ -40,14 +41,14 @@struct slqb_page {union {struct { -unsigned long flags;/* mandatory */ -atomic_t _count;/* mandatory */ -unsigned int inuse;/* Nr of objects */ +unsigned long flags;/* mandatory */ +atomic_t _count;/* mandatory */ +unsigned int inuse;/* Nr of objects */ struct kmem_cache_list *list;/* Pointer to list */ -void **freelist;/* LIFO freelist */ +void **freelist;/* LIFO freelist */ union { -struct list_head lru;/* misc. list */ -struct rcu_head rcu_head;/* for rcu freeing */ +struct list_head lru;/* misc. list */ +struct rcu_head rcu_head;/* for rcu freeing */ }; };struct page page;
@@ -120,16 +121,16 @@ static inline int slab_freebatch(struct* - There is no remote free queue. Nodes don't free objects, CPUs do. */ -static inline void slqb_stat_inc (struct kmem_cache_list *list, -enum stat_item si) +static inline void +slqb_stat_inc (struct kmem_cache_list *list,enum stat_item si) {#ifdef CONFIG_SLQB_STATS list->stats[si]++;#endif } -static inline void slqb_stat_add (struct kmem_cache_list *list, -enum stat_item si,unsigned long nr) +static inline void +slqb_stat_add (struct kmem_cache_list *list,enum stat_item si,unsigned long nr) {#ifdef CONFIG_SLQB_STATS list->stats[si] += nr;
@@ -196,12 +197,12 @@ static inline void __free_slqb_pages(str#ifdef CONFIG_SLQB_DEBUG static inline int slab_debug (struct kmem_cache *s) { -return (s->flags & +return s->flags & (SLAB_DEBUG_FREE |SLAB_RED_ZONE |SLAB_POISON |SLAB_STORE_USER | -SLAB_TRACE )); +SLAB_TRACE ); }static inline int slab_poison (struct kmem_cache *s) {
@@ -574,34 +575,34 @@ static int check_bytes_and_report(struct* Object layout: * * object address -* Bytes of the object to be managed. -* If the freepointer may overlay the object then the free -* pointer is the first word of the object. +* Bytes of the object to be managed. +* If the freepointer may overlay the object then the free +* pointer is the first word of the object. * -* Poisoning uses 0x6b (POISON_FREE) and the last byte is -* 0xa5 (POISON_END) +* Poisoning uses 0x6b (POISON_FREE) and the last byte is +* 0xa5 (POISON_END) * * object + s->objsize -* Padding to reach word boundary. This is also used for Redzoning. -* Padding is extended by another word if Redzoning is enabled and -* objsize == inuse. +* Padding to reach word boundary. This is also used for Redzoning. +* Padding is extended by another word if Redzoning is enabled and +* objsize == inuse. * -* We fill with 0xbb (RED_INACTIVE) for inactive objects and with -* 0xcc (RED_ACTIVE) for objects in use. +* We fill with 0xbb (RED_INACTIVE) for inactive objects and with +* 0xcc (RED_ACTIVE) for objects in use. * * object + s->inuse -* Meta data starts here. +* Meta data starts here. * -* A. Free pointer (if we cannot overwrite object on free) -* B. Tracking data for SLAB_STORE_USER -* C. Padding to reach required alignment boundary or at mininum -* one word if debuggin is on to be able to detect writes -* before the word boundary. +* A. Free pointer (if we cannot overwrite object on free) +* B. Tracking data for SLAB_STORE_USER +* C. Padding to reach required alignment boundary or at mininum +* one word if debuggin is on to be able to detect writes +* before the word boundary. * * Padding is done using 0x5a (POISON_INUSE) * * object + s->size -* Nothing is used beyond s->size. +* Nothing is used beyond s->size. */ static int check_pad_bytes (struct kmem_cache *s,struct slqb_page *page,u8 *p)
@@ -717,25 +718,26 @@ static int check_slab(struct kmem_cachereturn 1 ; } -static void trace (struct kmem_cache *s,struct slqb_page *page, -void *object,int alloc) +static void +trace (struct kmem_cache *s,struct slqb_page *page,void *object,int alloc) { -if (s->flags &SLAB_TRACE ) { -printk (KERN_INFO "TRACE %s %s 0x%p inuse=%d fp=0x%p\n" , - s->name, - alloc ?"alloc" :"free" , - object, page->inuse, - page->freelist); +if (likely (!(s->flags &SLAB_TRACE ))) +return ; -if (!alloc) -print_section ("Object" , (void *)object, s->objsize); +printk (KERN_INFO "TRACE %s %s 0x%p inuse=%d fp=0x%p\n" , + s->name, + alloc ?"alloc" :"free" , + object, page->inuse, + page->freelist); -dump_stack (); - } +if (!alloc) +print_section ("Object" , (void *)object, s->objsize); + +dump_stack (); } -static void setup_object_debug (struct kmem_cache *s,struct slqb_page *page, -void *object) +static void +setup_object_debug (struct kmem_cache *s,struct slqb_page *page,void *object) {if (!slab_debug (s))return ;
@@ -747,11 +749,10 @@ static void setup_object_debug(struct kminit_tracking (s, object); } -static int alloc_debug_processing (struct kmem_cache *s, -void *object,void *addr) +static int +alloc_debug_processing (struct kmem_cache *s,void *object,void *addr) { -struct slqb_page *page; - page =virt_to_head_slqb_page (object); +struct slqb_page *page =virt_to_head_slqb_page (object);if (!check_slab (s, page)) goto bad;
@@ -767,6 +768,7 @@ static int alloc_debug_processing(struct/* Success perform special debug activities for allocs */ if (s->flags &SLAB_STORE_USER )set_track (s, object,TRACK_ALLOC , addr); +trace (s, page, object,1 );init_object (s, object,1 );return 1 ;
@@ -775,11 +777,9 @@ bad:return 0 ; } -static int free_debug_processing (struct kmem_cache *s, -void *object,void *addr) +static int free_debug_processing (struct kmem_cache *s,void *object,void *addr) { -struct slqb_page *page; - page =virt_to_head_slqb_page (object); +struct slqb_page *page =virt_to_head_slqb_page (object);if (!check_slab (s, page)) goto fail;
@@ -870,29 +870,34 @@ static unsigned long kmem_cache_flags(un void (*ctor)(void *)) {/* -* Enable debugging if selected on the kernel commandline. +* Enable debugging if selected on the kernel commandline: */ -if (slqb_debug && (!slqb_debug_slabs || -strncmp (slqb_debug_slabs, name, -strlen (slqb_debug_slabs)) ==0 )) - flags |= slqb_debug; + +if (!slqb_debug) +return flags; + +if (slqb_debug_slabs) +return flags | slqb_debug; + +if (!strncmp (slqb_debug_slabs, name,strlen (slqb_debug_slabs))) +return flags | slqb_debug;return flags; }#else -static inline void setup_object_debug (struct kmem_cache *s, -struct slqb_page *page,void *object) +static inline void +setup_object_debug (struct kmem_cache *s,struct slqb_page *page,void *object) { } -static inline int alloc_debug_processing (struct kmem_cache *s, -void *object,void *addr) +static inline int +alloc_debug_processing (struct kmem_cache *s,void *object,void *addr) {return 0 ; } -static inline int free_debug_processing (struct kmem_cache *s, -void *object,void *addr) +static inline int +free_debug_processing (struct kmem_cache *s,void *object,void *addr) {return 0 ; }
@@ -903,7 +908,7 @@ static inline int slab_pad_check(struct }static inline int check_object (struct kmem_cache *s,struct slqb_page *page, -void *object,int active) +void *object,int active) {return 1 ; }
@@ -924,11 +929,11 @@ static const int slqb_debug = 0;/* * allocate a new slab (return its corresponding struct slqb_page) */ -static struct slqb_page *allocate_slab (struct kmem_cache *s, -gfp_t flags,int node) +static struct slqb_page * +allocate_slab (struct kmem_cache *s,gfp_t flags,int node) { -struct slqb_page *page;int pages =1 << s->order; +struct slqb_page *page; flags |= s->allocflags;
@@ -947,8 +952,8 @@ static struct slqb_page *allocate_slab(s/* * Called once for each object on a new slab page */ -static void setup_object (struct kmem_cache *s, -struct slqb_page *page,void *object) +static void +setup_object (struct kmem_cache *s,struct slqb_page *page,void *object) {setup_object_debug (s, page, object);if (unlikely (s->ctor))
@@ -958,8 +963,8 @@ static void setup_object(struct kmem_cac/* * Allocate a new slab, set up its object list. */ -static struct slqb_page *new_slab_page (struct kmem_cache *s, -gfp_t flags,int node,unsigned int colour) +static struct slqb_page * +new_slab_page (struct kmem_cache *s,gfp_t flags,int node,unsigned int colour) {struct slqb_page *page;void *start;
@@ -1030,6 +1035,7 @@ static void rcu_free_slab(struct rcu_heastatic void free_slab (struct kmem_cache *s,struct slqb_page *page) {VM_BUG_ON (page->inuse); +if (unlikely (s->flags &SLAB_DESTROY_BY_RCU ))call_rcu (&page->rcu_head, rcu_free_slab);else
@@ -1060,12 +1066,14 @@ static int free_object_to_page(struct km l->nr_slabs--;free_slab (s, page);slqb_stat_inc (l,FLUSH_SLAB_FREE ); +return 1 ; }else if (page->inuse +1 == s->objects) { l->nr_partial++;list_add (&page->lru, &l->partial);slqb_stat_inc (l,FLUSH_SLAB_PARTIAL ); +return 0 ; }return 0 ;
@@ -1146,8 +1154,8 @@ static void flush_free_list_all(struct k* Caller must be the owner CPU in the case of per-CPU list, or hold the node's * list_lock in the case of per-node list. */ -static void claim_remote_free_list (struct kmem_cache *s, -struct kmem_cache_list *l) +static void +claim_remote_free_list (struct kmem_cache *s,struct kmem_cache_list *l) {void **head, **tail;int nr;
@@ -1192,8 +1200,8 @@ static void claim_remote_free_list(struc* Caller must be the owner CPU in the case of per-CPU list, or hold the node's * list_lock in the case of per-node list. */ -static __always_inline void *__cache_list_get_object(struct kmem_cache *s, -struct kmem_cache_list *l) +static __always_inline void * +__cache_list_get_object (struct kmem_cache *s,struct kmem_cache_list *l) {void *object;
@@ -1243,8 +1251,8 @@ static __always_inline void *__cache_lis* Caller must be the owner CPU in the case of per-CPU list, or hold the node's * list_lock in the case of per-node list. */ -static noinline void *__cache_list_get_page(struct kmem_cache *s, -struct kmem_cache_list *l) +static noinline void * +__cache_list_get_page (struct kmem_cache *s,struct kmem_cache_list *l) {struct slqb_page *page;void *object;
@@ -1282,12 +1290,12 @@ static noinline void *__cache_list_get_p* * Must be called with interrupts disabled. */ -static noinline void *__slab_alloc_page(struct kmem_cache *s, -gfp_t gfpflags,int node) +static noinline void * +__slab_alloc_page (struct kmem_cache *s,gfp_t gfpflags,int node) { -struct slqb_page *page;struct kmem_cache_list *l;struct kmem_cache_cpu *c; +struct slqb_page *page;unsigned int colour;void *object;
@@ -1347,15 +1355,19 @@ static noinline void *__slab_alloc_page( }#ifdef CONFIG_NUMA -static noinline int alternate_nid(struct kmem_cache *s, -gfp_t gfpflags,int node) +static noinline int +alternate_nid (struct kmem_cache *s,gfp_t gfpflags,int node) {if (in_interrupt () || (gfpflags & __GFP_THISNODE))return node; -if (cpuset_do_slab_mem_spread () && (s->flags &SLAB_MEM_SPREAD )) + +if (cpuset_do_slab_mem_spread () && (s->flags &SLAB_MEM_SPREAD )) {return cpuset_mem_spread_node (); -else if (current->mempolicy) -return slab_node (current->mempolicy); + }else { +if (current->mempolicy) +return slab_node (current->mempolicy); + } +return node; }
@@ -1365,8 +1377,8 @@ static noinline int alternate_nid(struct* * Must be called with interrupts disabled. */ -static noinline void *__remote_slab_alloc(struct kmem_cache *s, -gfp_t gfpflags,int node) +static noinline void * +__remote_slab_alloc (struct kmem_cache *s,gfp_t gfpflags,int node) {struct kmem_cache_node *n;struct kmem_cache_list *l;
@@ -1375,6 +1387,7 @@ static noinline void *__remote_slab_allo n = s->node[node];if (unlikely (!n))/* node has no memory */ return NULL ; + l = &n->list;spin_lock (&n->list_lock);
@@ -1389,7 +1402,9 @@ static noinline void *__remote_slab_allo }if (likely (object))slqb_stat_inc (l,ALLOC ); +spin_unlock (&n->list_lock); +return object; }#endif
@@ -1399,12 +1414,12 @@ static noinline void *__remote_slab_allo* * Must be called with interrupts disabled. */ -static __always_inline void *__slab_alloc(struct kmem_cache *s, -gfp_t gfpflags,int node) +static __always_inline void * +__slab_alloc (struct kmem_cache *s,gfp_t gfpflags,int node) { -void *object; -struct kmem_cache_cpu *c;struct kmem_cache_list *l; +struct kmem_cache_cpu *c; +void *object;#ifdef CONFIG_NUMA if (unlikely (node !=-1 ) &&unlikely (node !=numa_node_id ()))
@@ -1422,6 +1437,7 @@ static __always_inline void *__slab_allo }if (likely (object))slqb_stat_inc (l,ALLOC ); +return object; }
@@ -1429,11 +1445,11 @@ static __always_inline void *__slab_allo* Perform some interrupts-on processing around the main allocation path * (debug checking and memset()ing). */ -static __always_inline void *slab_alloc(struct kmem_cache *s, -gfp_t gfpflags,int node,void *addr) +static __always_inline void * +slab_alloc (struct kmem_cache *s,gfp_t gfpflags,int node,void *addr) { -void *object;unsigned long flags; +void *object; again:local_irq_save (flags);
@@ -1451,10 +1467,11 @@ again:return object; } -static __always_inline void *__kmem_cache_alloc(struct kmem_cache *s, -gfp_t gfpflags,void *caller) +static __always_inline void * +__kmem_cache_alloc (struct kmem_cache *s,gfp_t gfpflags,void *caller) {int node =-1 ; +#ifdef CONFIG_NUMA if (unlikely (current->flags & (PF_SPREAD_SLAB |PF_MEMPOLICY ))) node =alternate_nid (s, gfpflags, node);
@@ -1487,8 +1504,8 @@ EXPORT_SYMBOL(kmem_cache_alloc_node);* * Must be called with interrupts disabled. */ -static void flush_remote_free_cache (struct kmem_cache *s, -struct kmem_cache_cpu *c) +static void +flush_remote_free_cache (struct kmem_cache *s,struct kmem_cache_cpu *c) {struct kmlist *src;struct kmem_cache_list *dst;
@@ -1575,12 +1592,12 @@ static noinline void slab_free_to_remote* * Must be called with interrupts disabled. */ -static __always_inline void __slab_free(struct kmem_cache *s, -struct slqb_page *page,void *object) +static __always_inline void +__slab_free (struct kmem_cache *s,struct slqb_page *page,void *object) { -struct kmem_cache_cpu *c; -struct kmem_cache_list *l;int thiscpu =smp_processor_id (); +struct kmem_cache_list *l; +struct kmem_cache_cpu *c; c =get_cpu_slab (s, thiscpu); l = &c->list;
@@ -1619,8 +1636,8 @@ static __always_inline void __slab_free(* Perform some interrupts-on processing around the main freeing path * (debug checking). */ -static __always_inline void slab_free(struct kmem_cache *s, -struct slqb_page *page,void *object) +static __always_inline void +slab_free (struct kmem_cache *s,struct slqb_page *page,void *object) {unsigned long flags;
@@ -1683,7 +1700,7 @@ static int slab_order(int size, int max_return order; } -static int calculate_order (int size) +static int calc_order (int size) {int order;
@@ -1710,8 +1727,8 @@ static int calculate_order(int size)/* * Figure out what the alignment of the objects will be. */ -static unsigned long calculate_alignment (unsigned long flags, -unsigned long align,unsigned long size) +static unsigned long +calc_alignment (unsigned long flags,unsigned long align,unsigned long size) {/* * If the user wants hardware cache aligned objects then follow that
@@ -1737,18 +1754,18 @@ static unsigned long calculate_alignmentstatic void init_kmem_cache_list (struct kmem_cache *s,struct kmem_cache_list *l) { - l->cache = s; - l->freelist.nr =0 ; - l->freelist.head =NULL ; - l->freelist.tail =NULL ; - l->nr_partial =0 ; - l->nr_slabs =0 ; + l->cache = s; + l->freelist.nr =0 ; + l->freelist.head =NULL ; + l->freelist.tail =NULL ; + l->nr_partial =0 ; + l->nr_slabs =0 ;INIT_LIST_HEAD (&l->partial);#ifdef CONFIG_SMP - l->remote_free_check =0 ; + l->remote_free_check =0 ;spin_lock_init (&l->remote_free.lock); - l->remote_free.list.nr =0 ; + l->remote_free.list.nr =0 ; l->remote_free.list.head =NULL ; l->remote_free.list.tail =NULL ;#endif
@@ -1758,8 +1775,7 @@ static void init_kmem_cache_list(struct#endif } -static void init_kmem_cache_cpu (struct kmem_cache *s, -struct kmem_cache_cpu *c) +static void init_kmem_cache_cpu (struct kmem_cache *s,struct kmem_cache_cpu *c) {init_kmem_cache_list (s, &c->list);
@@ -1773,8 +1789,8 @@ static void init_kmem_cache_cpu(struct k }#ifdef CONFIG_NUMA -static void init_kmem_cache_node (struct kmem_cache *s, -struct kmem_cache_node *n) +static void +init_kmem_cache_node (struct kmem_cache *s,struct kmem_cache_node *n) {spin_lock_init (&n->list_lock);init_kmem_cache_list (s, &n->list);
@@ -1804,8 +1820,8 @@ static struct kmem_cache_node kmem_node_#endif #ifdef CONFIG_SMP -static struct kmem_cache_cpu *alloc_kmem_cache_cpu (struct kmem_cache *s, -int cpu) +static struct kmem_cache_cpu * +alloc_kmem_cache_cpu (struct kmem_cache *s,int cpu) {struct kmem_cache_cpu *c;
@@ -1910,7 +1926,7 @@ static int alloc_kmem_cache_nodes(struct#endif /* -* calculate_sizes() determines the order and the distribution of data within +* calc_sizes() determines the order and the distribution of data within * a slab object. */ static int calculate_sizes (struct kmem_cache *s)
@@ -1991,7 +2007,7 @@ static int calculate_sizes(struct kmem_c* user specified and the dynamic determination of cache line size * on bootup. */ - align =calculate_alignment (flags, align, s->objsize); + align =calc_alignment (flags, align, s->objsize);/* * SLQB stores one object immediately after another beginning from
@@ -2000,7 +2016,7 @@ static int calculate_sizes(struct kmem_c*/ size =ALIGN (size, align); s->size = size; - s->order =calculate_order (size); + s->order =calc_order (size);if (s->order <0 )return 0 ;
@@ -2210,38 +2226,38 @@ static struct kmem_cache *open_kmalloc_c* fls. */ static s8 size_index[24] __cacheline_aligned = { -3 ,/* 8 */ -4 ,/* 16 */ -5 ,/* 24 */ -5 ,/* 32 */ -6 ,/* 40 */ -6 ,/* 48 */ -6 ,/* 56 */ -6 ,/* 64 */ +3 ,/* 8 */ +4 ,/* 16 */ +5 ,/* 24 */ +5 ,/* 32 */ +6 ,/* 40 */ +6 ,/* 48 */ +6 ,/* 56 */ +6 ,/* 64 */ #if L1_CACHE_BYTES < 64 - 1, /* 72 */ -1 ,/* 80 */ -1 ,/* 88 */ -1 ,/* 96 */ + 1, /* 72 */ +1 ,/* 80 */ +1 ,/* 88 */ +1 ,/* 96 */ #else -7 , -7 , -7 , -7 , -#endif -7 ,/* 104 */ -7 ,/* 112 */ -7 ,/* 120 */ -7 ,/* 128 */ +7 , +7 , +7 , +7 , +#endif +7 ,/* 104 */ +7 ,/* 112 */ +7 ,/* 120 */ +7 ,/* 128 */ #if L1_CACHE_BYTES < 128 - 2, /* 136 */ -2 ,/* 144 */ -2 ,/* 152 */ -2 ,/* 160 */ -2 ,/* 168 */ -2 ,/* 176 */ -2 ,/* 184 */ -2 /* 192 */ + 2, /* 136 */ +2 ,/* 144 */ +2 ,/* 152 */ +2 ,/* 160 */ +2 ,/* 168 */ +2 ,/* 176 */ +2 ,/* 184 */ +2 /* 192 */ #else -1, -1,
@@ -2278,9 +2294,8 @@ static struct kmem_cache *get_slab(size_void *__kmalloc (size_t size,gfp_t flags) { -struct kmem_cache *s; +struct kmem_cache *s =get_slab (size, flags); - s =get_slab (size, flags);if (unlikely (ZERO_OR_NULL_PTR (s)))return s;
@@ -2291,9 +2306,8 @@ EXPORT_SYMBOL(__kmalloc);#ifdef CONFIG_NUMA void *__kmalloc_node (size_t size,gfp_t flags,int node) { -struct kmem_cache *s; +struct kmem_cache *s =get_slab (size, flags); - s =get_slab (size, flags);if (unlikely (ZERO_OR_NULL_PTR (s)))return s;
@@ -2340,8 +2354,8 @@ EXPORT_SYMBOL(ksize);void kfree (const void *object) { -struct kmem_cache *s;struct slqb_page *page; +struct kmem_cache *s;if (unlikely (ZERO_OR_NULL_PTR (object)))return ;
@@ -2371,21 +2385,21 @@ static void kmem_cache_trim_percpu(voidint kmem_cache_shrink (struct kmem_cache *s) { -#ifdef CONFIG_NUMA -int node; -#endif -on_each_cpu (kmem_cache_trim_percpu, s,1 );#ifdef CONFIG_NUMA -for_each_node_state (node,N_NORMAL_MEMORY ) { -struct kmem_cache_node *n = s->node[node]; -struct kmem_cache_list *l = &n->list; + { +int node; -spin_lock_irq (&n->list_lock); -claim_remote_free_list (s, l); -flush_free_list (s, l); -spin_unlock_irq (&n->list_lock); +for_each_node_state (node,N_NORMAL_MEMORY ) { +struct kmem_cache_node *n = s->node[node]; +struct kmem_cache_list *l = &n->list; + +spin_lock_irq (&n->list_lock); +claim_remote_free_list (s, l); +flush_free_list (s, l); +spin_unlock_irq (&n->list_lock); + } }#endif
@@ -2397,8 +2411,8 @@ EXPORT_SYMBOL(kmem_cache_shrink);static void kmem_cache_reap_percpu (void *arg) {int cpu =smp_processor_id (); -struct kmem_cache *s;long phase = (long )arg; +struct kmem_cache *s;list_for_each_entry (s, &slab_caches, list) {struct kmem_cache_cpu *c =get_cpu_slab (s, cpu);
@@ -2442,8 +2456,7 @@ static void kmem_cache_reap(void)static void cache_trim_worker (struct work_struct *w) { -struct delayed_work *work = - container_of(w, struct delayed_work, work); +struct delayed_work *work;struct kmem_cache *s;int node;
@@ -2469,6 +2482,7 @@ static void cache_trim_worker(struct worup_read (&slqb_lock); out: + work = container_of(w, struct delayed_work, work);schedule_delayed_work (work,round_jiffies_relative (3 *HZ )); }
@@ -2587,8 +2601,8 @@ static int slab_memory_callback(struct nvoid __init kmem_cache_init (void ) { -int i;unsigned int flags =SLAB_HWCACHE_ALIGN |SLAB_PANIC ; +int i;/* * All the ifdefs are rather ugly here, but it's just the setup code,
@@ -2719,8 +2733,9 @@ void __init kmem_cache_init(void)/* * Some basic slab creation sanity checks */ -static int kmem_cache_create_ok (const char *name,size_t size, -size_t align,unsigned long flags) +static int +kmem_cache_create_ok (const char *name,size_t size, +size_t align,unsigned long flags) {struct kmem_cache *tmp;
@@ -2773,8 +2788,9 @@ static int kmem_cache_create_ok(const chreturn 1 ; } -struct kmem_cache *kmem_cache_create (const char *name,size_t size, -size_t align,unsigned long flags,void (*ctor)(void *)) +struct kmem_cache * +kmem_cache_create (const char *name,size_t size, +size_t align,unsigned long flags,void (*ctor)(void *)) {struct kmem_cache *s;
@@ -2804,7 +2820,7 @@ EXPORT_SYMBOL(kmem_cache_create);* necessary. */ static int __cpuinit slab_cpuup_callback (struct notifier_block *nfb, -unsigned long action,void *hcpu) +unsigned long action,void *hcpu) {long cpu = (long )hcpu;struct kmem_cache *s;
@@ -2855,7 +2871,7 @@ static int __cpuinit slab_cpuup_callback }static struct notifier_block __cpuinitdata slab_notifier = { - .notifier_call = slab_cpuup_callback + .notifier_call = slab_cpuup_callback };#endif
@@ -2878,11 +2894,10 @@ void *__kmalloc_track_caller(size_t size }void *__kmalloc_node_track_caller (size_t size,gfp_t flags,int node, -unsigned long caller) +unsigned long caller) { -struct kmem_cache *s; +struct kmem_cache *s =get_slab (size, flags); - s =get_slab (size, flags);if (unlikely (ZERO_OR_NULL_PTR (s)))return s;
@@ -2892,12 +2907,17 @@ void *__kmalloc_node_track_caller(size_t #if defined(CONFIG_SLQB_SYSFS) || defined(CONFIG_SLABINFO) struct stats_gather { - struct kmem_cache *s; - spinlock_t lock; - unsigned long nr_slabs; - unsigned long nr_partial; - unsigned long nr_inuse; - unsigned long nr_objects; + /* + * Serialize on_each_cpu() instances updating the summary + * stats structure: + */ + spinlock_t lock; + + struct kmem_cache *s; + unsigned long nr_slabs; + unsigned long nr_partial; + unsigned long nr_inuse; + unsigned long nr_objects; #ifdef CONFIG_SLQB_STATS unsigned long stats[NR_SLQB_STAT_ITEMS];
@@ -2915,25 +2935,25 @@ static void __gather_stats(void *arg)struct kmem_cache_cpu *c =get_cpu_slab (s, cpu);struct kmem_cache_list *l = &c->list;struct slqb_page *page; -#ifdef CONFIG_SLQB_STATS -int i; -#endif nr_slabs = l->nr_slabs; nr_partial = l->nr_partial; nr_inuse = (nr_slabs - nr_partial) * s->objects; -list_for_each_entry (page, &l->partial, lru) { +list_for_each_entry (page, &l->partial, lru) nr_inuse += page->inuse; - }spin_lock (&gather->lock); gather->nr_slabs += nr_slabs; gather->nr_partial += nr_partial; gather->nr_inuse += nr_inuse;#ifdef CONFIG_SLQB_STATS -for (i =0 ; i <NR_SLQB_STAT_ITEMS ; i++) - gather->stats[i] += l->stats[i]; + { +int i; + +for (i =0 ; i <NR_SLQB_STAT_ITEMS ; i++) + gather->stats[i] += l->stats[i]; + }#endif spin_unlock (&gather->lock); }
@@ -2956,14 +2976,15 @@ static void gather_stats(struct kmem_cacstruct kmem_cache_list *l = &n->list;struct slqb_page *page;unsigned long flags; -#ifdef CONFIG_SLQB_STATS -int i; -#endif spin_lock_irqsave (&n->list_lock, flags);#ifdef CONFIG_SLQB_STATS -for (i =0 ; i <NR_SLQB_STAT_ITEMS ; i++) - stats->stats[i] += l->stats[i]; + { +int i; + +for (i =0 ; i <NR_SLQB_STAT_ITEMS ; i++) + stats->stats[i] += l->stats[i]; + }#endif stats->nr_slabs += l->nr_slabs; stats->nr_partial += l->nr_partial;
@@ -3039,14 +3060,15 @@ static int s_show(struct seq_file *m, voseq_printf (m," : slabdata %6lu %6lu %6lu" , stats.nr_slabs, stats.nr_slabs,0UL );seq_putc (m,'\n' ); +return 0 ; }static const struct seq_operations slabinfo_op = { - .start = s_start, - .next = s_next, - .stop = s_stop, - .show = s_show, + .start = s_start, + .next = s_next, + .stop = s_stop, + .show = s_show, };static int slabinfo_open (struct inode *inode,struct file *file)
@@ -3205,8 +3227,8 @@ static ssize_t store_user_show(struct km }SLAB_ATTR_RO (store_user); -static ssize_t hiwater_store (struct kmem_cache *s, -const char *buf,size_t length) +static ssize_t +hiwater_store (struct kmem_cache *s,const char *buf,size_t length) {long hiwater;int err;
@@ -3229,8 +3251,8 @@ static ssize_t hiwater_show(struct kmem_ }SLAB_ATTR (hiwater); -static ssize_t freebatch_store (struct kmem_cache *s, -const char *buf,size_t length) +static ssize_t +freebatch_store (struct kmem_cache *s,const char *buf,size_t length) {long freebatch;int err;
@@ -3258,91 +3280,95 @@ static int show_stat(struct kmem_cache * {struct stats_gather stats;int len; -#ifdef CONFIG_SMP -int cpu; -#endif gather_stats (s, &stats); len =sprintf (buf,"%lu" , stats.stats[si]);#ifdef CONFIG_SMP -for_each_online_cpu (cpu) { -struct kmem_cache_cpu *c =get_cpu_slab (s, cpu); -struct kmem_cache_list *l = &c->list; + { +int cpu; -if (len <PAGE_SIZE -20 ) - len +=sprintf (buf+len," C%d=%lu" , cpu, l->stats[si]); +for_each_online_cpu (cpu) { +struct kmem_cache_cpu *c =get_cpu_slab (s, cpu); +struct kmem_cache_list *l = &c->list; + +if (len <PAGE_SIZE -20 ) { + len +=sprintf (buf+len, +" C%d=%lu" , cpu, l->stats[si]); + } + } }#endif return len +sprintf (buf + len,"\n" ); } -#define STAT_ATTR (si, text) \ +#define STAT_ATTR (si, text) \ static ssize_t text##_show(struct kmem_cache *s, char *buf) \ { \ return show_stat(s, buf, si); \ } \ SLAB_ATTR_RO(text); \ -STAT_ATTR (ALLOC , alloc); -STAT_ATTR (ALLOC_SLAB_FILL , alloc_slab_fill); -STAT_ATTR (ALLOC_SLAB_NEW , alloc_slab_new); -STAT_ATTR (FREE , free); -STAT_ATTR (FREE_REMOTE , free_remote); -STAT_ATTR (FLUSH_FREE_LIST , flush_free_list); -STAT_ATTR (FLUSH_FREE_LIST_OBJECTS , flush_free_list_objects); -STAT_ATTR (FLUSH_FREE_LIST_REMOTE , flush_free_list_remote); -STAT_ATTR (FLUSH_SLAB_PARTIAL , flush_slab_partial); -STAT_ATTR (FLUSH_SLAB_FREE , flush_slab_free); -STAT_ATTR (FLUSH_RFREE_LIST , flush_rfree_list); -STAT_ATTR (FLUSH_RFREE_LIST_OBJECTS , flush_rfree_list_objects); -STAT_ATTR (CLAIM_REMOTE_LIST , claim_remote_list); -STAT_ATTR (CLAIM_REMOTE_LIST_OBJECTS , claim_remote_list_objects); +STAT_ATTR (ALLOC , alloc); +STAT_ATTR (ALLOC_SLAB_FILL , alloc_slab_fill); +STAT_ATTR (ALLOC_SLAB_NEW , alloc_slab_new); +STAT_ATTR (FREE , free); +STAT_ATTR (FREE_REMOTE , free_remote); +STAT_ATTR (FLUSH_FREE_LIST , flush_free_list); +STAT_ATTR (FLUSH_FREE_LIST_OBJECTS , flush_free_list_objects); +STAT_ATTR (FLUSH_FREE_LIST_REMOTE , flush_free_list_remote); +STAT_ATTR (FLUSH_SLAB_PARTIAL , flush_slab_partial); +STAT_ATTR (FLUSH_SLAB_FREE , flush_slab_free); +STAT_ATTR (FLUSH_RFREE_LIST , flush_rfree_list); +STAT_ATTR (FLUSH_RFREE_LIST_OBJECTS , flush_rfree_list_objects); +STAT_ATTR (CLAIM_REMOTE_LIST , claim_remote_list); +STAT_ATTR (CLAIM_REMOTE_LIST_OBJECTS , claim_remote_list_objects);#endif static struct attribute *slab_attrs[] = { - &slab_size_attr.attr, - &object_size_attr.attr, - &objs_per_slab_attr.attr, - &order_attr.attr, - &objects_attr.attr, - &total_objects_attr.attr, - &slabs_attr.attr, - &ctor_attr.attr, - &align_attr.attr, - &hwcache_align_attr.attr, - &reclaim_account_attr.attr, - &destroy_by_rcu_attr.attr, - &red_zone_attr.attr, - &poison_attr.attr, - &store_user_attr.attr, - &hiwater_attr.attr, - &freebatch_attr.attr, + + & slab_size_attr.attr, + & object_size_attr.attr, + & objs_per_slab_attr.attr, + & order_attr.attr, + & objects_attr.attr, + & total_objects_attr.attr, + & slabs_attr.attr, + & ctor_attr.attr, + & align_attr.attr, + & hwcache_align_attr.attr, + & reclaim_account_attr.attr, + & destroy_by_rcu_attr.attr, + & red_zone_attr.attr, + & poison_attr.attr, + & store_user_attr.attr, + & hiwater_attr.attr, + & freebatch_attr.attr, #ifdef CONFIG_ZONE_DMA - &cache_dma_attr.attr, + & cache_dma_attr.attr, #endif #ifdef CONFIG_SLQB_STATS - &alloc_attr.attr, - &alloc_slab_fill_attr.attr, - &alloc_slab_new_attr.attr, - &free_attr.attr, - &free_remote_attr.attr, - &flush_free_list_attr.attr, - &flush_free_list_objects_attr.attr, - &flush_free_list_remote_attr.attr, - &flush_slab_partial_attr.attr, - &flush_slab_free_attr.attr, - &flush_rfree_list_attr.attr, - &flush_rfree_list_objects_attr.attr, - &claim_remote_list_attr.attr, - &claim_remote_list_objects_attr.attr, + & alloc_attr.attr, + & alloc_slab_fill_attr.attr, + & alloc_slab_new_attr.attr, + & free_attr.attr, + & free_remote_attr.attr, + & flush_free_list_attr.attr, + & flush_free_list_objects_attr.attr, + & flush_free_list_remote_attr.attr, + & flush_slab_partial_attr.attr, + & flush_slab_free_attr.attr, + & flush_rfree_list_attr.attr, + & flush_rfree_list_objects_attr.attr, + & claim_remote_list_attr.attr, + & claim_remote_list_objects_attr.attr, #endifNULL };static struct attribute_group slab_attr_group = { - .attrs = slab_attrs, + .attrs = slab_attrs, }; static ssize_t slab_attr_show(struct kobject *kobj,
@@ -3389,13 +3415,13 @@ static void kmem_cache_release(struct ko }static struct sysfs_ops slab_sysfs_ops = { - .show = slab_attr_show, - .store = slab_attr_store, + .show = slab_attr_show, + .store = slab_attr_store, };static struct kobj_type slab_ktype = { - .sysfs_ops = &slab_sysfs_ops, - .release = kmem_cache_release + .sysfs_ops = &slab_sysfs_ops, + .release = kmem_cache_release };static int uevent_filter (struct kset *kset,struct kobject *kobj)
@@ -3413,7 +3439,7 @@ static struct kset_uevent_ops slab_uevenstatic struct kset *slab_kset; -static int sysfs_available__read_mostly =0 ; +static int sysfs_available__read_mostly ;static int sysfs_slab_add (struct kmem_cache *s) {
@@ -3462,9 +3488,11 @@ static int __init slab_sysfs_init(void)list_for_each_entry (s, &slab_caches, list) { err =sysfs_slab_add (s); -if (err) -printk (KERN_ERR "SLQB: Unable to add boot slab %s" -" to sysfs\n" , s->name); +if (!err) +continue ; + +printk (KERN_ERR +"SLQB: Unable to add boot slab %s to sysfs\n" , s->name); }up_write (&slqb_lock);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Ingo Molnar
Quoted 5 lines · Nick Piggin
Quoted 3 lines · Ingo Molnar
Mind if i nitpick a bit about minor style issues? Since this is going to be the next Linux SLAB allocator we might as well do it perfectly :-)
Quoted 6 lines
Quoted 3 lines
Well here is an incremental patch which should get most of the issues you pointed out, most of the sane ones that checkpatch pointed out, and a few of my own ;)
here's an incremental one ontop of your incremental patch, enhancing some more issues. I now find the code very readable! :-)
Thanks! I'll go through it and apply it. I'll raise any issues if I am particularly against them ;)
Quoted 3 lines
( in case you are wondering about the placement of
bit_spinlock.h- that
file needs fixing, just move it to the top of the file and see the build break. But that's a separate patch.)
Ah, SLQB doesn't use bit spinlocks anyway, so I'll just get rid of that. I'll see if there are any other obviously unneeded headers too.
Thanks,
Nick
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
mm/slqb.c static inline int slab_freebatch(struct · line 126
Quoted 5 lines · Ingo Molnar
mm/slqb.c +2
-2-static inline void slqb_stat_inc (struct kmem_cache_list *list, -enum stat_item si) +static inline void +slqb_stat_inc (struct kmem_cache_list *list,enum stat_item si) {
Hmm, I'm not entirely fond of this style. The former scales to longer lines with just a single style change (putting args into new lines), wheras the latter first moves its prefixes to a newline, then moves args as the line grows even longer.
I guess it is a matter of taste, not wrong either way... but I think most of the mm code I'm used to looking at uses the former. Do you feel strongly?
mm/slqb.c static int check_slab(struct kmem_cache · line 725
Quoted 11 lines
mm/slqb.c +4
-6+static void +trace (struct kmem_cache *s,struct slqb_page *page,void *object,int alloc) { -if (s->flags &SLAB_TRACE ) { -printk (KERN_INFO "TRACE %s %s 0x%p inuse=%d fp=0x%p\n" , - s->name, - alloc ?"alloc" :"free" , - object, page->inuse, - page->freelist); +if (likely (!(s->flags &SLAB_TRACE ))) +return ;
I think most of your flow control changes are improvements (others even more than this, but this is the first one so I comment here). Thanks.
mm/slqb.c static noinline void *__remote_slab_allo · line 1410
Quoted 10 lines
mm/slqb.c +2
-0@@ -1389,7 +1402,9 @@ static noinline void *__remote_slab_allo }if (likely (object))slqb_stat_inc (l,ALLOC ); +spin_unlock (&n->list_lock); +return object; }#endif
Whitespace, I never really know if I'm "doing it right" or not :) And often it is easy to tell a badly wrong one, but harder to tell what is better between two reasonable ones. But I guess I'm the same way with paragraphs in my writing...
Quoted 4 lines
mm/slqb.c +0
-0@@ -1399,12 +1414,12 @@ static noinline void *__remote_slab_allo* * Must be called with interrupts disabled. */
mm/slqb.c static noinline void *__remote_slab_allo · line 1422
Quoted 10 lines
mm/slqb.c +4
-4-static __always_inline void *__slab_alloc(struct kmem_cache *s, -gfp_t gfpflags,int node) +static __always_inline void * +__slab_alloc (struct kmem_cache *s,gfp_t gfpflags,int node) { -void *object; -struct kmem_cache_cpu *c;struct kmem_cache_list *l; +struct kmem_cache_cpu *c; +void *object;
Same with order of local variables. You like longest lines to shortest I know. I think I vaguely try to arrange them from the most important or high level "actor" to the least, and then in order of when they get discovered/used.
For example, in the above function, "object" is the raison d'etre. kmem_cache_cpu is found first, and from that, kmem_cache_list is found. Which slightly explains the order.
mm/slqb.c static __always_inline void *__slab_allo · line 1452
Quoted 6 lines
mm/slqb.c +3
-1+static __always_inline void * +slab_alloc (struct kmem_cache *s,gfp_t gfpflags,int node,void *addr) { -void *object;unsigned long flags; +void *object;
And here, eg. flags comes last because mostly inconsequential to the bigger picture.
Your method is easier though, I'll grant you that :)
Quoted 6 lines
static void init_kmem_cache_list(struct
kmem_cache*s,mm/slqb.c +0
-3struct kmem_cache_list *l) { - l->cache = s; - l->freelist.nr =0 ; - l->freelist.head =NULL ;
mm/slqb.c static unsigned long calculate_alignment · line 1763
Quoted 10 lines
mm/slqb.c +6
-3- l->freelist.tail =NULL ; - l->nr_partial =0 ; - l->nr_slabs =0 ; + l->cache = s; + l->freelist.nr =0 ; + l->freelist.head =NULL ; + l->freelist.tail =NULL ; + l->nr_partial =0 ; + l->nr_slabs =0 ;INIT_LIST_HEAD (&l->partial);
Hmm, we seem to have gathered an extra space...
mm/slqb.c static unsigned long calculate_alignment · line 1771
Quoted 10 lines
#ifdef
CONFIG_SMPmm/slqb.c +2
-2- l->remote_free_check =0 ; + l->remote_free_check =0 ;spin_lock_init (&l->remote_free.lock); - l->remote_free.list.nr =0 ; + l->remote_free.list.nr =0 ; l->remote_free.list.head =NULL ; l->remote_free.list.tail =NULL ;#endif
... ah, to line up with this guy. TBH, I prefer not to religiously line things up like this. If there is the odd long-line, just give it the normal single space. I find it just keeps it easier to maintain. Although you might counter that of course it is easier to keep something clean if one relaxes their definition of "clean".
Quoted 7 lines
static s8 size_index[24]
__cacheline_aligned= {mm/slqb.c +0
-6-3 ,/* 8 */ -4 ,/* 16 */ -5 ,/* 24 */ -5 ,/* 32 */ -6 ,/* 40 */ - 6, /* 48 */
mm/slqb.c static struct kmem_cache *open_kmalloc_c · line 2236
Quoted 10 lines
mm/slqb.c +8
-2-6 ,/* 56 */ - 6, /* 64 */ +3 ,/* 8 */ +4 ,/* 16 */ +5 ,/* 24 */ +5 ,/* 32 */ +6 ,/* 40 */ +6 ,/* 48 */ +6 ,/* 56 */ + 6, /* 64 */
However justifying numbers, like this, I'm happy to do (may as well align the numbers in the comments too while we're here).
mm/slqb.c static struct kmem_cache *get_slab(size_ · line 2300
Quoted 10 lines
mm/slqb.c +1
-2@@ -2278,9 +2294,8 @@ static struct kmem_cache *get_slab(size_void *__kmalloc (size_t size,gfp_t flags) { -struct kmem_cache *s; +struct kmem_cache *s =get_slab (size, flags); - s =get_slab (size, flags);if (unlikely (ZERO_OR_NULL_PTR (s)))return s;
I've got yet the same problem with these... I mostly try to avoid doing this, although there are some cases where it works well (eg. constants, or a simple assignment of an argument to a local).
At some point, you start putting real code in there, at which point the space after the local vars doesn't seem to serve much purpose. get_slab I feel logically belongs close to the subsequent check, because that's basically sanitizing its return value / extracting the error case from it and leaving the rest of the function to work on the common case.
mm/slqb.c static struct kset_uevent_ops slab_ueven · line 3442
Quoted 2 lines
mm/slqb.c +1
-1-static int sysfs_available__read_mostly =0 ; +static int sysfs_available__read_mostly ;
These, I actually like initializing to zero explicitly. I'm pretty sure gcc no longer makes it any more expensive than leaving out. Yes of course everybody who knows C has to know this, but.... I just don't feel much harm in leaving it.
Lots of good stuff, lots I'm on the fence with, some I dislike ;) I'll concentrate on picking up the obvious ones, and get the bugs fixed. Will see where the discussion goes with the rest.
Thanks,
Nick
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Nick Piggin
mm/slqb.c static inline int slab_freebatch(struct · line 126
Quoted 5 lines · Ingo Molnar
mm/slqb.c +2
-2-static inline void slqb_stat_inc (struct kmem_cache_list *list, -enum stat_item si) +static inline void +slqb_stat_inc (struct kmem_cache_list *list,enum stat_item si) {Hmm, I'm not entirely fond of this style. [...]
well, it's a borderline situation and a nuance, and i think we agree on the two (much more common) boundary conditions:
1) line fits into 80 cols - in that case we keep it all on a single line (this is the ideal case)
2) line does not fit on two lines either - in that case we do the style that you used above.
On the boundary there's a special case though, and i tend to prefer:
+static inline void
+slqb_stat_inc(struct kmem_cache_list *list, enum stat_item si)
over:
-static inline void slqb_stat_inc(struct kmem_cache_list *list,
- enum stat_item si)
for two reasons:
1) the line break is not just arbitrarily in the middle of the enumeration of arguments - it is right after function return type.
2) the arguments fit on a single line - and often one wants to know that signature. (return values are usually a separate thought)
3) the return type stands out much better.
But again ... this is a nuance.
Quoted 3 lines
[...] The former scales to longer lines with just a single style change (putting args into new lines), wheras the latter first moves its prefixes to a newline, then moves args as the line grows even longer.
the moment this 'boundary style' "overflows", it falls back to the 'lots of lines' case, where we generally put the function return type and the function name on the first line.
Quoted 3 lines
I guess it is a matter of taste, not wrong either way... but I think most of the mm code I'm used to looking at uses the former. Do you feel strongly?
there are a handful of cases where the return type (and the function attributes) are _really_ long - in this case it really helps to have them decoupled from the arguments.
Ingo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
One thing you might consider is that
Q is visually close enough to O to be
misread.
Perhaps a different letter would be good.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 5 lines · Joe Perches
One thing you might consider is that
Q is visually close enough to O to be
misread.Perhaps a different letter would be good.
That's a fair point. Hugh dislikes it too, I see ;) What to do... I had been toying with the idea that if slqb (or slub) becomes "the" allocator, then we could rename it all back to slAb after replacing the existing slab?
Or I could make it a 128 bit allocator and call it SLZB, which would definitely make it "the final" allocator ;)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 4 lines · Nick Piggin
That's a fair point. Hugh dislikes it too, I see ;) What to do... I had been toying with the idea that if slqb (or slub) becomes "the" allocator, then we could rename it all back to slAb after replacing the existing slab?
maybe SLIB (slab-improved) or SLAB_NG or NSLAB or SLABX Who says it has to be 4 letters?
Quoted 2 lines
Or I could make it a 128 bit allocator and call it SLZB, which would definitely make it "the final" allocator ;)
That leads to the phone book game.
SLZZB - and a crystal bridge now spans the fissure.
Hmm, wrong game.
cheers, j
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 5 lines · Nick Piggin
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with), and improved performance and reduced locking somewhat for node-specific and interleaved allocations.
I haven't reviewed your postings, but I did give the previous version of your patch a try on all my machines. Some observations and one patch.
I was initially _very_ impressed by how well it did on my venerable tmpfs loop swapping loads, where I'd expected next to no effect; but that turned out to be because on three machines I'd been using SLUB, without remembering how default slub_max_order got raised from 1 to 3 in 2.6.26 (hmm, and Documentation/vm/slub.txt not updated).
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
So in fact SLQB comes in very much like SLAB, as I think you'd expect:
slightly ahead of it on most of the machines, but probably in the noise. (SLOB behaves decently: not a winner, but no catastrophic behaviour.)
What I love most about SLUB is the way you can reasonably build with CONFIG_SLUB_DEBUG=y, very little impact, then switch on the specific debugging you want with a boot option when you want it. That was a great stride forward, which you've followed in SLQB: so I'd have to prefer SLQB to SLAB (on debuggability) and to SLUB (on high orders).
I do hate the name SLQB. Despite having no experience of databases, I find it almost impossible to type, coming out as SQLB most times. Wish you'd invented a plausible vowel instead of the Q; but probably too late for that.
init/Kconfig describes it as "Qeued allocator": should say "Queued".
Documentation/vm/slqbinfo.c gives several compilation warnings:
I'd rather leave it to you to fix them, maybe the unused variables are about to be used, or maybe there's much worse wrong with it than a few compilation warnings, I didn't investigate.
The only bug I found (but you'll probably want to change the patch
- which I've rediffed to today's slqb.c, but not retested).
On fake NUMA I hit kernel BUG at mm/slqb.c:1107! claim_remote_free_list() is doing several things without remote_free.lock: that VM_BUG_ON is unsafe for one, and even if others are somehow safe today, it will be more robust to take the lock sooner.
I moved the prefetchw(head) down to where we know it's going to be the head, and replaced the offending VM_BUG_ON by a later WARN_ON which you'd probably better remove altogether: once we got the lock, it's hardly interesting.
---
mm/slqb.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
mm/slqb.c renamed from mm/slqb.c.orig +9 -8
@@ -1115,17 +1115,12 @@ static void claim_remote_free_list(strucvoid **head, **tail;int nr; -VM_BUG_ON (!l->remote_free.list.head != !l->remote_free.list.tail); -if (!l->remote_free.list.nr)return ; +spin_lock (&l->remote_free.lock); l->remote_free_check =0 ; head = l->remote_free.list.head; -/* Get the head hot for the likely subsequent allocation or flush */ -prefetchw (head); - -spin_lock (&l->remote_free.lock); l->remote_free.list.head =NULL ; tail = l->remote_free.list.tail; l->remote_free.list.tail =NULL ;
@@ -1133,9 +1128,15 @@ static void claim_remote_free_list(struc l->remote_free.list.nr =0 ;spin_unlock (&l->remote_free.lock); -if (!l->freelist.nr) +WARN_ON (!head + !tail != !nr + !nr); +if (!nr) +return ; + +if (!l->freelist.nr) { +/* Get head hot for likely subsequent allocation or flush */ +prefetchw (head); l->freelist.head = head; -else + }else set_freepointer (s, l->freelist.tail, head); l->freelist.tail = tail;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi Hugh,
Quoted 6 lines · Hugh Dickins
I was initially _very_ impressed by how well it did on my venerable tmpfs loop swapping loads, where I'd expected next to no effect; but that turned out to be because on three machines I'd been using SLUB, without remembering how default
slub_max_ordergot raised from 1 to 3 in 2.6.26 (hmm, andDocumentation/vm/slub.txtnot updated).
Quoted 9 lines
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit 9b2cd506e5f2117f94c28a0040bf5da058105316 ("slub: Calculate min_objects based on number of processors").
Pekka
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 17 lines · Pekka Enberg
Quoted 6 lines · Hugh Dickins
I was initially _very_ impressed by how well it did on my venerable tmpfs loop swapping loads, where I'd expected next to no effect; but that turned out to be because on three machines I'd been using SLUB, without remembering how default
slub_max_ordergot raised from 1 to 3 in 2.6.26 (hmm, andDocumentation/vm/slub.txtnot updated).Quoted 9 lines
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
Quoted 3 lines
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit
9b2cd506e5f2117f94c28a0040bf5da058105316("slub: Calculatemin_objectsbased on number of processors").
Thanks, yes, that could well account for the residual difference: the machines in question have 2 or 4 cpus, so the old slub_min_objects=4 has effectively become slub_min_objects=12 or slub_min_objects=16.
I'm now trying with slub_max_order=1 slub_min_objects=4 on the boot lines (though I'll need to curtail tests on a couple of machines), and will report back later.
It's great that SLUB provides these knobs; not so great that it needs them.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 17 lines · Hugh Dickins
Quoted 12 lines · Pekka Enberg
Quoted 10 lines · Hugh Dickins
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
Quoted 3 lines
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit
9b2cd506e5f2117f94c28a0040bf5da058105316("slub: Calculatemin_objectsbased on number of processors").
Quoted 7 lines
Thanks, yes, that could well account for the residual difference: the machines in question have 2 or 4 cpus, so the old slub_min_objects=4 has effectively become slub_min_objects=12 or slub_min_objects=16.
I'm now trying with slub_max_order=1 slub_min_objects=4 on the boot lines (though I'll need to curtail tests on a couple of machines), and will report back later.
Yes, slub_max_order=1 with slub_min_objects=4 certainly helps this swapping load. I've not tried slub_max_order=0, but I'm running with 8kB stacks, so order 1 seems a reasonable choice.
I can't say where I pulled that "e.g. 2% slower" from: on different machines slub was 5% or 10% or 20% slower than slab and slqb even with slub_max_order=1 (but not significantly slower on the "immune" machine). How much slub_min_objects=4 helps again varies widely, between halving or eliminating the difference.
But I think it's more important that I focus on the worst case machine, try to understand what's going on there.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Pekka Enberg
Tested-by Zhang, YanminHi Hugh,
Quoted 9 lines · Hugh Dickins
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
Quoted 3 lines · Pekka Enberg
Quoted 3 lines
Quoted 3 lines
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit
9b2cd506e5f2117f94c28a0040bf5da058105316("slub: Calculatemin_objectsbased on number of processors").
Quoted 7 lines · Hugh Dickins
Quoted 7 lines
Thanks, yes, that could well account for the residual difference: the machines in question have 2 or 4 cpus, so the old slub_min_objects=4 has effectively become slub_min_objects=12 or slub_min_objects=16.
I'm now trying with slub_max_order=1 slub_min_objects=4 on the boot lines (though I'll need to curtail tests on a couple of machines), and will report back later.
Quoted 3 lines · Hugh Dickins
Yes, slub_max_order=1 with slub_min_objects=4 certainly helps this swapping load. I've not tried slub_max_order=0, but I'm running with 8kB stacks, so order 1 seems a reasonable choice.
Yanmin/Christoph, maybe we should revisit the min objects logic calculate_order()?
Quoted 8 lines · Hugh Dickins
I can't say where I pulled that "e.g. 2% slower" from: on different machines slub was 5% or 10% or 20% slower than slab and slqb even with slub_max_order=1 (but not significantly slower on the "immune" machine). How much slub_min_objects=4 helps again varies widely, between halving or eliminating the difference.
But I think it's more important that I focus on the worst case machine, try to understand what's going on there.
Yeah. Oprofile and CONFIG_SLUB_STATS are usually quite helpful. You might want to test the included patch which targets one known SLAB vs. SLUB regression discovered quite recently.
Pekka
Subject: [PATCH] SLUB: revert direct page allocator pass through
From: Pekka Enberg <[email protected]>
This patch reverts page allocator pass-through logic from the SLUB allocator.
Commit aadb4bc4a1f9108c1d0fbd121827c936c2ed4217 ("SLUB: direct pass through of page size or higher kmalloc requests") added page allocator pass-through to the SLUB allocator for large sized allocations. This, however, results in a performance regression compared to SLAB in the netperf UDP-U-4k test.
The regression comes from the kfree(skb->head) call in skb_release_data() that is subject to page allocator pass-through as the size passed to __alloc_skb() is larger than 4 KB in this test. With this patch, the performance regression is almost closed:
<insert numbers here>
---
include/linux/slub_def.h +21 -13
@@ -124,7 +124,7 @@ struct kmem_cache {* We keep the general caches in an array of slab caches that are used for * 2^x bytes of allocations. */ -extern struct kmem_cache kmalloc_caches[PAGE_SHIFT +1 ]; +extern struct kmem_cache kmalloc_caches[KMALLOC_SHIFT_HIGH +1 ];/* * Sorry that the following has to be that ugly but some versions of GCC
@@ -135,6 +135,9 @@ static __always_inline int kmalloc_index(size_t size)if (!size)return 0 ; +if (size >KMALLOC_MAX_SIZE ) +return -1 ; +if (size <=KMALLOC_MIN_SIZE )return KMALLOC_SHIFT_LOW ;
@@ -154,10 +157,6 @@ static __always_inline int kmalloc_index(size_t size)if (size <=1024 )return 10 ;if (size <=2 *1024 )return 11 ;if (size <=4 *1024 )return 12 ; -/* -* The following is only needed to support architectures with a larger page -* size than 4k. -*/ if (size <=8 *1024 )return 13 ;if (size <=16 *1024 )return 14 ;if (size <=32 *1024 )return 15 ;
@@ -167,6 +166,10 @@ static __always_inline int kmalloc_index(size_t size)if (size <=512 *1024 )return 19 ;if (size <=1024 *1024 )return 20 ;if (size <=2 *1024 *1024 )return 21 ; +if (size <=4 *1024 *1024 )return 22 ; +if (size <=8 *1024 *1024 )return 23 ; +if (size <=16 *1024 *1024 )return 24 ; +if (size <=32 *1024 *1024 )return 25 ;return -1 ;/*
@@ -191,6 +194,19 @@ static __always_inline struct kmem_cache *kmalloc_slab(size_t size)if (index ==0 )return NULL ; +/* +* This function only gets expanded if __builtin_constant_p(size), so +* testing it here shouldn't be needed. But some versions of gcc need +* help. +*/ +if (__builtin_constant_p (size) && index <0 ) { +/* +* Generate a link failure. Would be great if we could +* do something to stop the compile here. +*/ +extern void __kmalloc_size_too_large (void ); +__kmalloc_size_too_large (); + }return &kmalloc_caches[index]; }
@@ -204,17 +220,9 @@ static __always_inline struct kmem_cache *kmalloc_slab(size_t size)void *kmem_cache_alloc (struct kmem_cache *,gfp_t );void *__kmalloc (size_t size,gfp_t flags); -static __always_inline void *kmalloc_large(size_t size, gfp_t flags) -{ -return (void *)__get_free_pages (flags | __GFP_COMP,get_order (size)); -} - static __always_inline void *kmalloc(size_t size, gfp_t flags) {if (__builtin_constant_p (size)) { -if (size >PAGE_SIZE ) -return kmalloc_large (size, flags); -if (!(flags &SLUB_DMA )) {struct kmem_cache *s =kmalloc_slab (size);
mm/slub.c +10 -32
109 lines not on this page · raw message
Hi, Hugh,
Quoted 6 lines · Hugh Dickins
Quoted 5 lines · Hugh Dickins
Quoted 4 lines · Pekka Enberg
Quoted 3 lines · Hugh Dickins
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though
Would you like to share your tmpfs loop swap load with me, so I could reproduce it on my machines? Do your machines run at i386 mode or x86-64 mode? How much memory do your machines have?
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit
9b2cd506e5f2117f94c28a0040bf5da058105316("slub: Calculatemin_objectsbased on number of processors").
i>>?As I know little about your workload, I just guess from 'loop swap load' that your load eats memory quickly and kernel/swap is started to keep a low free memory.
Commit i>>?9b2cd506e5f2117f94c28a0040bf5da058105316 is just a method to increase the page order for slub so there more free objects available in a slab. That promotes performance for many benchmarks if there are enough __free__ pages. Because memory is cheaper and comparing with cpu number increasing, memory is increased more rapidly. So we create commit 9b2cd506e5f2117f94c28a0040bf5da058105316. In addition, if we have no this commit, we will have another similiar commit to just increase slub_min_objects and slub_max_order.
However, our assumption about free memory seems inappropriate when memory is hungry just like your case. Function allocate_slab always tries the higher order firstly. If it fails to get a new slab, it will tries the minimum order. As for your case, I think the first try always fails, and it takes too much time. Perhaps alloc_pages does far away from a checking even with flag __GFP_NORETRY to consume extra time?
Christoph and Pekka,
Can we add a checking about free memory page number/percentage in function allocate_slab that we can bypass the first try of alloc_pages when memory is hungry?
Quoted 13 lines
Quoted 8 lines
Thanks, yes, that could well account for the residual difference: the machines in question have 2 or 4 cpus, so the old slub_min_objects=4 has effectively become slub_min_objects=12 or slub_min_objects=16.
I'm now trying with slub_max_order=1 slub_min_objects=4 on the boot lines (though I'll need to curtail tests on a couple of machines), and will report back later.
Yes, slub_max_order=1 with slub_min_objects=4 certainly helps this swapping load. I've not tried slub_max_order=0, but I'm running with 8kB stacks, so order 1 seems a reasonable choice.
Quoted 5 lines
I can't say where I pulled that "e.g. 2% slower" from: on different machines slub was 5% or 10% or 20% slower than slab and slqb even with slub_max_order=1 (but not significantly slower on the "immune" machine). How much slub_min_objects=4 helps again varies widely, between halving or eliminating the difference.
I guess your machines have different memory quantity, but your workload mostly consumes specified number of pages, so the result percent is different.
Quoted 3 lines
But I think it's more important that I focus on the worst case machine, try to understand what's going on there.
oprofile data and 'slabinfo -AD' output might help.
yanmin
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi Yanmin,
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?
If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
Pekka
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Pekka Enberg
Hi Yanmin,
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
If the free memory is low then reclaim needs to be run to increase the free memory. Falling back immediately incurs the overhead of going through the order 0 queues.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 9 lines · Christoph Lameter
Quoted 7 lines · Pekka Enberg
Hi Yanmin,
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?
Quoted 6 lines
Quoted 3 lines
If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
If the free memory is low then reclaim needs to be run to increase the free memory.
I think reclaim did start often with Hugh's case. There would be no swap if not.
Quoted 2 lines
Falling back immediately incurs the overhead of going through
the order 0 queues.
The falling back is temporal. Later on when there is enough free pages available, new slab allocations go back to higher order automatically. This is to save the first high-order allocation try because it often fails if memory is hungry.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Pekka Enberg
Hi Yanmin,
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
i>>?We can use nr_free_pages(), totalram_pages and hugetlb_total_pages(). Below patch is a try. I tested it with hackbench and tbench on my stoakley (2 qual-core processors) and tigerton (4 qual-core processors). There is almost no regression.
Besides this patch, I have another patch to try to reduce the calculation of "i>>?totalram_pages - hugetlb_total_pages()", but it touches many files. So just post the first simple patch here for review.
Hugh,
Would you like to test it on your machines?
Thanks,
Yanmin
---
mm/slub.c +10 -4
@@ -23,6 +23,8 @@#include <linux/debugobjects.h> #include <linux/kallsyms.h> #include <linux/memory.h> +#include <linux/swap.h> +#include <linux/hugetlb.h> #include <linux/math64.h> #include <linux/fault-inject.h>
@@ -1076,14 +1078,18 @@ static inline struct page *alloc_slab_pastatic struct page *allocate_slab (struct kmem_cache *s,gfp_t flags,int node) { -struct page *page; +struct page *page =NULL ;struct kmem_cache_order_objects oo = s->oo; +unsigned long free_pages =nr_free_pages (); +unsigned long total_pages = totalram_pages -hugetlb_total_pages (); flags |= s->allocflags; - page =alloc_slab_page (flags | __GFP_NOWARN | __GFP_NORETRY, node, - oo); -if (unlikely (!page)) { +if (free_pages > total_pages >>3 ) { + page =alloc_slab_page (flags | __GFP_NOWARN | __GFP_NORETRY, + node, oo); + } +if (!page) { oo = s->min;/* * Allocation may have failed due to fragmentation.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Yanmin
Quoted 5 lines · Pekka Enberg
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?
Quoted 8 lines
Quoted 3 lines
If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
We can use
nr_free_pages(),totalram_pagesandhugetlb_total_pages(). Below patch is a try. I tested it with hackbench and tbench on my stoakley (2 qual-core processors) and tigerton (4 qual-core processors). There is almost no regression.
May I repeat what I said yesterday? Certainly I'm oversimplifying, but if I'm plain wrong, please correct me.
Having lots of free memory is a temporary accident following process exit (when lots of anonymous memory has suddenly been freed), before it has been put to use for page cache. The kernel tries to run with a certain amount of free memory in reserve, and the rest of memory put to (potentially) good use. I don't think we have the number you're looking for there, though perhaps some approximation could be devised (or I'm looking at the problem the wrong way round).
Perhaps feedback from vmscan.c, on how much it's having to write back, would provide a good clue. There's plenty of stats maintained there.
Quoted 9 lines
Besides this patch, I have another patch to try to reduce the calculation of "
totalram_pages-hugetlb_total_pages()", but it touches many files. So just post the first simple patch here for review.Hugh,
Would you like to test it on your machines?
Indeed I shall, starting in a few hours when I've finished with trying the script I promised yesterday to send you. And I won't be at all surprised if your patch eliminates my worst cases, because I don't expect to have any significant amount of free memory during my testing, and my swap testing suffers from slub's thirst for higher orders.
But I don't believe the kind of check you're making is appropriate, and I do believe that when you try more extensive testing, you'll find regressions in other tests which were relying on the higher orders. If all of your testing happens to have lots of free memory around, I'm surprised; but perhaps I'm naive about how things actually work, especially on the larger machines.
Or maybe your tests are relying crucially on the slabs allocated at system startup, when of course there should be plenty of free memory around.
By the way, when I went to remind myself of what nr_free_pages() actually does, my grep immediately hit this remark in mm/mmap.c:
* nr_free_pages() is very expensive on large systems,
I hope that's just a stale comment from before it was converted to global_page_state(NR_FREE_PAGES)!
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · Hugh Dickins
Quoted 10 lines · Yanmin
Quoted 8 lines · Pekka Enberg
Quoted 3 lines · Yanmin
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?If the check isn't too expensive, I don't any reason not to. How would you go about checking how much free pages there are, though? Is there something in the page allocator that we can use for this?
Quoted 7 lines
Quoted 4 lines
We can use
nr_free_pages(),totalram_pagesandhugetlb_total_pages(). Below patch is a try. I tested it with hackbench and tbench on my stoakley (2 qual-core processors) and tigerton (4 qual-core processors). There is almost no regression.May I repeat what I said yesterday? Certainly I'm oversimplifying, but if I'm plain wrong, please correct me.
I did read your previous email carefully.
Quoted 4 lines
Having lots of free memory is a temporary accident following process exit (when lots of anonymous memory has suddenly been freed), before it has been put to use for page cache.
Some workloads do something like what you said. But lots of workloads don't use up most memory. Keepping an amount of free memory could quicken up system running.
Quoted 8 lines
The kernel tries to run with
a certain amount of free memory in reserve, and the rest of memory put to (potentially) good use. I don't think we have the number you're looking for there, though perhaps some approximation could be devised (or I'm looking at the problem the wrong way round).Perhaps feedback from
vmscan.c, on how much it's having to write back, would provide a good clue. There's plenty of stats maintained there.
My startpoint is to look for a simple formula and the patch is a RFC. If we consider statistics of write back (and so on), what I am worried about is that page allocation called by allocate_slab might cause too many try_to_free_pages/page reclaim/swap when memory is hungry. Originally, I thought that's the root cause of your issue.
In other hand, free memory wouldn't vary too much with a good stably running system. So the patch tries to change allocate_slab:
If there are high-order free pages available without reclaim/swap/free_pages, just go to the first alloc_slab_page; If not, just jump to the second try of alloc_slab_page. Of course, the checking is just a guess, not a guarantee.
Quoted 17 lines
Quoted 10 lines
Besides this patch, I have another patch to try to reduce the calculation of "
totalram_pages-hugetlb_total_pages()", but it touches many files. So just post the first simple patch here for review.Hugh,
Would you like to test it on your machines?
Indeed I shall, starting in a few hours when I've finished with trying the script I promised yesterday to send you. And I won't be at all surprised if your patch eliminates my worst cases, because I don't expect to have any significant amount of free memory during my testing, and my swap testing suffers from slub's thirst for higher orders.
Quoted 3 lines
But I don't believe the kind of check you're making is appropriate, and I do believe that when you try more extensive testing, you'll find regressions in other tests which were relying on the higher orders.
Yes, I agree. And we need find such tests which causes both memory used up and lots of higher-order allocations.
Quoted 3 lines
If all of your testing happens to have lots of free memory around, I'm surprised; but perhaps I'm naive about how things actually work, especially on the larger machines.
I use dozeons of benchmarks, but they are mostly microbench. In addition, mostly, there are lots of free memory when they (except fio and membench) are running.
I tried to enable some big benchmarks, such like specweb2005, but I failed because Tomcat scalability looks not good on 4 qual-core processor machines.
I would find and add new benchmark/tests which use up memory even cause lots of swap into my testing framework.
Quoted 10 lines
Or maybe your tests are relying crucially on the slabs allocated at system startup, when of course there should be plenty of free memory around.
By the way, when I went to remind myself of what
nr_free_pages()actually does, my grep immediately hit this remark inmm/mmap.c:
*nr_free_pages()is very expensive on large systems,
I hope that's just a stale comment from before it was converted toglobal_page_state(NR_FREE_PAGES)!
I think so.
I would try to reproduce your issue on my machines.
Thanks for your scripts and suggestions.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · Yanmin
Quoted 10 lines · Hugh Dickins
Quoted 2 lines · Yanmin
Would you like to test it on your machines?
Indeed I shall, starting in a few hours when I've finished with trying the script I promised yesterday to send you. And I won't be at all surprised if your patch eliminates my worst cases, because I don't expect to have any significant amount of free memory during my testing, and my swap testing suffers from slub's thirst for higher orders.
Quoted 6 lines
Quoted 3 lines
But I don't believe the kind of check you're making is appropriate, and I do believe that when you try more extensive testing, you'll find regressions in other tests which were relying on the higher orders.
Yes, I agree. And we need find such tests which causes both memory used up and lots of higher-order allocations.
Sceptical though I am about your free_pages test in slub's allocate_slab(), I can confirm that your patch does well on my swapping loads, performing slightly (not necessarily significantly) better than slab on those loads (though not quite as well on the "immune" machine where slub was already keeping up with slab; and I haven't even bothered to try it on the machine which behaves so very badly that no conclusions can yet be drawn).
I then tried a patch I thought obviously better than yours: just mask off __GFP_WAIT in that __GFP_NOWARN|__GFP_NORETRY preliminary call to alloc_slab_page(): so we're not trying to infer anything about high- order availability from the number of free order-0 pages, but actually going to look for it and taking it if it's free, forgetting it if not.
That didn't work well at all: almost as bad as the unmodified slub.c. I decided that was due to __alloc_pages_internal()'s wakeup_kswapd(zone, order): just expressing an interest in a high- order page was enough to send it off trying to reclaim them, though not directly. Hacked in a condition to suppress that in this case:
worked a lot better, but not nearly as well as yours. I supposed that was somehow(?) due to the subsequent get_page_from_freelist() calls with different watermarking: hacked in another __GFP flag to break out to nopage just like the NUMA_BUILD GFP_THISNODE case does. Much better, getting close, but still not as good as yours.
I think I'd better turn back to things I understand better!
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 16 lines · Hugh Dickins
Quoted 15 lines · Yanmin
Quoted 13 lines · Hugh Dickins
Quoted 2 lines · Yanmin
Would you like to test it on your machines?
Indeed I shall, starting in a few hours when I've finished with trying the script I promised yesterday to send you. And I won't be at all surprised if your patch eliminates my worst cases, because I don't expect to have any significant amount of free memory during my testing, and my swap testing suffers from slub's thirst for higher orders.
But I don't believe the kind of check you're making is appropriate, and I do believe that when you try more extensive testing, you'll find regressions in other tests which were relying on the higher orders.
Quoted 6 lines
Quoted 2 lines
Yes, I agree. And we need find such tests which causes both memory used up and lots of higher-order allocations.
Sceptical though I am about your
free_pagestest in slub'sallocate_slab(), I can confirm that your patch does well on my swapping loads, performing slightly (not necessarily significantly) better than slab on those loads
As matter of fact, the patch has the same effect like slub_max_order=0 on your workload, except the additional cost to check free pages.
Quoted 21 lines
(though not quite as well on the "immune" machine where slub was already keeping up with slab; and I haven't even bothered to try it on the machine which behaves so very badly that no conclusions can yet be drawn).
I then tried a patch I thought obviously better than yours: just mask off
__GFP_WAITin that __GFP_NOWARN|__GFP_NORETRY preliminary call toalloc_slab_page(): so we're not trying to infer anything about high- order availability from the number of free order-0 pages, but actually going to look for it and taking it if it's free, forgetting it if not.That didn't work well at all: almost as bad as the unmodified
slub.c. I decided that was due to __alloc_pages_internal()'swakeup_kswapd(zone, order): just expressing an interest in a high- order page was enough to send it off trying to reclaim them, though not directly. Hacked in a condition to suppress that in this case:
worked a lot better, but not nearly as well as yours. I supposed that was somehow(?) due to the subsequentget_page_from_freelist()calls with different watermarking: hacked in another__GFPflag to break out to nopage just like theNUMA_BUILDGFP_THISNODEcase does. Much better, getting close, but still not as good as yours.
Quoted 1 line
I think I'd better turn back to things I understand better!
Your investigation is really detail-focused. I also did some testing. i>>?
I changed the script a little. i>>?As no the laptop devices which create the worst result difference, I tested on my stoakley which has 2 qual-core processors, 8GB memory i>>?(started kernel with mem=1GB), i>>?a scsi disk as swap partition (35GB).
The testing runs in a loop. It starts 2 tasks to run kbuild of 2.6.28, build1 and build2 separately. build1 runs on tmpfs directly. build2 runs on a ext2 loop fs on tmpfs. Both build untar the source tarball firstly, then use the defconfig to compile kernel. The script does a sync between build1 and build2, so they could start at the same time every iteration. i>>?
[root@lkp-st02-x8664 ~]# slabinfo -AD|head -n 15
Name Objects Alloc Free %Fast
names_cache 64 11734829 11734830 99 99 filp 1195 8484074 8482982 90 3 vm_area_struct 3830 7688583 7684900 92 54 buffer_head 33970 3832771 3798977 94 0 bio-0 5906 2383929 2378119 91 13 journal_handle 1360 2182580 2182580 99 99
As a matter of fact, I got similiar cache statstics with kbuild on different machines. i>>?names_cache's object size is 4096. filp and i>>?vm_area_struct's are 192/168. i>>?i>>?names_cache's default order is 3. Other active kmem_cache's order is 0. names_cache is used by getname=>__getname from sys_open/execve/faccessstat, etc. Although kernel allocates a page every time for i>>?i>>?names_cache object, mostly, kernel only uses a dozen of bytes per i>>?names_cache object.
With kernel 2.6.29-rc2-slqb0121 (get slqb patch from Pekka's git tree):
Thu Feb 5 15:50:24 CST 2009 2.6.29-rc2slqb0121stat x86_64
[ymzhang@lkp-st02-x8664 Hugh]$ build1 144.15 91.70 32.99
build2 159.81 91.83 34.27
Thu Feb 5 15:53:09 CST 2009: 165 secs for 1 iters, 165 secs per iter
build1 123.02 90.29 33.08 build2 204.52 90.28 34.17 Thu Feb 5 15:56:39 CST 2009: 375 secs for 2 iters, 187 secs per iter build1 132.74 90.60 33.45 build2 210.11 90.80 33.98 Thu Feb 5 16:00:15 CST 2009: 591 secs for 3 iters, 197 secs per iter build1 135.34 90.71 32.95 build2 220.43 91.55 33.99 Thu Feb 5 16:04:00 CST 2009: 816 secs for 4 iters, 204 secs per iter build1 121.68 91.09 33.26 build2 202.45 91.01 34.37 Thu Feb 5 16:07:30 CST 2009: 1026 secs for 5 iters, 205 secs per iter build1 120.51 90.19 33.42 build2 217.56 90.38 34.18 Thu Feb 5 16:11:13 CST 2009: 1249 secs for 6 iters, 208 secs per iter build1 137.14 90.33 34.54 build2 243.14 90.93 34.33 Thu Feb 5 16:15:22 CST 2009: 1498 secs for 7 iters, 214 secs per iter build1 141.47 91.14 33.42 build2 249.78 91.57 34.10 Thu Feb 5 16:19:37 CST 2009: 1753 secs for 8 iters, 219 secs per iter build1 147.72 90.42 34.04 build2 252.57 90.91 33.73 Thu Feb 5 16:23:58 CST 2009: 2014 secs for 9 iters, 223 secs per iter build1 137.40 89.80 33.99 build2 248.67 91.18 34.03
Thu Feb 5 16:28:13 CST 2009: 2269 secs for 10 iters, 226 secs per iter
With kernel 2.6.29-rc2-slubstat (default slub_max_slub):
[ymzhang@lkp-st02-x8664 Hugh]$ sh tmpfs_swap.sh
Thu Feb 5 13:21:37 CST 2009 2.6.29-rc2slubstat x86_64
[ymzhang@lkp-st02-x8664 Hugh]$ build1 155.54 91.90 33.56
build2 163.86 91.69 34.52
Thu Feb 5 13:24:30 CST 2009: 173 secs for 1 iters, 173 secs per iter
build1 135.63 90.42 33.88 build2 308.88 91.63 34.71 Thu Feb 5 13:29:57 CST 2009: 500 secs for 2 iters, 250 secs per iter build1 127.49 90.79 33.24 ymzhang 28382 4079 0 13:29 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 414.77 91.58 34.01 Thu Feb 5 13:37:05 CST 2009: 928 secs for 3 iters, 309 secs per iter build1 146.99 91.07 33.59 ymzhang 24569 4079 0 13:37 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 505.73 93.01 34.12
Thu Feb 5 13:45:46 CST 2009: 1449 secs for 4 iters, 362 secs per iter
build1 163.20 91.35 34.39
ymzhang 20830 4079 0 13:45 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2
The 'tar xfj' line is a sign that if build2's untar finishs when build1 finishs compiling. Above result shows since iters 3, build2's untar isn't finished although build1 finishs compiling already. So build1 result seems quite stable while build2 result is growing.
Comparing with slqb, the result is bad.
i>>?
With kernel 2.6.29-rc2-slubstat (slub_max_slub=1, so i>>?names_cache's order is 1):
[ymzhang@lkp-st02-x8664 Hugh]$ sh tmpfs_swap.sh
Thu Feb 5 14:42:35 CST 2009 2.6.29-rc2slubstat x86_64
[ymzhang@lkp-st02-x8664 Hugh]$ build1 161.61 92.09 34.14
build2 167.92 91.78 34.38
Thu Feb 5 14:45:30 CST 2009: 175 secs for 1 iters, 174 secs per iter
build1 128.22 91.02 33.39 build2 236.95 90.59 34.45 Thu Feb 5 14:49:37 CST 2009: 422 secs for 2 iters, 211 secs per iter build1 134.34 90.56 33.94 ymzhang 28297 4069 0 14:49 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 338.49 91.10 34.33 Thu Feb 5 14:55:27 CST 2009: 772 secs for 3 iters, 257 secs per iter build1 144.50 90.63 34.00 ymzhang 24398 4069 0 14:55 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 415.44 91.32 34.29 Thu Feb 5 15:02:33 CST 2009: 1198 secs for 4 iters, 299 secs per iter build1 137.31 91.03 33.80 ymzhang 20580 4069 0 15:02 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 399.31 91.88 34.31 Thu Feb 5 15:09:24 CST 2009: 1609 secs for 5 iters, 321 secs per iter build1 147.69 91.39 33.98 ymzhang 16743 4069 0 15:09 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 397.33 91.72 34.52 Thu Feb 5 15:16:12 CST 2009: 2017 secs for 6 iters, 336 secs per iter build1 149.65 91.28 33.65 ymzhang 12864 4069 0 15:16 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 469.35 91.78 34.15 Thu Feb 5 15:24:12 CST 2009: 2497 secs for 7 iters, 356 secs per iter build1 138.36 90.66 34.03 ymzhang 9077 4069 0 15:24 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 498.02 91.39 34.60
Thu Feb 5 15:32:38 CST 2009: 3003 secs for 8 iters, 375 secs per iter
We see some improvement, but the improvement isn't big. The result is still worse than slqb's.
i>>?
With kernel 2.6.29-rc2-slubstat (slub_max_slub=0, i>>?names_cache order is 0):
[ymzhang@lkp-st02-x8664 Hugh]$ sh tmpfs_swap.sh
Thu Feb 5 13:59:02 CST 2009 2.6.29-rc2slubstat x86_64
[ymzhang@lkp-st02-x8664 Hugh]$ build1 170.00 92.26 33.63
build2 176.22 91.18 35.16
Thu Feb 5 14:02:04 CST 2009: 182 secs for 1 iters, 182 secs per iter
build1 136.31 90.58 33.98 build2 201.79 91.32 34.92 Thu Feb 5 14:05:31 CST 2009: 389 secs for 2 iters, 194 secs per iter build1 114.12 91.03 33.86 build2 205.86 90.70 34.27 Thu Feb 5 14:09:02 CST 2009: 600 secs for 3 iters, 200 secs per iter build1 131.26 90.63 35.46 build2 227.58 91.36 34.97 Thu Feb 5 14:12:56 CST 2009: 834 secs for 4 iters, 208 secs per iter build1 151.93 90.47 35.87 build2 259.79 91.01 35.35 Thu Feb 5 14:17:21 CST 2009: 1099 secs for 5 iters, 219 secs per iter build1 106.57 92.21 35.75 ymzhang 16139 4052 0 14:17 pts/0 00:00:00 tar xfj /home/ymzhang/tmpfs/linux-2.6.28.tar.bz2 build2 233.17 90.77 35.05 Thu Feb 5 14:21:19 CST 2009: 1337 secs for 6 iters, 222 secs per iter build1 139.56 90.82 33.61 build2 214.44 91.87 34.43 Thu Feb 5 14:25:02 CST 2009: 1560 secs for 7 iters, 222 secs per iter build1 124.91 90.98 34.30 build2 214.43 91.79 34.35 Thu Feb 5 14:28:44 CST 2009: 1782 secs for 8 iters, 222 secs per iter build1 134.76 90.80 33.59 build2 239.88 91.81 34.45 Thu Feb 5 14:32:48 CST 2009: 2026 secs for 9 iters, 225 secs per iter build1 141.23 90.98 33.74 build2 250.96 91.72 34.20
Thu Feb 5 14:37:06 CST 2009: 2284 secs for 10 iters, 228 secs per iter
I repeat the testing and the results have fluctuation. I would like to consider the result of slub (slub_max_order=0) is equal to slqb's.
Another testing is to start 2 parallel build1 testing. slub (default order) seems having 17% regression against slqb. With slub_max_order=1, slub is ok.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi Hugh,
Quoted 6 lines · Hugh Dickins
I then tried a patch I thought obviously better than yours: just mask off
__GFP_WAITin that __GFP_NOWARN|__GFP_NORETRY preliminary call toalloc_slab_page(): so we're not trying to infer anything about high- order availability from the number of free order-0 pages, but actually going to look for it and taking it if it's free, forgetting it if not.
Quoted 10 lines
That didn't work well at all: almost as bad as the unmodified
slub.c. I decided that was due to __alloc_pages_internal()'swakeup_kswapd(zone, order): just expressing an interest in a high- order page was enough to send it off trying to reclaim them, though not directly. Hacked in a condition to suppress that in this case:
worked a lot better, but not nearly as well as yours. I supposed that was somehow(?) due to the subsequentget_page_from_freelist()calls with different watermarking: hacked in another__GFPflag to break out to nopage just like theNUMA_BUILDGFP_THISNODEcase does. Much better, getting close, but still not as good as yours.
Did you look at it with oprofile? One thing to keep in mind is that if there are 4K allocations going on, your approach will get double the overhead of page allocations (which can be substantial performance hit for slab).
Pekka
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 18 lines · Pekka Enberg
Quoted 6 lines · Hugh Dickins
I then tried a patch I thought obviously better than yours: just mask off
__GFP_WAITin that __GFP_NOWARN|__GFP_NORETRY preliminary call toalloc_slab_page(): so we're not trying to infer anything about high- order availability from the number of free order-0 pages, but actually going to look for it and taking it if it's free, forgetting it if not.Quoted 10 lines
That didn't work well at all: almost as bad as the unmodified
slub.c. I decided that was due to __alloc_pages_internal()'swakeup_kswapd(zone, order): just expressing an interest in a high- order page was enough to send it off trying to reclaim them, though not directly. Hacked in a condition to suppress that in this case:
worked a lot better, but not nearly as well as yours. I supposed that was somehow(?) due to the subsequentget_page_from_freelist()calls with different watermarking: hacked in another__GFPflag to break out to nopage just like theNUMA_BUILDGFP_THISNODEcase does. Much better, getting close, but still not as good as yours.
Quoted 1 line
Did you look at it with oprofile?
No, I didn't. I didn't say so, but again it was elapsed time that I was focussing on, so I don't think oprofile would be relevant. There are some differences in system time, of course, consistent with your point; but they're generally an order of magnitude less, so didn't excite my interest.
Quoted 4 lines
One thing to keep in mind is that if
there are 4K allocations going on, your approach will get double the overhead of page allocations (which can be substantial performance hit for slab).
Sure, and even the current allocate_slab() is inefficient in that
respect: I've followed it because I do for now have an interest in the stats, but if stats are configured off then there's no point in dividing it into two stages; and if they are really intended to be ORDER_FALLBACK stats, then it shouldn't divide into two stages when oo_order(s->oo) == oo_order(s->min). On the other hand, I find it interesting to see how often the __GFP_NORETRY fails, even when the order is the same each time (and usually 0).
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Hugh Dickins
Quoted 7 lines · Pekka Enberg
Quoted 6 lines · Hugh Dickins
I then tried a patch I thought obviously better than yours: just mask off
__GFP_WAITin that __GFP_NOWARN|__GFP_NORETRY preliminary call toalloc_slab_page(): so we're not trying to infer anything about high- order availability from the number of free order-0 pages, but actually going to look for it and taking it if it's free, forgetting it if not.
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
That didn't work well at all: almost as bad as the unmodified
slub.c. I decided that was due to __alloc_pages_internal()'swakeup_kswapd(zone, order): just expressing an interest in a high- order page was enough to send it off trying to reclaim them, though not directly. Hacked in a condition to suppress that in this case:
worked a lot better, but not nearly as well as yours. I supposed that was somehow(?) due to the subsequentget_page_from_freelist()calls with different watermarking: hacked in another__GFPflag to break out to nopage just like theNUMA_BUILDGFP_THISNODEcase does. Much better, getting close, but still not as good as yours.
I did the similiar hack. i>>?get_page_from_freelist, wakeup_kswapd, try_to_free_pages, and drain_all_pages consume time. If I disable them one by one, I see the result is improved gradually.
Quoted 5 lines
Quoted 2 lines
Did you look at it with oprofile?
No, I didn't. I didn't say so, but again it was elapsed time that I was focussing on, so I don't think oprofile would be relevant.
The vmstat data varies very much when testing runs. The original test case consists of 2 kbuild tasks and sometimes the 2 tasks almost run serially because it takes a long time to untie kernel source tarball on the loop ext2 fs. So it's not appropriate to collect oprofile data.
I changed the script to run 2 tasks on tmpfs without loop ext2 device. The result difference between slub_max_order=0 and default order is about 25%. When kernel building is started, vmstat sys time is about 4%~10% on my
2 qual-core processor stoakley. io-wait is mostly 40%~80%. I collected the oprofile data. Mostly, only free_pages_bulk seems a little abnormal. With default order, i>>?free_pages_bulk is more than 1% while it's 0.23%. By changing total memory quantity, i>>?free_pages_bulk difference between i>>?slub_max_order=0 and default order is about 1%.
Quoted 9 lines
There are some differences in system time, of course, consistent with your point; but they're generally an order of magnitude less, so didn't excite my interest.
Quoted 4 lines
One thing to keep in mind is that if
there are 4K allocations going on, your approach will get double the overhead of page allocations (which can be substantial performance hit for slab).
Quoted 6 lines
Sure, and even the current
allocate_slab()is inefficient in that
respect: I've followed it because I do for now have an interest in the stats, but if stats are configured off then there's no point in dividing it into two stages; and if they are really intended to beORDER_FALLBACKstats, then it shouldn't divide into two stages whenoo_order(s->oo)==oo_order(s->min).
You are right theoretically. Under the real environment, the order mostly is 0 when i>>?oo_order(s->oo) == oo_order(s->min), and order 0 page allocation almost doesn't fail even with flag i>>?__GFP_NORETRY. When default order isn't 0, mostly, i>>?oo_order(s->oo) isn't equal to i>>?oo_order(s->min).
Quoted 3 lines
On the other hand, I find it
interesting to see how often the__GFP_NORETRYfails, even when the order is the same each time (and usually 0).
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 9 lines · Yanmin
Quoted 6 lines · Hugh Dickins
Quoted 5 lines · Hugh Dickins
Quoted 4 lines · Pekka Enberg
Quoted 3 lines · Hugh Dickins
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though
Would you like to share your tmpfs loop swap load with me, so I could reproduce it on my machines?
A very reasonable request that I feared someone would make! I'm sure we all have test scripts that we can run happily ourselves, but as soon as someone else asks, we want to make this and that and the other adjustment, if only to reduce the amount of setup description required - this is one such. I guess I can restrain myself a little if I'm just sending it to you, separately.
Quoted 1 line
Do your machines run at i386 mode or x86-64 mode?
Both: one is a ppc64 (G5 Quad), one is i386 only (Atom SSD netbook), three I can run either way (though my common habit is to run two as i386 with 32bit userspace and one as x86_64 with 64bit userspace).
Quoted 1 line
How much memory do your machines have?
I use mem=700M when running such tests on all of them (but leave the netbook with its 1GB mem): otherwise I'd have to ramp up the test in different ways to get them all swapping enough - it is tmpfs and swapping that I'm personally most concerned to test.
Quoted 24 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
What kind of machine are you seeing this on? It sounds like it could be a side-effect from commit
9b2cd506e5f2117f94c28a0040bf5da058105316("slub: Calculatemin_objectsbased on number of processors").As I know little about your workload, I just guess from 'loop swap load' that your load eats memory quickly and kernel/swap is started to keep a low free memory.
Commit 9b2cd506e5f2117f94c28a0040bf5da058105316 is just a method to increase the page order for slub so there more free objects available in a slab. That promotes performance for many benchmarks if there are enough
__free__pages. Because memory is cheaper and comparing with cpu number increasing, memory is increased more rapidly. So we create commit9b2cd506e5f2117f94c28a0040bf5da058105316. In addition, if we have no this commit, we will have another similiar commit to just increaseslub_min_objectsandslub_max_order.
Quoted 6 lines
However, our assumption about free memory seems inappropriate when memory is hungry just like your case. Function
allocate_slabalways tries the higher order firstly. If it fails to get a new slab, it will tries the minimum order. As for your case, I think the first try always fails, and it takes too much time. Perhapsalloc_pagesdoes far away from a checking even with flag__GFP_NORETRYto consume extra time?
I believe you're thinking there of how much system time is used. I haven't been paying much attention to that, and don't have any complaints about slub from that angle (what's most noticeable there is that, as expected, slob uses more system time than slab or slqb or slub). Although I do record the system time reported for the test, I very rarely think to add in kswapd0's and loop0's times, which would be very significant missed contributions.
What I've been worried by is the total elapsed times, that's where slub shows up badly. That means, I think, that bad decisions are being made about what to swap out when, so that altogether there's too much swapping: which is understandable when slub is aiming for higher order allocations. One page of the high order is selected according to vmscan's usual criteria, but the remaining pages will be chosen according to their adjacence rather than their age (to some extent: there is code in there to resist bad decisions too). If we imagine that vmscan's usual criteria are perfect (ha ha), then it's unsurprising that going for higher order allocations leads it to make inferior decisions and swap out too much.
Quoted 6 lines
Christoph and Pekka,
Can we add a checking about free memory page number/percentage in function
allocate_slabthat we can bypass the first try ofalloc_pageswhen memory is hungry?
Having lots of free memory is a temporary accident following process exit (when lots of anonymous memory has suddenly been freed), before it has been put to use for page cache. The kernel tries to run with a certain amount of free memory in reserve, and the rest of memory put to (potentially) good use. I don't think we have the number you're looking for there, though perhaps some approximation could be devised (or I'm looking at the problem the wrong way round).
Perhaps feedback from vmscan.c, on how much it's having to write back, would provide a good clue. There's plenty of stats maintained there.
Quoted 13 lines
Quoted 13 lines
Quoted 8 lines
Thanks, yes, that could well account for the residual difference: the machines in question have 2 or 4 cpus, so the old slub_min_objects=4 has effectively become slub_min_objects=12 or slub_min_objects=16.
I'm now trying with slub_max_order=1 slub_min_objects=4 on the boot lines (though I'll need to curtail tests on a couple of machines), and will report back later.
Yes, slub_max_order=1 with slub_min_objects=4 certainly helps this swapping load. I've not tried slub_max_order=0, but I'm running with 8kB stacks, so order 1 seems a reasonable choice.
Quoted 8 lines
Quoted 5 lines
I can't say where I pulled that "e.g. 2% slower" from: on different machines slub was 5% or 10% or 20% slower than slab and slqb even with slub_max_order=1 (but not significantly slower on the "immune" machine). How much slub_min_objects=4 helps again varies widely, between halving or eliminating the difference.
I guess your machines have different memory quantity, but your workload mostly consumes specified number of pages, so the result percent is different.
No, mem=700M in each case but the netbook.
Quoted 4 lines
Quoted 3 lines
But I think it's more important that I focus on the worst case machine, try to understand what's going on there.
oprofile data and 'slabinfo -AD' output might help.
oprofile I doubt here, since it's the total elapsed time that worries me. I had to look up 'slabinfo -AD', yes, thanks for that pointer, it may help when I get around to investigating my totally unsubstantiated suspicion ...
... on the laptop which suffers worst from slub, I am using an SD card accessed as USB storage for swap (but no USB storage on the others). I'm suspecting there's something down that stack which is slow to recover from allocation failures: when I tried a much simplified test using just two "cp -a"s, they can hang on that box. So my current guess is that slub makes something significantly worse (some debug options make it significantly worse too), but the actual bug is elsewhere.
Hugh
Quoted 9 lines · Hugh Dickins
Quoted 5 lines · Nick Piggin
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with), and improved performance and reduced locking somewhat for node-specific and interleaved allocations.
I haven't reviewed your postings, but I did give the previous version of your patch a try on all my machines. Some observations and one patch.
Great, thanks!
Quoted 6 lines
I was initially _very_ impressed by how well it did on my venerable tmpfs loop swapping loads, where I'd expected next to no effect; but that turned out to be because on three machines I'd been using SLUB, without remembering how default
slub_max_ordergot raised from 1 to 3 in 2.6.26 (hmm, andDocumentation/vm/slub.txtnot updated).
Quoted 9 lines
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
Yeah, that's to be expected with higher order allocations I think. Does your immune machine simply have fewer CPUs and thus doesn't use such high order allocations?
Quoted 9 lines
So in fact SLQB comes in very much like SLAB, as I think you'd expect:
slightly ahead of it on most of the machines, but probably in the noise. (SLOB behaves decently: not a winner, but no catastrophic behaviour.)What I love most about SLUB is the way you can reasonably build with
CONFIG_SLUB_DEBUG=y, very little impact, then switch on the specific debugging you want with a boot option when you want it. That was a great stride forward, which you've followed in SLQB: so I'd have to prefer SLQB to SLAB (on debuggability) and to SLUB (on high orders).
It is nice. All credit to Christoph for that (and the fine grained sysfs code).
Quoted 4 lines
I do hate the name SLQB. Despite having no experience of databases, I find it almost impossible to type, coming out as SQLB most times. Wish you'd invented a plausible vowel instead of the Q; but probably too late for that.
Yeah, apologies for the name :P
Quoted 1 line
init/Kconfig describes it as "Qeued allocator": should say "Queued".
Thanks.
Quoted 4 lines
Documentation/vm/slqbinfo.cgives several compilation warnings:
I'd rather leave it to you to fix them, maybe the unused variables are about to be used, or maybe there's much worse wrong with it than a few compilation warnings, I didn't investigate.
OK.
Quoted 7 lines
The only bug I found (but you'll probably want to change the patch
- which I've rediffed to today'sslqb.c, but not retested).On fake NUMA I hit kernel BUG at
mm/slqb.c:1107!claim_remote_free_list()is doing several things without remote_free.lock: thatVM_BUG_ONis unsafe for one, and even if others are somehow safe today, it will be more robust to take the lock sooner.
Good catch, thanks. The BUG should be OK where it is if we only claim the remote free list when remote_free_check is is set, but some of the periodic reaping and teardown code calls it unconditionally. But it's not critical so it should definitely go inside the lock.
Quoted 3 lines
I moved the prefetchw(head) down to where we know it's going to be the head, and replaced the offending
VM_BUG_ONby a laterWARN_ONwhich you'd probably better remove altogether: once we got the lock, it's hardly interesting.
Right, I'll probably do that. Thanks!
Quoted 35 lines
Signed-off-by Hugh Dickins <[email protected]>---
mm/slqb.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)mm/slqb.c renamed from mm/slqb.c.orig +4
-7@@ -1115,17 +1115,12 @@ static void claim_remote_free_list(strucvoid **head, **tail;int nr; -VM_BUG_ON (!l->remote_free.list.head != !l->remote_free.list.tail); -if (!l->remote_free.list.nr)return ; +spin_lock (&l->remote_free.lock); l->remote_free_check =0 ; head = l->remote_free.list.head; -/* Get the head hot for the likely subsequent allocation or flush */ -prefetchw (head); - -spin_lock (&l->remote_free.lock); l->remote_free.list.head =NULL ; tail = l->remote_free.list.tail; l->remote_free.list.tail =NULL ;@@ -1133,9 +1128,15 @@ static void claim_remote_free_list(struc l->remote_free.list.nr =0 ;spin_unlock (&l->remote_free.lock); -if (!l->freelist.nr) +WARN_ON (!head + !tail != !nr + !nr); +if (!nr) +return ;
mm/slqb.c static void claim_remote_free_list(struc · line 1142
Quoted 10 lines
mm/slqb.c renamed from mm/slqb.c.orig +5
-1+ +if (!l->freelist.nr) { +/* Get head hot for likely subsequent allocation or flush */ +prefetchw (head); l->freelist.head = head; - else + }else set_freepointer (s, l->freelist.tail, head); l->freelist.tail = tail;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 12 lines · Nick Piggin
Quoted 10 lines · Hugh Dickins
That's been making SLUB behave pretty badly (e.g. elapsed time 30% more than SLAB) with swapping loads on most of my machines. Though oddly one seems immune, and another takes four times as long: guess it depends on how close to thrashing, but probably more to investigate there. I think my original SLUB versus SLAB comparisons were done on the immune one: as I remember, SLUB and SLAB were equivalent on those loads when SLUB came in, but even with boot option slub_max_order=1, SLUB is still slower than SLAB on such tests (e.g. 2% slower). FWIW - swapping loads are not what anybody should tune for.
Quoted 3 lines
Yeah, that's to be expected with higher order allocations I think. Does your immune machine simply have fewer CPUs and thus doesn't use such high order allocations?
No, it's just one of the quads. Whereas the worst affected (laptop) is a duo. I should probably be worrying more about that one: it may be that I'm thrashing it and its results are meaningless, though still curious that slab and slqb and slob all do so markedly better on it.
It's behaving much better with slub_max_order=1 slub_min_objects=4, but to get competitive I've had to switch off most of the debugging options I usually have on that one - and I've not yet tried slab, slqb and slob with those off too. Hmm, it looks like its getting progressively slower.
I'll continue to investigate at leisure,
but can't give it too much attention.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 5 lines · Nick Piggin
Hi,
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with),
Panic again on my Montvale Itanium NUMA machine if I start kernel with parameter mem=2G.
The call chain is mnt_init => sysfs_init. i>>?kmem_cache_create fails, so later on when i>>?mnt_init uses kmem_cache sysfs_dir_cache, kernel panic
at __slab_alloc => get_cpu_slab because parameter s is equal to NULL.
Function __remote_slab_alloc return NULL when s->node[node]==NULL. That causes i>>?sysfs_init => kmem_cache_create fails.
------------------log----------------
Dentry cache hash table entries: 262144 (order: 7, 2097152 bytes) Inode-cache hash table entries: 131072 (order: 6, 1048576 bytes) Mount-cache hash table entries: 1024mnt_init: sysfs_init error: -12
Unable to handle kernel NULL pointer dereference (address 0000000000002058) swapper[0]: Oops 8813272891392 [1] Modules linked in:
Pid: 0, CPU 0, comm: swapper
psr : 00001010084a2018 ifs : 8000000000000690 ip : [<a000000100180350>] Not tainted (2.6.29-rc2slqb0121)
ip is at kmem_cache_alloc+0x150/0x4e0
unat: 0000000000000000 pfs : 0000000000000690 rsc : 0000000000000003
rnat: 0009804c8a70433f bsps: a000000100f484b0 pr : 656960155aa65959
ldrs: 0000000000000000 ccv : 000000000000001a fpsr: 0009804c8a70433f csd : 893fffff000f0000 ssd : 893fffff00090000
b0 : a000000100180270 b6 : a000000100507360 b7 : a000000100507360 f6 : 000000000000000000000 f7 : 1003e0000000000000800 f8 : 1003e0000000000000008 f9 : 1003e0000000000000001 f10 : 1003e0000000000000031 f11 : 1003e7d6343eb1a1f58d1 r1 : a0000001011bc810 r2 : 0000000000000008 r3 : ffffffffffffffff r8 : 0000000000000000 r9 : a000000100ded800 r10 : 0000000000000000 r11 : a000000100ded800 r12 : a000000100db3d80 r13 : a000000100dac000 r14 : 0000000000000000 r15 : fffffffffffffffe r16 : a000000100fbcd30 r17 : a000000100dacc44 r18 : 0000000000002058 r19 : 0000000000000000 r20 : 0000000000000000 r21 : a000000100dacc44 r22 : 0000000000000002 r23 : 0000000000000066 r24 : 0000000000000073 r25 : 0000000000000000 r26 : e000000102014030 r27 : a0007fffffc9f120 r28 : 0000000000000000 r29 : 0000000000000000 r30 : 0000000000000008 r31 : 0000000000000001
Call Trace:
[<a000000100016240>] show_stack+0x40/0xa0
sp=a000000100db3950 bsp=a000000100dad140
[<a000000100016b50>] show_regs+0x850/0x8a0
sp=a000000100db3b20 bsp=a000000100dad0e8
[<a00000010003a5f0>] die+0x230/0x360
sp=a000000100db3b20 bsp=a000000100dad0a0
[<a00000010005e0e0>] ia64_do_page_fault+0x8e0/0xa40
sp=a000000100db3b20 bsp=a000000100dad050
[<a00000010000c700>] ia64_native_leave_kernel+0x0/0x280
sp=a000000100db3bb0 bsp=a000000100dad050
[<a000000100180350>] kmem_cache_alloc+0x150/0x4e0
sp=a000000100db3d80 bsp=a000000100dacfc8
[<a000000100238610>] sysfs_new_dirent+0x90/0x240
sp=a000000100db3d80 bsp=a000000100dacf80
[<a000000100239140>] create_dir+0x40/0x100
sp=a000000100db3d90 bsp=a000000100dacf48
[<a0000001002392b0>] sysfs_create_dir+0xb0/0x100
sp=a000000100db3db0 bsp=a000000100dacf28
[<a0000001004eca60>] kobject_add_internal+0x1e0/0x420
sp=a000000100db3dc0 bsp=a000000100dacee8
[<a0000001004eceb0>] kobject_add_varg+0x90/0xc0
sp=a000000100db3dc0 bsp=a000000100daceb0
[<a0000001004ed620>] kobject_add+0x100/0x140
sp=a000000100db3dc0 bsp=a000000100dace50
[<a0000001004ed6b0>] kobject_create_and_add+0x50/0xc0
sp=a000000100db3e00 bsp=a000000100dace20
[<a000000100c28ff0>] mnt_init+0x1b0/0x480
sp=a000000100db3e00 bsp=a000000100dacde0
[<a000000100c28610>] vfs_caches_init+0x230/0x280
sp=a000000100db3e20 bsp=a000000100dacdb8
[<a000000100c01410>] start_kernel+0x830/0x8c0
sp=a000000100db3e20 bsp=a000000100dacd40
[<a0000001009d7b60>] __kprobes_text_end+0x760/0x780
sp=a000000100db3e30 bsp=a000000100dacca0
Kernel panic - not syncing: Attempted to kill the idle task!--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 9 lines · Yanmin
Quoted 5 lines · Nick Piggin
Hi,
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with),
Panic again on my Montvale Itanium NUMA machine if I start kernel with parameter mem=2G.
Quoted 6 lines
The call chain is
mnt_init=>sysfs_init. ???kmem_cache_create fails, so later on when ???mnt_init useskmem_cachesysfs_dir_cache, kernel panic
at__slab_alloc=>get_cpu_slabbecause parameter s is equal to NULL.Function
__remote_slab_allocreturn NULL when s->node[node]==NULL. That causes ???sysfs_init =>kmem_cache_createfails.
Hmm, I'll probably have to add a bit more fallback logic. I'll have to work out what semantics the callers require here. Thanks for the report.
Quoted 61 lines
------------------log----------------
Dentry cache hash table entries: 262144 (order: 7, 2097152 bytes) Inode-cache hash table entries: 131072 (order: 6, 1048576 bytes) Mount-cache hash table entries: 1024
mnt_init:sysfs_initerror: -12Unable to handle kernel NULL pointer dereference (address 0000000000002058) swapper[0]: Oops 8813272891392 [1] Modules linked in:Pid: 0, CPU 0, comm: swapper
psr :00001010084a2018ifs : 8000000000000690 ip : [<a000000100180350>] Not tainted (2.6.29-rc2slqb0121)
ip is atkmem_cache_alloc+0x150/0x4e0
unat: 0000000000000000 pfs : 0000000000000690 rsc : 0000000000000003
rnat:0009804c8a70433fbsps:a000000100f484b0pr :656960155aa65959
ldrs: 0000000000000000 ccv :000000000000001afpsr:0009804c8a70433fcsd :893fffff000f0000ssd :893fffff00090000
b0 :a000000100180270b6 :a000000100507360b7 :a000000100507360f6 : 000000000000000000000 f7 :1003e0000000000000800f8 :1003e0000000000000008f9 :1003e0000000000000001f10 :1003e0000000000000031f11 :1003e7d6343eb1a1f58d1r1 :a0000001011bc810r2 : 0000000000000008 r3 : ffffffffffffffff r8 : 0000000000000000 r9 :a000000100ded800r10 : 0000000000000000 r11 :a000000100ded800r12 :a000000100db3d80r13 :a000000100dac000r14 : 0000000000000000 r15 : fffffffffffffffe r16 :a000000100fbcd30r17 :a000000100dacc44r18 : 0000000000002058 r19 : 0000000000000000 r20 : 0000000000000000 r21 :a000000100dacc44r22 : 0000000000000002 r23 : 0000000000000066 r24 : 0000000000000073 r25 : 0000000000000000 r26 :e000000102014030r27 :a0007fffffc9f120r28 : 0000000000000000 r29 : 0000000000000000 r30 : 0000000000000008 r31 : 0000000000000001Call Trace: [<a000000100016240>] show_stack+0x40/0xa0 sp=a000000100db3950 bsp=a000000100dad140 [<a000000100016b50>] show_regs+0x850/0x8a0 sp=a000000100db3b20 bsp=a000000100dad0e8 [<a00000010003a5f0>] die+0x230/0x360 sp=a000000100db3b20 bsp=a000000100dad0a0 [<a00000010005e0e0>] ia64_do_page_fault+0x8e0/0xa40 sp=a000000100db3b20 bsp=a000000100dad050 [<a00000010000c700>] ia64_native_leave_kernel+0x0/0x280 sp=a000000100db3bb0 bsp=a000000100dad050 [<a000000100180350>] kmem_cache_alloc+0x150/0x4e0 sp=a000000100db3d80 bsp=a000000100dacfc8 [<a000000100238610>] sysfs_new_dirent+0x90/0x240 sp=a000000100db3d80 bsp=a000000100dacf80 [<a000000100239140>] create_dir+0x40/0x100 sp=a000000100db3d90 bsp=a000000100dacf48 [<a0000001002392b0>] sysfs_create_dir+0xb0/0x100 sp=a000000100db3db0 bsp=a000000100dacf28 [<a0000001004eca60>] kobject_add_internal+0x1e0/0x420 sp=a000000100db3dc0 bsp=a000000100dacee8 [<a0000001004eceb0>] kobject_add_varg+0x90/0xc0 sp=a000000100db3dc0 bsp=a000000100daceb0 [<a0000001004ed620>] kobject_add+0x100/0x140 sp=a000000100db3dc0 bsp=a000000100dace50 [<a0000001004ed6b0>] kobject_create_and_add+0x50/0xc0 sp=a000000100db3e00 bsp=a000000100dace20 [<a000000100c28ff0>] mnt_init+0x1b0/0x480
Quoted 9 lines
sp=a000000100db3e00 bsp=a000000100dacde0
[<a000000100c28610>] vfs_caches_init+0x230/0x280
sp=a000000100db3e20 bsp=a000000100dacdb8
[<a000000100c01410>] start_kernel+0x830/0x8c0
sp=a000000100db3e20 bsp=a000000100dacd40
[<a0000001009d7b60>] __kprobes_text_end+0x760/0x780
sp=a000000100db3e30 bsp=a000000100dacca0
Kernel panic - not syncing: Attempted to kill the idle task!--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 9 lines · Yanmin
Quoted 5 lines · Nick Piggin
Hi,
Since last posted, I've cleaned up a few bits and pieces, (hopefully) fixed a known bug where it wouldn't boot on memoryless nodes (I don't have a system to test with),
Panic again on my Montvale Itanium NUMA machine if I start kernel with parameter mem=2G.
Quoted 6 lines
The call chain is
mnt_init=>sysfs_init. ???kmem_cache_create fails, so later on when ???mnt_init useskmem_cachesysfs_dir_cache, kernel panic
at__slab_alloc=>get_cpu_slabbecause parameter s is equal to NULL.Function
__remote_slab_allocreturn NULL when s->node[node]==NULL. That causes ???sysfs_init =>kmem_cache_createfails.
Booting with mem= is a good trick to create memoryless nodes easily. Unfortunately it didn't trigger any bugs on my system, so I couldn't actually verify that the fallback code solve your problem. Would you be able to test with this updated patch (which also includes Hugh's fix and some code style changes).
The other thing is that this bug has uncovered a little buglet in the sysfs setup code: if it is unable to continue in a degraded mode after the allocation failure, it should be using SLAB_PANIC.
Thanks,
Nick
---
include/linux/rcupdate.h +1 -10
@@ -33,6 +33,7 @@#ifndef __LINUX_RCUPDATE_H#define __LINUX_RCUPDATE_H +#include <linux/rcu_types.h> #include <linux/cache.h> #include <linux/spinlock.h> #include <linux/threads.h>
@@ -42,16 +43,6 @@#include <linux/lockdep.h> #include <linux/completion.h> -/** -* struct rcu_head - callback structure for use with RCU -* @next: next update requests in a list -* @func: actual update function to call after the grace period. -*/ -struct rcu_head { -struct rcu_head *next; -void (*func)(struct rcu_head *head); -}; -#if defined(CONFIG_CLASSIC_RCU )#include <linux/rcuclassic.h> #elif defined(CONFIG_TREE_RCU )
include/linux/slqb_def.h new file +289 -0
289 lines not on this page · raw message
init/Kconfig +7 -2
@@ -806,7 +806,7 @@ config SLUB_DEBUG choice prompt "Choose SLAB allocator" - default SLUB + default SLQB help This option allows to select a slab allocator.
@@ -827,6 +827,11 @@ config SLUB and has enhanced diagnostics. SLUB is the default choice for a slab allocator. +config SLQB + bool "SLQB (Qeued allocator)" + help + SLQB is a proposed new slab allocator. + config SLOB depends on EMBEDDED bool "SLOB (Simple Allocator)"
@@ -868,7 +873,7 @@ config HAVE_GENERIC_DMA_COHERENT config SLABINFO bool depends on PROC_FS - depends on SLAB || SLUB_DEBUG + depends on SLAB || SLUB_DEBUG || SLQB default y config RT_MUTEXES
lib/Kconfig.debug +20 -0
@@ -298,6 +298,26 @@ config SLUB_STATS out which slabs are relevant to a particular load. Try running: slabinfo -DA +config SLQB_DEBUG + default y + bool "Enable SLQB debugging support" + depends on SLQB + +config SLQB_DEBUG_ON + default n + bool "SLQB debugging on by default" + depends on SLQB_DEBUG + +config SLQB_SYSFS + bool "Create SYSFS entries for slab caches" + default n + depends on SLQB + +config SLQB_STATS + bool "Enable SLQB performance statistics" + default n + depends on SLQB_SYSFS + config DEBUG_PREEMPT bool "Debug preemptible kernel" depends on DEBUG_KERNEL && PREEMPT && (TRACE_IRQFLAGS_SUPPORT || PPC64)
mm/slqb.c new file +3509 -0
2,635 lines not on this page · raw message
Documentation/vm/slqbinfo.c new file +1054 -0
1,054 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Part of the diff is not shown here. View the raw message.
Quoted 3 lines · Nick Piggin
... Would you be able to test with this updated patch (which also includes Hugh's fix ...
In fact not: claim_remote_free_list() still has the offending unlocked
+ VM_BUG_ON(!l->remote_free.list.head != !l->remote_free.list.tail);
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
mm/slqb.c line 1153
Quoted 7 lines · Hugh Dickins
Quoted 3 lines · Nick Piggin
... Would you be able to test with this updated patch (which also includes Hugh's fix ...
In fact not:
claim_remote_free_list()still has the offending unlockedmm/slqb.c new file +1
-0+VM_BUG_ON (!l->remote_free.list.head != !l->remote_free.list.tail);
Doh, thanks. Turned out to still miss a few cases where it wasn't checking for memoryless nodes (Andi explains why I didn't see it with x86-64: because it handles the case differently and assigns the default node to the nearest one with memory. I think).
Working on a new version, so I've definitely got your bug covered now :)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Nick Piggin <[email protected]> writes:
Not a full review, just some things i noticed.
The code is very readable thanks (that's imho the main reason slab.c should go btw, it's really messy and hard to get through)
Quoted 7 lines
Using lists rather than arrays can reduce the cacheline footprint. When moving objects around, SLQB can move a list of objects from one CPU to another by simply manipulating a head pointer, wheras SLAB needs to memcpy arrays. Some SLAB per-CPU arrays can be up to 1K in size, which is a lot of cachelines that can be touched during alloc/free. Newly freed objects tend to be cache hot, and newly allocated ones tend to soon be touched anyway, so often there is little cost to using metadata in the objects.
You're probably aware of that, but the obvious counter argument is that for manipulating a single object a double linked list will always require touching three cache lines (prev, current, next), while an array access only a single one. A possible alternative would be a list of shorter arrays.
Quoted 6 lines
include/linux/slqb_def.h new file +6
-0+int objsize;/* The size of an object without meta data */ +int offset;/* Free pointer offset. */ +int objects;/* Number of objects in slab */ + +int size;/* The size of an object including meta data */ +int order;/* Allocation order */
include/linux/slqb_def.h line 139
Quoted 10 lines
include/linux/slqb_def.h new file +10
-0+gfp_t allocflags;/* gfp flags to use on allocation */ +unsigned int colour_range;/* range of colour counter */ +unsigned int colour_off;/* offset per colour */ +void (*ctor)(void *); + +const char *name;/* Name (only for display!) */ +struct list_head list;/* List of slab caches */ + +int align;/* Alignment */ +int inuse;/* Offset to metadata */
I suspect some of these fields could be short or char (E.g. alignment), possibly lowering cache line impact.
include/linux/slqb_def.h line 148
Quoted 9 lines
include/linux/slqb_def.h new file +9
-0+ +#ifdef CONFIG_SLQB_SYSFS +struct kobject kobj;/* For sysfs */ +#endif +#ifdef CONFIG_NUMA +struct kmem_cache_node *node[MAX_NUMNODES ]; +#endif +#ifdef CONFIG_SMP +struct kmem_cache_cpu *cpu_slab[NR_CPUS ];
Those both really need to be dynamically allocated, otherwise it wastes a lot of memory in the common case
(e.g. NR_CPUS==128 kernel on dual core system). And of course on the proposed NR_CPUS==4096 kernels it becomes prohibitive.
You could use alloc_percpu? There's no alloc_pernode unfortunately, perhaps there should be one.
Quoted 17 lines
include/linux/slqb_def.h new file +17
-0+#if L1_CACHE_BYTES <64 +if (size >64 && size <=96 ) +return 1 ; +#endif +#if L1_CACHE_BYTES <128 +if (size >128 && size <=192 ) +return 2 ; +#endif +if (size <=8 )return 3 ; +if (size <=16 )return 4 ; +if (size <=32 )return 5 ; +if (size <=64 )return 6 ; +if (size <=128 )return 7 ; +if (size <=256 )return 8 ; +if (size <=512 )return 9 ; +if (size <=1024 )return 10 ; +if (size <=2 *1024 )return 11 ;
include/linux/slqb_def.h line 209
Quoted 10 lines
include/linux/slqb_def.h new file +10
-0+if (size <=4 *1024 )return 12 ; +if (size <=8 *1024 )return 13 ; +if (size <=16 *1024 )return 14 ; +if (size <=32 *1024 )return 15 ; +if (size <=64 *1024 )return 16 ; +if (size <=128 *1024 )return 17 ; +if (size <=256 *1024 )return 18 ; +if (size <=512 *1024 )return 19 ; +if (size <=1024 *1024 )return 20 ; +if (size <=2 *1024 *1024 )return 21 ;
Have you looked into other binsizes? iirc the original slab paper mentioned that power of two is usually not the best.
Quoted 4 lines
include/linux/slqb_def.h new file +3
-0+return -1 ; +} +
include/linux/slqb_def.h line 222
Quoted 10 lines
include/linux/slqb_def.h new file +10
-0+#ifdef CONFIG_ZONE_DMA +#define SLQB_DMA __GFP_DMA +#else +/* Disable "DMA slabs" */ +#define SLQB_DMA (__force gfp_t)0 +#endif + +/* +* Find the kmalloc slab cache for a given combination of allocation flags and +* size.
You should mention that this would be a very bad idea to call for !__builtin_constant_p(size)
include/linux/slqb_def.h line 234
Quoted 12 lines
+ */
include/linux/slqb_def.h new file +11
-0+static __always_inline struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags) +{ +int index =kmalloc_index (size); + +if (unlikely (index ==0 )) +return NULL ; + +if (likely (!(flags &SLQB_DMA ))) +return &kmalloc_caches[index]; +else +return &kmalloc_caches_dma[index];
BTW i had an old patchkit to kill all GFP_DMA slab users. Perhaps should warm that up again. That would lower the inline footprint.
include/linux/slqb_def.h line 268
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+#ifdef CONFIG_NUMA +void *__kmalloc_node (size_t size,gfp_t flags,int node); +void *kmem_cache_alloc_node (struct kmem_cache *,gfp_t flags,int node); + +static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
kmalloc_node should be infrequent, i suspect it can be safely out of lined.
Quoted 2 lines
+ * - investiage performance with memoryless nodes. Perhaps CPUs can be given
+ * a default closest home node via which it can use fastpath functions.
FWIW that is what x86-64 always did. Perhaps you can just fix ia64 to do that too and be happy.
mm/slqb.c line 38
Quoted 7 lines
+ * Perhaps it is not a big problem.
+ */mm/slqb.c new file +5
-0+ +/* +* slqb_page overloads struct page, and is used to manage some slob allocation +* aspects, however to avoid the horrible mess in include/linux/mm_types.h, +* we'll just define our own struct slqb_page type variant here.
Hopefully this works for the crash dumpers. Do they have a way to distingush slub/slqb/slab kernels with different struct page usage?
mm/slqb.c line 66
Quoted 8 lines
mm/slqb.c new file +8
-0+#define PG_SLQB_BIT (1 << PG_slab) + +static int kmem_size__read_mostly ; +#ifdef CONFIG_NUMA +static int numa_platform__read_mostly ; +#else +#define numa_platform 0 +#endif
It would be cheaper if you put that as a flag into the kmem_caches flags, this way you avoid an additional cache line touched.
mm/slqb.c line 141
Quoted 4 lines
mm/slqb.c new file +4
-0+static inline int slqb_page_to_nid (struct slqb_page *page) +{ +return page_to_nid (&page->page); +}
etc. you got a lot of wrappers...
mm/slqb.c line 178
Quoted 9 lines
mm/slqb.c new file +9
-0+static inline struct slqb_page *alloc_slqb_pages_node (int nid,gfp_t flags, +unsigned int order) +{ +struct page *p; + +if (nid ==-1 ) + p =alloc_pages (flags, order); +else + p =alloc_pages_node (nid, flags, order);
alloc_pages_nodes does that check anyways.
mm/slqb.c line 230
Quoted 5 lines
mm/slqb.c new file +5
-0+/* Not all arches define cache_line_size */ +#ifndef cache_line_size +#define cache_line_size () L1_CACHE_BYTES +#endif +
They should. better fix them?
mm/slqb.c line 824
Quoted 4 lines
mm/slqb.c new file +4
-0+ +/* +* Determine which debug features should be switched on +*/
It would be nicer if you could use long options. At least for me that would increase the probability that I could remember them without having to look them up.
Quoted 15 lines
mm/slqb.c new file +15
-0+/* +* Allocate a new slab, set up its object list. +*/ +static struct slqb_page *new_slab_page (struct kmem_cache *s,gfp_t flags,int node,unsigned int colour) +{ +struct slqb_page *page; +void *start; +void *last; +void *p; + +BUG_ON (flags &GFP_SLAB_BUG_MASK ); + + page =allocate_slab (s, + flags & (GFP_RECLAIM_MASK |GFP_CONSTRAINT_MASK ), node); +if (!page)
mm/slqb.c line 951
Quoted 10 lines
mm/slqb.c new file +10
-0+ goto out; + + page->flags |=PG_SLQB_BIT ; + + start =page_address (&page->page); + +if (unlikely (slab_poison (s))) +memset (start,POISON_INUSE ,PAGE_SIZE << s->order); + + start += colour;
One thing i was wondering. Did you try to disable the colouring and see if it makes much difference on modern systems? They tend to have either larger caches or higher associativity caches.
Or perhaps it could be made optional based on CPU type?
Quoted 11 lines
mm/slqb.c new file +11
-0+static noinline void *__slab_alloc_page(struct kmem_cache *s, gfp_t gfpflags, int node) +{ +struct slqb_page *page; +struct kmem_cache_list *l; +struct kmem_cache_cpu *c; +unsigned int colour; +void *object; + + c =get_cpu_slab (s,smp_processor_id ()); + colour = c->colour_next; + c->colour_next += s->colour_off;
mm/slqb.c line 1267
Quoted 10 lines
mm/slqb.c new file +10
-0+if (c->colour_next >= s->colour_range) + c->colour_next =0 ; + +/* XXX: load any partial? */ + +/* Caller handles __GFP_ZERO */ + gfpflags &= ~__GFP_ZERO; + +if (gfpflags & __GFP_WAIT) +local_irq_enable ();
At least on P4 you could get some win by avoiding the local_irq_save() up in the fast path when __GFP_WAIT is set (because storing the eflags is very expensive there)
mm/slqb.c line 1406
Quoted 7 lines
mm/slqb.c new file +7
-0+ +again: +local_irq_save (flags); + object =__slab_alloc (s, gfpflags, node); +local_irq_restore (flags); + +if (unlikely (slab_debug (s)) &&likely (object)) {
AFAIK gcc cannot process multiple likelys in a single condition.
Quoted 11 lines
mm/slqb.c new file +11
-0+/* Initial slabs */ +#ifdef CONFIG_SMP +static struct kmem_cache_cpu kmem_cache_cpus[NR_CPUS ]; +#endif +#ifdef CONFIG_NUMA +static struct kmem_cache_node kmem_cache_nodes[MAX_NUMNODES ]; +#endif + +#ifdef CONFIG_SMP +static struct kmem_cache kmem_cpu_cache; +static struct kmem_cache_cpu kmem_cpu_cpus[NR_CPUS ];
mm/slqb.c line 1757
Quoted 10 lines
mm/slqb.c new file +10
-0+#ifdef CONFIG_NUMA +static struct kmem_cache_node kmem_cpu_nodes[MAX_NUMNODES ]; +#endif +#endif + +#ifdef CONFIG_NUMA +static struct kmem_cache kmem_node_cache; +static struct kmem_cache_cpu kmem_node_cpus[NR_CPUS ]; +static struct kmem_cache_node kmem_node_nodes[MAX_NUMNODES ]; +#endif
That all needs fixing too of course.
Quoted 9 lines
mm/slqb.c new file +9
-0+ +#ifdef CONFIG_SMP +static struct kmem_cache_cpu *alloc_kmem_cache_cpu (struct kmem_cache *s,int cpu) +{ +struct kmem_cache_cpu *c; + + c =kmem_cache_alloc_node (&kmem_cpu_cache,GFP_KERNEL ,cpu_to_node (cpu)); +if (!c) +return NULL ;
mm/slqb.c line 1776
Quoted 10 lines
mm/slqb.c new file +10
-0+ +init_kmem_cache_cpu (s, c); +return c; +} + +static void free_kmem_cache_cpus (struct kmem_cache *s) +{ +int cpu; + +for_each_online_cpu (cpu) {
Is this protected against racing cpu hotplugs? Doesn't look like it. Multiple occurrences.
mm/slqb.c line 2401
Quoted 9 lines
mm/slqb.c new file +9
-0+static void cache_trim_worker (struct work_struct *w) +{ +struct delayed_work *work = + container_of(w, struct delayed_work, work); +struct kmem_cache *s; +int node; + +if (!down_read_trylock (&slqb_lock)) + goto out;
No counter for this?
Quoted 8 lines
mm/slqb.c new file +8
-0+ +/* +* We are bringing a node online. No memory is availabe yet. We must +* allocate a kmem_cache_node structure in order to bring the node +* online. +*/ +down_read (&slqb_lock); +list_for_each_entry (s, &slab_caches, list) {
mm/slqb.c line 2518
Quoted 10 lines
mm/slqb.c new file +10
-0+/* +* XXX: kmem_cache_alloc_node will fallback to other nodes +* since memory is not yet available from the node that +* is brought up. +*/ +if (s->node[nid])/* could be lefover from last online */ +continue ; + n =kmem_cache_alloc (&kmem_node_cache,GFP_KERNEL ); +if (!n) { + ret = -ENOMEM ;
Surely that should panic? I don't think a slab less node will be very useful later.
Quoted 22 lines
mm/slqb.c new file +22
-0+#ifdef CONFIG_SLQB_SYSFS +/* +* sysfs API +*/ +#define to_slab_attr (n) container_of(n, struct slab_attribute, attr) +#define to_slab (n) container_of(n, struct kmem_cache, kobj); + +struct slab_attribute { +struct attribute attr; +ssize_t (*show)(struct kmem_cache *s,char *buf); +ssize_t (*store)(struct kmem_cache *s,const char *x,size_t count); +}; + +#define SLAB_ATTR_RO (_name) \ + static struct slab_attribute _name##_attr = __ATTR_RO(_name) + +#define SLAB_ATTR (_name) \ + static struct slab_attribute _name##_attr = \ + __ATTR(_name, 0644, _name##_show, _name##_store) + +static ssize_t slab_size_show (struct kmem_cache *s,char *buf) +{
mm/slqb.c line 3076
Quoted 10 lines
mm/slqb.c new file +10
-0+return sprintf (buf,"%d\n" , s->size); +} +SLAB_ATTR_RO (slab_size); + +static ssize_t align_show (struct kmem_cache *s,char *buf) +{ +return sprintf (buf,"%d\n" , s->align); +} +SLAB_ATTR_RO (align); +
When you map back to the attribute you can use a index into a table for the field, saving that many functions?
Quoted 11 lines
mm/slqb.c new file +11
-0+#define STAT_ATTR (si, text) \ +static ssize_t text##_show(struct kmem_cache *s, char *buf) \ +{ \ + return show_stat(s, buf, si); \ +} \ +SLAB_ATTR_RO(text); \ + +STAT_ATTR (ALLOC , alloc); +STAT_ATTR (ALLOC_SLAB_FILL , alloc_slab_fill); +STAT_ATTR (ALLOC_SLAB_NEW , alloc_slab_new); +STAT_ATTR (FREE , free);
mm/slqb.c line 3263
Quoted 10 lines
mm/slqb.c new file +10
-0+STAT_ATTR (FREE_REMOTE , free_remote); +STAT_ATTR (FLUSH_FREE_LIST , flush_free_list); +STAT_ATTR (FLUSH_FREE_LIST_OBJECTS , flush_free_list_objects); +STAT_ATTR (FLUSH_FREE_LIST_REMOTE , flush_free_list_remote); +STAT_ATTR (FLUSH_SLAB_PARTIAL , flush_slab_partial); +STAT_ATTR (FLUSH_SLAB_FREE , flush_slab_free); +STAT_ATTR (FLUSH_RFREE_LIST , flush_rfree_list); +STAT_ATTR (FLUSH_RFREE_LIST_OBJECTS , flush_rfree_list_objects); +STAT_ATTR (CLAIM_REMOTE_LIST , claim_remote_list); +STAT_ATTR (CLAIM_REMOTE_LIST_OBJECTS , claim_remote_list_objects);
This really should be table driven, shouldn't it? That would give much smaller code.
-Andi
Signature · 7 lines
[email protected] -- Speaking for myself only. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi Andi,
Quoted 22 lines · Andi Kleen
Quoted 22 lines
include/linux/slqb_def.h new file +22
-0+#if L1_CACHE_BYTES <64 +if (size >64 && size <=96 ) +return 1 ; +#endif +#if L1_CACHE_BYTES <128 +if (size >128 && size <=192 ) +return 2 ; +#endif +if (size <=8 )return 3 ; +if (size <=16 )return 4 ; +if (size <=32 )return 5 ; +if (size <=64 )return 6 ; +if (size <=128 )return 7 ; +if (size <=256 )return 8 ; +if (size <=512 )return 9 ; +if (size <=1024 )return 10 ; +if (size <=2 *1024 )return 11 ; +if (size <=4 *1024 )return 12 ; +if (size <=8 *1024 )return 13 ; +if (size <=16 *1024 )return 14 ; +if (size <=32 *1024 )return 15 ; +if (size <=64 *1024 )return 16 ;
Quoted 8 lines
include/linux/slqb_def.h line 209
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+if (size <=128 *1024 )return 17 ; +if (size <=256 *1024 )return 18 ; +if (size <=512 *1024 )return 19 ; +if (size <=1024 *1024 )return 20 ; +if (size <=2 *1024 *1024 )return 21 ;Have you looked into other binsizes? iirc the original slab paper mentioned that power of two is usually not the best.
Judging by the limited boot-time testing I've done with kmemtrace, the bulk of kmalloc() allocations are under 64 bytes or so and actually a pretty ok fit with the current sizes. The badly fitting objects are usually very big and of different sizes (so they won't share a cache easily) so I'm not expecting big gains from non-power of two sizes.
Pekka
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Andi Kleen
Nick Piggin <[email protected]> writes:
Not a full review, just some things i noticed.
The code is very readable thanks (that's imho the main reason
slab.cshould go btw, it's really messy and hard to get through)
Thanks, appreciated. It is very helpful.
Quoted 8 lines
Quoted 7 lines
Using lists rather than arrays can reduce the cacheline footprint. When moving objects around, SLQB can move a list of objects from one CPU to another by simply manipulating a head pointer, wheras SLAB needs to memcpy arrays. Some SLAB per-CPU arrays can be up to 1K in size, which is a lot of cachelines that can be touched during alloc/free. Newly freed objects tend to be cache hot, and newly allocated ones tend to soon be touched anyway, so often there is little cost to using metadata in the objects.
Quoted 5 lines
You're probably aware of that, but the obvious counter argument is that for manipulating a single object a double linked list will always require touching three cache lines (prev, current, next), while an array access only a single one. A possible alternative would be a list of shorter arrays.
That's true, but SLQB doesn't use double linked lists, but single. An allocation needs to load a "head" pointer to the first object, then load a "next" pointer from that object and assign it to "head". The 2nd load touches memory which should be subsequently touched bythe caller anyway. A free just has to assign a pointer in the to-be-freed object to point to the old head, and then update the head to the new object. So this 1st touch should usually be cache hot memory.
But yes there are situations where SLAB scheme could result in fewer cache misses. I haven't yet noticed it is a problem.
Quoted 8 lines
include/linux/slqb_def.h line 139
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+const char *name;/* Name (only for display!) */ +struct list_head list;/* List of slab caches */ + +int align;/* Alignment */ +int inuse;/* Offset to metadata */ I suspect some of these fields could be short or char (E.g. alignment), possibly lowering cache line impact.
Good point. I'll have to do a pass through all structures and make sure sizes and alignments etc are optimal. I have somewhat ordered it eg. so that LIFO freelist allocations only have to touch the first few fields in structures, then partial page list allocations touch the next few, then page allocator etc.
But that might have gone out of date a little bit.
Quoted 14 lines
include/linux/slqb_def.h line 148
Quoted 8 lines
include/linux/slqb_def.h new file +8
-0+#ifdef CONFIG_SLQB_SYSFS +struct kobject kobj;/* For sysfs */ +#endif +#ifdef CONFIG_NUMA +struct kmem_cache_node *node[MAX_NUMNODES ]; +#endif +#ifdef CONFIG_SMP +struct kmem_cache_cpu *cpu_slab[NR_CPUS ];Those both really need to be dynamically allocated, otherwise it wastes a lot of memory in the common case
(e.g. NR_CPUS==128 kernel on dual core system). And of course on the proposed NR_CPUS==4096 kernels it becomes prohibitive.
Quoted 2 lines
You could use
alloc_percpu? There's noalloc_pernodeunfortunately, perhaps there should be one.
cpu_slab is dynamically allocated, by just changing the size of the kmem_cache cache at boot time. Probably the best way would be to have dynamic cpu and node allocs for them, I agree.
Any plans for an alloc_pernode?
Quoted 4 lines
include/linux/slqb_def.h line 209
Quoted 1 line
include/linux/slqb_def.h new file +1
-0+if (size <=2 *1024 *1024 )return 21 ;Have you looked into other binsizes? iirc the original slab paper mentioned that power of two is usually not the best.
No I haven't. Although I have been spending most effort at this point just to improve SLQB versus the other allocators without changing things like this. But it would be fine to investigate when SLQB is more mature or for somebody else to look at it.
Quoted 5 lines
Quoted 3 lines
+/*
+ * Find the kmalloc slab cache for a given combination of allocation flags and
+ * size.You should mention that this would be a very bad idea to call for !__builtin_constant_p(size)
OK. It's not meant to be used outside slqb_def.h, however.
Quoted 6 lines
Quoted 6 lines
include/linux/slqb_def.h new file +6
-0+static __always_inline struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags) +{ +int index =kmalloc_index (size); + +if (unlikely (index ==0 )) +return NULL ;
Quoted 8 lines
include/linux/slqb_def.h line 234
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+ +if (likely (!(flags &SLQB_DMA ))) +return &kmalloc_caches[index]; +else +return &kmalloc_caches_dma[index];BTW i had an old patchkit to kill all
GFP_DMAslab users. Perhaps should warm that up again. That would lower the inline footprint.
That would be excellent. It would also reduce constant data overheads for SLAB and SLQB, and some nasty code from SLUB.
Quoted 7 lines
include/linux/slqb_def.h line 268
Quoted 5 lines
include/linux/slqb_def.h new file +5
-0+#ifdef CONFIG_NUMA +void *__kmalloc_node (size_t size,gfp_t flags,int node); +void *kmem_cache_alloc_node (struct kmem_cache *,gfp_t flags,int node); + +static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
kmalloc_nodeshould be infrequent, i suspect it can be safely out of lined.
Hmm... I wonder how much it increases code size...
Quoted 5 lines
Quoted 2 lines
+ * - investiage performance with memoryless nodes. Perhaps CPUs can be given
+ * a default closest home node via which it can use fastpath functions.FWIW that is what x86-64 always did. Perhaps you can just fix ia64 to do that too and be happy.
What if the node is possible but not currently online?
Quoted 5 lines
Quoted 2 lines
+ * aspects, however to avoid the horrible mess in
include/linux/mm_types.h,
+ * we'll just define our own structslqb_pagetype variant here.Hopefully this works for the crash dumpers. Do they have a way to distingush slub/slqb/slab kernels with different struct page usage?
Beyond looking at configs or hacks like looking at symbols, I don't think so... It probably should go into vermagic I guess.
Quoted 11 lines
mm/slqb.c line 66
Quoted 8 lines
mm/slqb.c new file +8
-0+#define PG_SLQB_BIT (1 << PG_slab) + +static int kmem_size__read_mostly ; +#ifdef CONFIG_NUMA +static int numa_platform__read_mostly ; +#else +#define numa_platform 0 +#endif It would be cheaper if you put that as a flag into the
kmem_cachesflags, this way you avoid an additional cache line touched.
Ok, that works.
Quoted 6 lines
mm/slqb.c line 141
Quoted 4 lines
mm/slqb.c new file +4
-0+static inline int slqb_page_to_nid (struct slqb_page *page) +{ +return page_to_nid (&page->page); +}etc. you got a lot of wrappers...
I think they're not too bad though.
Quoted 11 lines
mm/slqb.c line 178
Quoted 9 lines
mm/slqb.c new file +9
-0+static inline struct slqb_page *alloc_slqb_pages_node (int nid,gfp_t flags, +unsigned int order) +{ +struct page *p; + +if (nid ==-1 ) + p =alloc_pages (flags, order); +else + p =alloc_pages_node (nid, flags, order);
alloc_pages_nodesdoes that check anyways.
OK, I rip out that wrapper completely.
Quoted 7 lines
mm/slqb.c line 230
Quoted 5 lines
mm/slqb.c new file +5
-0+/* Not all arches define cache_line_size */ +#ifndef cache_line_size +#define cache_line_size () L1_CACHE_BYTES +#endif +They should. better fix them?
git grep -l -e cache_line_size arch/ | egrep '\.h$'
Only ia64, mips, powerpc, sparc, x86...
Quoted 7 lines
mm/slqb.c line 824
Quoted 3 lines
mm/slqb.c new file +3
-0+/* +* Determine which debug features should be switched on +*/ It would be nicer if you could use long options. At least for me that would increase the probability that I could remember them without having to look them up.
I haven't looked closely at the debug code which is mostly straight out of SLUB and minimal changes to get it working. Of course it is very important, but useless if the core allocator isn't good. I also don't want to diverge from SLUB in these areas if possible until we reduce the number of allocators in the tree...
Long options is probably not a bad idea, though.
Quoted 8 lines
mm/slqb.c line 951
Quoted 4 lines
mm/slqb.c new file +4
-0+if (unlikely (slab_poison (s))) +memset (start,POISON_INUSE ,PAGE_SIZE << s->order); + + start += colour;One thing i was wondering. Did you try to disable the colouring and see if it makes much difference on modern systems? They tend to have either larger caches or higher associativity caches.
I have tried, but I don't think I found a test where it made a statistically significant difference. It is not very costly to implement, though.
Quoted 1 line
Or perhaps it could be made optional based on CPU type?
It could easily be changed, yes.
Quoted 8 lines
mm/slqb.c line 1404
Quoted 5 lines
mm/slqb.c new file +5
-0+ +again: +local_irq_save (flags); + object =__slab_alloc (s, gfpflags, node); +local_irq_restore (flags);At least on P4 you could get some win by avoiding the
local_irq_save()up in the fast path when__GFP_WAITis set (because storing the eflags is very expensive there)
That's a good point, although also something trivially applicable to all allocators and as such I prefer not to add such differences to the SLQB patch if we are going into an evaluation phase.
Quoted 15 lines
Quoted 15 lines
mm/slqb.c new file +15
-0+/* Initial slabs */ +#ifdef CONFIG_SMP +static struct kmem_cache_cpu kmem_cache_cpus[NR_CPUS ]; +#endif +#ifdef CONFIG_NUMA +static struct kmem_cache_node kmem_cache_nodes[MAX_NUMNODES ]; +#endif + +#ifdef CONFIG_SMP +static struct kmem_cache kmem_cpu_cache; +static struct kmem_cache_cpu kmem_cpu_cpus[NR_CPUS ]; +#ifdef CONFIG_NUMA +static struct kmem_cache_node kmem_cpu_nodes[MAX_NUMNODES ]; +#endif +#endif
Quoted 8 lines
mm/slqb.c line 1757
Quoted 6 lines
mm/slqb.c new file +6
-0+ +#ifdef CONFIG_NUMA +static struct kmem_cache kmem_node_cache; +static struct kmem_cache_cpu kmem_node_cpus[NR_CPUS ]; +static struct kmem_cache_node kmem_node_nodes[MAX_NUMNODES ]; +#endif That all needs fixing too of course.
Hmm. I was hoping it could stay simple as it is just a static constant (for a given NR_CPUS) overhead. I wonder if bootmem is still up here? How fine grained is it these days?
Could bite the bullet and do a multi-stage bootstap like SLUB, but I want to try avoiding that (but init code is also of course much less important than core code and total overheads).
Quoted 7 lines
mm/slqb.c line 1776
Quoted 5 lines
mm/slqb.c new file +5
-0+static void free_kmem_cache_cpus (struct kmem_cache *s) +{ +int cpu; + +for_each_online_cpu (cpu) {Is this protected against racing cpu hotplugs? Doesn't look like it. Multiple occurrences.
I think you're right.
Quoted 11 lines
mm/slqb.c line 2401
Quoted 9 lines
mm/slqb.c new file +9
-0+static void cache_trim_worker (struct work_struct *w) +{ +struct delayed_work *work = + container_of(w, struct delayed_work, work); +struct kmem_cache *s; +int node; + +if (!down_read_trylock (&slqb_lock)) + goto out;No counter for this?
It's quite unimportant. It will only race with creating or destroying actual kmem caches, and cache trimming is infrequent too.
Quoted 7 lines
Quoted 7 lines
mm/slqb.c new file +7
-0+down_read (&slqb_lock); +list_for_each_entry (s, &slab_caches, list) { +/* +* XXX: kmem_cache_alloc_node will fallback to other nodes +* since memory is not yet available from the node that +* is brought up. +*/
Quoted 8 lines
mm/slqb.c line 2518
Quoted 5 lines
mm/slqb.c new file +5
-0+if (s->node[nid])/* could be lefover from last online */ +continue ; + n =kmem_cache_alloc (&kmem_node_cache,GFP_KERNEL ); +if (!n) { + ret = -ENOMEM ;Surely that should panic? I don't think a slab less node will be very useful later.
Returning error here I think will just fail the online operation? Better than a panic :)
Quoted 10 lines
mm/slqb.c line 3076
Quoted 6 lines
mm/slqb.c new file +6
-0+static ssize_t align_show (struct kmem_cache *s,char *buf) +{ +return sprintf (buf,"%d\n" , s->align); +} +SLAB_ATTR_RO (align); +When you map back to the attribute you can use a index into a table for the field, saving that many functions?
Quoted 5 lines
mm/slqb.c line 3263
Quoted 2 lines
mm/slqb.c new file +2
-0+STAT_ATTR (CLAIM_REMOTE_LIST , claim_remote_list); +STAT_ATTR (CLAIM_REMOTE_LIST_OBJECTS , claim_remote_list_objects);This really should be table driven, shouldn't it? That would give much smaller code.
Tables probably would help. I will keep it close to SLUB for now, though.
Thanks,
Nick
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 14 lines · Nick Piggin
Quoted 14 lines
include/linux/slqb_def.h line 148
Quoted 8 lines
include/linux/slqb_def.h new file +8
-0+#ifdef CONFIG_SLQB_SYSFS +struct kobject kobj;/* For sysfs */ +#endif +#ifdef CONFIG_NUMA +struct kmem_cache_node *node[MAX_NUMNODES ]; +#endif +#ifdef CONFIG_SMP +struct kmem_cache_cpu *cpu_slab[NR_CPUS ];Those both really need to be dynamically allocated, otherwise it wastes a lot of memory in the common case
(e.g. NR_CPUS==128 kernel on dual core system). And of course on the proposed NR_CPUS==4096 kernels it becomes prohibitive.
Quoted 5 lines
Quoted 2 lines
You could use
alloc_percpu? There's noalloc_pernodeunfortunately, perhaps there should be one.
cpu_slabis dynamically allocated, by just changing the size of thekmem_cachecache at boot time.
You'll always have at least the MAX_NUMNODES waste because you cannot tell the compiler that the cpu_slab field has moved.
Quoted 2 lines
Probably the best way would
be to have dynamic cpu and node allocs for them, I agree.
It's really needed.
Quoted 1 line
Any plans for an
alloc_pernode?
It shouldn't be very hard to implement. Or do you ask if I'm volunteering? @)
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
+ * - investiage performance with memoryless nodes. Perhaps CPUs can be given
+ * a default closest home node via which it can use fastpath functions.FWIW that is what x86-64 always did. Perhaps you can just fix ia64 to do that too and be happy.
What if the node is possible but not currently online?
Nobody should allocate on it then.
Quoted 11 lines
Quoted 7 lines
mm/slqb.c line 230
Quoted 5 lines
mm/slqb.c new file +5
-0+/* Not all arches define cache_line_size */ +#ifndef cache_line_size +#define cache_line_size () L1_CACHE_BYTES +#endif +They should. better fix them?
git grep -l -e
cache_line_sizearch/| egrep '\.h$'Only ia64, mips, powerpc, sparc, x86...
It's straight forward to that define everywhere.
Quoted 10 lines
Quoted 9 lines
mm/slqb.c line 951
Quoted 5 lines
mm/slqb.c new file +4
-0+if (unlikely (slab_poison (s))) +memset (start,POISON_INUSE ,PAGE_SIZE << s->order); + + start += colour;One thing i was wondering. Did you try to disable the colouring and see if it makes much difference on modern systems? They tend to have either larger caches or higher associativity caches.
Quoted 3 lines
I have tried, but I don't think I found a test where it made a statistically significant difference. It is not very costly to implement, though.
how about the memory usage?
also this is all so complicated already that every simplification helps.
Quoted 10 lines
Quoted 9 lines
mm/slqb.c line 1757
Quoted 7 lines
mm/slqb.c new file +7
-0+#endif + +#ifdef CONFIG_NUMA +static struct kmem_cache kmem_node_cache; +static struct kmem_cache_cpu kmem_node_cpus[NR_CPUS ]; +static struct kmem_cache_node kmem_node_nodes[MAX_NUMNODES ]; +#endif That all needs fixing too of course.
Quoted 2 lines
Hmm. I was hoping it could stay simple as it is just a static constant (for a given
NR_CPUS) overhead.
The issue is that distro kernels typically run with NR_CPUS >>> num_possible_cpus() And we'll see likely higher NR_CPUS (and MAX_NUMNODES) in the future, but also still want to run the same kernels on really small systems (e.g. Atom based) without wasting their memory.
So for anything NR_CPUS you should use per_cpu data -- that is correctly sized automatically.
For MAX_NUMNODES we don't have anything equivalent currently, so you would also need alloc_pernode() I guess.
Ok you can just use per cpu for them too and only use the first entry in each node. That's cheating, but not too bad.
Quoted 1 line
I wonder if bootmem is still up here?
bootmem is finished when slab comes up.
Quoted 4 lines
Could bite the bullet and do a multi-stage bootstap like SLUB, but I want to try avoiding that (but init code is also of course much less important than core code and total overheads).
For DEFINE_PER_CPU you don't need special allocation.
Probably want a DEFINE_PER_NODE() for this or see above.
Quoted 11 lines
Quoted 11 lines
mm/slqb.c line 3076
Quoted 7 lines
mm/slqb.c new file +6
-0+static ssize_t align_show (struct kmem_cache *s,char *buf) +{ +return sprintf (buf,"%d\n" , s->align); +} +SLAB_ATTR_RO (align); +When you map back to the attribute you can use a index into a table for the field, saving that many functions?
Quoted 8 lines
Quoted 5 lines
mm/slqb.c line 3263
Quoted 2 lines
mm/slqb.c new file +2
-0+STAT_ATTR (CLAIM_REMOTE_LIST , claim_remote_list); +STAT_ATTR (CLAIM_REMOTE_LIST_OBJECTS , claim_remote_list_objects);This really should be table driven, shouldn't it? That would give much smaller code.
Tables probably would help. I will keep it close to SLUB for now, though.
Hmm, then fix slub?
-Andi
Signature · 7 lines
[email protected] -- Speaking for myself only. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 18 lines · Andi Kleen
Quoted 17 lines · Nick Piggin
Quoted 16 lines
Review
Quoted 8 lines
(quoted diff) +8
-0+#ifdef CONFIG_SLQB_SYSFS + struct kobject kobj; /* For sysfs */ +#endif +#ifdef CONFIG_NUMA + struct kmem_cache_node *node[MAX_NUMNODES]; +#endif +#ifdef CONFIG_SMP + struct kmem_cache_cpu *cpu_slab[NR_CPUS];Those both really need to be dynamically allocated, otherwise it wastes a lot of memory in the common case
(e.g. NR_CPUS==128 kernel on dual core system). And of course on the proposed NR_CPUS==4096 kernels it becomes prohibitive.You could use
alloc_percpu? There's noalloc_pernodeunfortunately, perhaps there should be one.
Quoted 6 lines
Quoted 2 lines
cpu_slabis dynamically allocated, by just changing the size of thekmem_cachecache at boot time.You'll always have at least the
MAX_NUMNODESwaste because you cannot tell the compiler that thecpu_slabfield has moved.
Right. It could go into a completely different per-cpu structure if needed to work around that (using node is a relatively rare operation). But an alloc_pernode would be nicer.
Quoted 8 lines
Quoted 2 lines
Probably the best way would
be to have dynamic cpu and node allocs for them, I agree.It's really needed.
Quoted 1 line
Any plans for an
alloc_pernode?It shouldn't be very hard to implement. Or do you ask if I'm volunteering? @)
Just if you knew about plans. I won't get too much time to work on it next week, so I hope to have something in slab tree in the meantime. I think it is OK to leave now, with a mind to improving it before a possible mainline merge (there will possibly be more serious issues discovered anyway).
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
+ * - investiage performance with memoryless nodes. Perhaps CPUs can be given
+ * a default closest home node via which it can use fastpath functions.FWIW that is what x86-64 always did. Perhaps you can just fix ia64 to do that too and be happy.
What if the node is possible but not currently online?
Nobody should allocate on it then.
But then it goes online and what happens? Your numa_node_id() changes? How does that work? Or you mean x86-64 does not do that same trick for possible but offline nodes?
Quoted 5 lines
Quoted 3 lines
git grep -l -e
cache_line_sizearch/| egrep '\.h$'Only ia64, mips, powerpc, sparc, x86...
It's straight forward to that define everywhere.
OK, but this code is just copied straight from SLAB... I don't want to add such dependency at this point I'm trying to get something reasonable to merge. But it would be a fine cleanup.
Quoted 11 lines
Quoted 7 lines
Quoted 3 lines
One thing i was wondering. Did you try to disable the colouring and see if it makes much difference on modern systems? They tend to have either larger caches or higher associativity caches.
I have tried, but I don't think I found a test where it made a statistically significant difference. It is not very costly to implement, though.
how about the memory usage?
also this is all so complicated already that every simplification helps.
Oh, it only uses slack space in the slabs as such, so it should be almost zero cost. I tried testing extra colour at the cost of space, but no obvious difference there either. But I think I'll leave in the code because it might be a win for some embedded or unusual CPUs.
Quoted 7 lines
Quoted 3 lines
Could bite the bullet and do a multi-stage bootstap like SLUB, but I want to try avoiding that (but init code is also of course much less important than core code and total overheads).
For
DEFINE_PER_CPUyou don't need special allocation.Probably want a
DEFINE_PER_NODE()for this or see above.
Ah yes DEFINE_PER_CPU of course. Not quite correct for per-node data, but it should be good enough for wider testing in linux-next.
Quoted 4 lines
Quoted 2 lines
Tables probably would help. I will keep it close to SLUB for now, though.
Hmm, then fix slub?
That's my plan, but I go about it a different way ;) I don't want to spend too much time on other allocators or cleanup etc code too much right now (except cleanups in SLQB, which of course is required).
Here is an incremental patch for your review points. Thanks very much, it's a big improvement (getting rid of those static arrays vastly decreases memory consumption with bigger NR_CPUs, so that's a good start; will need to investigate alloc_percpu / pernode etc, but that may have to wait until next week.
--- include/linux/slab.h | 4 + include/linux/slqb_def.h | 10 +++ mm/slqb.c | 125 ++++++++++++++++++++++++++--------------------- 3 files changed, 82 insertions(+), 57 deletions(-)
include/linux/slab.h +4 -0
@@ -65,6 +65,10 @@/* The following flags affect the page allocator grouping pages by mobility */ #define SLAB_RECLAIM_ACCOUNT 0x00020000UL/* Objects are reclaimable */ #define SLAB_TEMPORARY SLAB_RECLAIM_ACCOUNT/* Objects are short-lived */ + +/* Following flags should only be used by allocator specific flags */ +#define SLAB_ALLOC_PRIVATE 0x000000ffUL +/* * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests. *
include/linux/slqb_def.h +8 -2
@@ -15,6 +15,8 @@#include <linux/kernel.h> #include <linux/kobject.h> +#define SLAB_NUMA 0x00000001UL/* shortcut */ +enum stat_item {ALLOC ,/* Allocation count */ ALLOC_SLAB_FILL ,/* Fill freelist from page list */
@@ -224,12 +226,16 @@ static __always_inline int kmalloc_index/* * Find the kmalloc slab cache for a given combination of allocation flags and -* size. +* size. Should really only be used for constant 'size' arguments, due to +* bloat. */ static __always_inline struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags) { -int index =kmalloc_index (size); +int index; + +BUILD_BUG_ON (!__builtin_constant_p (size)); + index =kmalloc_index (size);if (unlikely (index ==0 ))return NULL ;
mm/slqb.c +70 -55
289 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
[dropping lameters' outdated address]
Quoted 11 lines · Nick Piggin
Quoted 9 lines
Quoted 3 lines
Probably the best way would
be to have dynamic cpu and node allocs for them, I agree.It's really needed.
Quoted 1 line
Any plans for an
alloc_pernode?It shouldn't be very hard to implement. Or do you ask if I'm volunteering? @)
Just if you knew about plans. I won't get too much time to work on
Not aware of anyone working on it.
Quoted 2 lines
it next week, so I hope to have something in slab tree in the meantime. I think it is OK to leave now, with a mind to improving
Sorry, the NR_CPUS/MAX_NUMNODE arrays are a merge blocker imho because they explode with CONFIG_MAXSMP.
Quoted 2 lines
it before a possible mainline merge (there will possibly be more serious issues discovered anyway).
I see you fixed the static arrays.
Doing the same for the kmem_cache arrays with making them a pointer and then using num_possible_{cpus,nodes}() would seem straight forward, wouldn't it?
Although I think I would prefer alloc_percpu, possibly with per_cpu_ptr(first_cpu(node_to_cpumask(node)), ...)
Quoted 11 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
+ * - investiage performance with memoryless nodes. Perhaps CPUs can be given
+ * a default closest home node via which it can use fastpath functions.FWIW that is what x86-64 always did. Perhaps you can just fix ia64 to do that too and be happy.
What if the node is possible but not currently online?
Nobody should allocate on it then.
But then it goes online and what happens?
You already have a node online notifier that should handle that then, don't you?
x86-64 btw currently doesn't support node hotplug (but I expect it will be added at some point), but it should be ok even on architectures that do.
Quoted 1 line
Your
numa_node_id()changes?
What do you mean?
Quoted 2 lines
How does that work? Or you mean x86-64 does not do that same trick for possible but offline nodes?
All I'm saying is that when x86-64 finds a memory less node it assigns its CPUs to other nodes. Hmm ok perhaps there's a backdoor when someone sets it with kmalloc_node() but that should normally not happen I think.
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
git grep -l -e
cache_line_sizearch/| egrep '\.h$'Only ia64, mips, powerpc, sparc, x86...
It's straight forward to that define everywhere.
OK, but this code is just copied straight from SLAB... I don't want to add such dependency at this point I'm trying to get something
I'm sure such a straight forward change could be still put into .29
Quoted 1 line
reasonable to merge. But it would be a fine cleanup.
Hmm to be honest it's a little weird to post so much code and then say you can't change large parts of it.
Could you perhaps mark all the code you don't want to change?
I'm not sure I follow the rationale for not changing code that has been copied from elsewhere. If you copied it why can't you change it?
Quoted 6 lines
Quoted 2 lines
Hmm, then fix slub?
That's my plan, but I go about it a different way ;) I don't want to spend too much time on other allocators or cleanup etc code too much right now (except cleanups in SLQB, which of course is required).
But still if you copy code from slub you can improve it, can't you? The sysfs code definitely could be done much nicer (ok for small values of "nice"; sysfs is always ugly of course @). But at least it can be done in a way that doesn't bloat the text so much.
Thanks for the patch.
One thing I'm not sure about is using a private lock to hold off hotplug. I don't have a concrete scenario, but it makes me uneasy considering deadlocks when someone sleeps etc. Safer is get/put_online_cpus()
-Andi
Signature · 7 lines
[email protected] -- Speaking for myself only. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 15 lines · Andi Kleen
[dropping lameters' outdated address]
Quoted 11 lines · Nick Piggin
Quoted 9 lines
Quoted 3 lines
Probably the best way would
be to have dynamic cpu and node allocs for them, I agree.It's really needed.
Quoted 1 line
Any plans for an
alloc_pernode?It shouldn't be very hard to implement. Or do you ask if I'm volunteering? @)
Just if you knew about plans. I won't get too much time to work on
Quoted 7 lines
Not aware of anyone working on it.
Quoted 2 lines
it next week, so I hope to have something in slab tree in the meantime. I think it is OK to leave now, with a mind to improving
Sorry, the NR_CPUS/MAX_NUMNODE arrays are a merge blocker imho because they explode with
CONFIG_MAXSMP.
This is a linux-next merge, I'm talking about. The point is to get some parallelism between testing and making slqb perfect (not because I don't agree with the problem you point out).
Quoted 8 lines
Quoted 2 lines
it before a possible mainline merge (there will possibly be more serious issues discovered anyway).
I see you fixed the static arrays.
Doing the same for the
kmem_cachearrays with making them a pointer and then using num_possible_{cpus,nodes}() would seem straight forward, wouldn't it?
Hmm, yes that might be the way to go. I'll do that with the node array, the cpu array can stay where it is (this reduces cacheline footprint for small NR_CPUS configs).
Quoted 2 lines
Although I think I would prefer
alloc_percpu, possibly withper_cpu_ptr(first_cpu(node_to_cpumask(node)), ...)
I don't think we have the NUMA information available early enough to do that. But it would be the best idea indeed because it would take advantage of improvements in percpu allocator.
Quoted 10 lines
Quoted 1 line
But then it goes online and what happens?
You already have a node online notifier that should handle that then, don't you?
x86-64 btw currently doesn't support node hotplug (but I expect it will be added at some point), but it should be ok even on architectures that do.
Quoted 1 line
Your
numa_node_id()changes?
Quoted 8 lines
What do you mean?
Quoted 2 lines
How does that work? Or you mean x86-64 does not do that same trick for possible but offline nodes?
All I'm saying is that when x86-64 finds a memory less node it assigns its CPUs to other nodes. Hmm ok perhaps there's a backdoor when someone sets it with
kmalloc_node()but that should normally not happen I think.
OK, but if it is _possible_ for the node to gain memory, then you can't do that of course. If the node is always memoryless then yes I think it is probably a good idea to just assign it to the closest node with memory.
Quoted 9 lines
Quoted 2 lines
OK, but this code is just copied straight from SLAB... I don't want to add such dependency at this point I'm trying to get something
I'm sure such a straight forward change could be still put into .29
Quoted 1 line
reasonable to merge. But it would be a fine cleanup.
Hmm to be honest it's a little weird to post so much code and then say you can't change large parts of it.
The cache_line_size() change wouldn't change slqb code significantly. I have no problem with it, but I simply won't have time to do it and test all architectures and get them merged and hold off merging SLQB until they all get merged.
Quoted 1 line
Could you perhaps mark all the code you don't want to change?
Primarily the debug code from SLUB.
Quoted 2 lines
I'm not sure I follow the rationale for not changing code that has been copied from elsewhere. If you copied it why can't you change it?
I have, very extensively. Just diff mm/slqb.c mm/slub.c ;)
The point of not cleaning up peripheral (non-core) code that works, and exists upstream, is because it will actually be less hassle for me to maintain. By all means make improvements to the slub version which I can then pull into slqb.
Quoted 8 lines
Quoted 3 lines
That's my plan, but I go about it a different way ;) I don't want to spend too much time on other allocators or cleanup etc code too much right now (except cleanups in SLQB, which of course is required).
But still if you copy code from slub you can improve it, can't you? The sysfs code definitely could be done much nicer (ok for small values of "nice"; sysfs is always ugly of course @). But at least it can be done in a way that doesn't bloat the text so much.
I'm definitely not adverse to cleanups at all, but I just want to try avoid duplicating work or diverging if it is not necessary which makes it harder to track fixes etc. Just in this point in development...
Quoted 5 lines
Thanks for the patch.
One thing I'm not sure about is using a private lock to hold off hotplug. I don't have a concrete scenario, but it makes me uneasy considering deadlocks when someone sleeps etc. Safer is get/put_online_cpus()
I think it is OK, considering those locks must usually be taken anyway in the path, I've just tended to widen the coverage. But I'll think if anything can be improved with get/put API.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Nick Piggin
Quoted 3 lines
Although I think I would prefer
alloc_percpu, possibly withper_cpu_ptr(first_cpu(node_to_cpumask(node)), ...)I don't think we have the NUMA information available early enough to do that.
How early? At mem_init time it should be there because bootmem needed it already. It meaning the architectural level NUMA information.
Quoted 2 lines
OK, but if it is _possible_ for the node to gain memory, then you can't do that of course.
In theory it could gain memory through memory hotplug.
Quoted 11 lines
Quoted 6 lines
I'm sure such a straight forward change could be still put into .29
Quoted 1 line
reasonable to merge. But it would be a fine cleanup.
Hmm to be honest it's a little weird to post so much code and then say you can't change large parts of it.
The
cache_line_size()change wouldn't change slqb code significantly. I have no problem with it, but I simply won't have time to do it and test all architectures and get them merged and hold off merging SLQB until they all get merged.
I was mainly refering to the sysfs code here.
Quoted 3 lines
Quoted 1 line
Could you perhaps mark all the code you don't want to change?
Primarily the debug code from SLUB.
Ok so you could fix the sysfs code? @)
Anyways, if you have such shared pieces perhaps it would be better if you just pull them all out into a separate file.
-Andi
Signature · 7 lines
[email protected] -- Speaking for myself only. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Andi Kleen
Quoted 6 lines · Nick Piggin
Quoted 3 lines
Although I think I would prefer
alloc_percpu, possibly withper_cpu_ptr(first_cpu(node_to_cpumask(node)), ...)I don't think we have the NUMA information available early enough to do that.
How early? At
mem_inittime it should be there because bootmem needed it already. It meaning the architectural level NUMA information.
node_to_cpumask(0) returned 0 at kmem_cache_init time.
Quoted 4 lines
Quoted 2 lines
OK, but if it is _possible_ for the node to gain memory, then you can't do that of course.
In theory it could gain memory through memory hotplug.
Yes.
Quoted 6 lines
Quoted 4 lines
The
cache_line_size()change wouldn't change slqb code significantly. I have no problem with it, but I simply won't have time to do it and test all architectures and get them merged and hold off merging SLQB until they all get merged.I was mainly refering to the sysfs code here.
OK.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Could you perhaps mark all the code you don't want to change?
Primarily the debug code from SLUB.
Ok so you could fix the sysfs code? @)
Anyways, if you have such shared pieces perhaps it would be better if you just pull them all out into a separate file.
I'll see. I do plan to try making improvements to this peripheral code but it just has to wait a little bit for other improvements first.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Andi Kleen
include/linux/slqb_def.h line 268
Quoted 5 lines · Nick Piggin
include/linux/slqb_def.h new file +5
-0+#ifdef CONFIG_NUMA +void *__kmalloc_node (size_t size,gfp_t flags,int node); +void *kmem_cache_alloc_node (struct kmem_cache *,gfp_t flags,int node); + +static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
kmalloc_nodeshould be infrequent, i suspect it can be safely out of lined.
Hmm, it only takes up another couple of hundred bytes for a full numa kernel. Completely out of lining it can take a slightly slower path and makes the code slightly different from the kmalloc case. So I'll leave this change for now.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>