mm-slab: allocate kmem_cache with __GFP_REPEAT

Order of sizeof(struct kmem_cache) can be bigger than PAGE_ALLOC_COSTLY_ORDER, thus there is a good chance of unsuccessful allocation.
With __GFP_REPEAT buddy-allocator will reclaim/compact memory more aggressively.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
mm/slab.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

mm/slab.c +0 -0

index d96e223..53bddc8 100644

Quoted 16 lines · Konstantin Khlebnikov

Order of sizeof(struct kmem_cache) can be bigger than PAGE_ALLOC_COSTLY_ORDER, thus there is a good chance of unsuccessful allocation.
With __GFP_REPEAT buddy-allocator will reclaim/compact memory more aggressively.

Signed-off-by Konstantin Khlebnikov<[email protected]>

---
mm/slab.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

mm/slab.c +0 -0

index d96e223..53bddc8 100644
@@ -2304,7 +2304,7 @@ kmem_cache_create (const char *name, size_t size, size_t align,
 		gfp = GFP_NOWAIT;
 

mm/slab.c kmem_cache_create (const char *name, size_t size, size_t align, · line 2309

Quoted 5 lines

mm/slab.c +1 -1

 	/* Get cache's description obj. */
-	cachep = kmem_cache_zalloc(&cache_cache, gfp);
+	cachep = kmem_cache_zalloc(&cache_cache, gfp | __GFP_REPEAT);
 	if (!cachep)
 		goto oops;

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

Pekka

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace,
this patch of course not eliminates the chance of failure, but makes it more acceptable.

struct kmem_size for slub is more compact, it uses pecpu-pointers instead of dumb NR_CPUS-size array. probably better to fix this side...

Quoted 8 lines · Konstantin Khlebnikov

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

Quoted 3 lines · Konstantin Khlebnikov

struct kmem_size for slub is more compact, it uses pecpu-pointers instead of dumb NR_CPUS-size array.
probably better to fix this side...

So how big is 'struct kmem_cache' for your configuration anyway? Fixing the per-cpu data structures would be nice but I'm guessing it'll be slightly painful for mm/slab.c.

Pekka

Quoted 19 lines

Quoted 8 lines

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

Quoted 3 lines · Konstantin Khlebnikov

struct kmem_size for slub is more compact, it uses pecpu-pointers instead of dumb NR_CPUS-size array.
probably better to fix this side...

Quoted 3 lines

So how big is 'struct kmem_cache' for your configuration anyway? Fixing the per-cpu data structures would be nice but I'm guessing it'll be slightly painful for mm/slab.c.

With NR_CPUS=4096 and MAX_NUMNODES=512 its over 9k!
so it require order-4 page, meanwhile PAGE_ALLOC_COSTLY_ORDER is 3

That's somewhat sad. I suppose I can just merge your patch unless other people object to it. I'd like a v2 with better changelog though.

Quoted 20 lines · Pekka Enberg

Quoted 8 lines · Konstantin Khlebnikov

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

Quoted 3 lines · Konstantin Khlebnikov

struct kmem_size for slub is more compact, it uses pecpu-pointers instead of dumb NR_CPUS-size array.
probably better to fix this side...

Quoted 3 lines

So how big is 'struct kmem_cache' for your configuration anyway? Fixing the per-cpu data structures would be nice but I'm guessing it'll be slightly painful for mm/slab.c.

With NR_CPUS=4096 and MAX_NUMNODES=512 its over 9k!
so it require order-4 page, meanwhile PAGE_ALLOC_COSTLY_ORDER is 3

Quoted 3 lines

Pekka

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 5 lines · Konstantin Khlebnikov

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

Quoted 3 lines

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

Quoted 10 lines · Pekka Enberg

Quoted 8 lines · Konstantin Khlebnikov

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

Quoted 3 lines

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

There was a lot of recent fiddling with the reclaim logic. Maybe some of those changes caused the problem?

Quoted 12 lines · Christoph Lameter

Quoted 10 lines · Pekka Enberg

Quoted 8 lines · Konstantin Khlebnikov

Quoted 4 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

nf_conntrack creates separate slab-cache for each net-namespace, this patch of course not eliminates the chance of failure, but makes it more acceptable.

Quoted 7 lines

Quoted 3 lines

I'm still surprised you are seeing failures. mm/slab.c hasn't changed significantly in a long time. Why hasn't anyone reported this before? I'd still be inclined to shift the blame to the page allocator... Mel, Christoph?

There was a lot of recent fiddling with the reclaim logic. Maybe some of those changes caused the problem?

It's more likely that creating new slabs while under memory pressure significant enough to fail an order-4 allocation is a situation that is rarely tested.

What kernel version did this failure occur on? What was the system doing at the time of failure? Can the page allocation failure message be posted?

I catch this on our rhel6-openvz kernel, and yes it very patchy, but I don't see any reasons why this cannot be reproduced on mainline kernel.

there was abount ten containers with random stuff, node already do intensive swapout but still alive, in this situation starting new containers sometimes (1 per 1000) fails due to kmem_cache_create failures in nf_conntrack, there no other messages except:
Unable to create nf_conn slab cache
and some
nf_conntrack: falling back to vmalloc.
(it try allocates huge hash table and do it via vmalloc if kmalloc fails)

Le mercredi 20 juillet 2011 à 18:32 +0400, Konstantin Khlebnikov a écrit :

Quoted 10 lines

I catch this on our rhel6-openvz kernel, and yes it very patchy, but I don't see any reasons why this cannot be reproduced on mainline kernel.

there was abount ten containers with random stuff, node already do intensive swapout but still alive, in this situation starting new containers sometimes (1 per 1000) fails due to kmem_cache_create failures in nf_conntrack, there no other messages except:
Unable to create nf_conn slab cache
and some
nf_conntrack: falling back to vmalloc.
(it try allocates huge hash table and do it via vmalloc if kmalloc fails)

Does this kernel contain commit 6d4831c2 ?
(vfs: avoid large kmalloc()s for the fdtable)

Quoted 16 lines · Konstantin Khlebnikov

Order of sizeof(struct kmem_cache) can be bigger than PAGE_ALLOC_COSTLY_ORDER, thus there is a good chance of unsuccessful allocation.
With __GFP_REPEAT buddy-allocator will reclaim/compact memory more aggressively.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
mm/slab.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

mm/slab.c +0 -0

index d96e223..53bddc8 100644
@@ -2304,7 +2304,7 @@ kmem_cache_create (const char *name, size_t size, size_t align,
 		gfp = GFP_NOWAIT;
 

mm/slab.c kmem_cache_create (const char *name, size_t size, size_t align, · line 2309

Quoted 5 lines

mm/slab.c +1 -1

 	/* Get cache's description obj. */
-	cachep = kmem_cache_zalloc(&cache_cache, gfp);
+	cachep = kmem_cache_zalloc(&cache_cache, gfp | __GFP_REPEAT);
 	if (!cachep)
 		goto oops;

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

Quoted 6 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

Besides, is allocating from cache_cache really a PAGE_ALLOC_COSTLY_ORDER allocation? On my laptop at least, it's an order-2 allocation which is supporting up to 512 CPUs and 512 nodes.

Quoted 7 lines · Mel Gorman

Quoted 6 lines

The changelog isn't that convincing, really. This is kmem_cache_create() so I'm surprised we'd ever get NULL here in practice. Does this fix some problem you're seeing? If this is really an issue, I'd blame the page allocator as GFP_KERNEL should just work.

Quoted 3 lines

Besides, is allocating from cache_cache really a PAGE_ALLOC_COSTLY_ORDER allocation? On my laptop at least, it's an order-2 allocation which is supporting up to 512 CPUs and 512 nodes.

Slab's kmem_cache is configured with an array of NR_CPUS which is the maximum nr of cpus supported. Some distros support 4096 cpus in order to accomodate SGI machines. That array then will have the size of 4096 * 8 = 32k

Quoted 4 lines

Slab's kmem_cache is configured with an array of NR_CPUS which is the maximum nr of cpus supported. Some distros support 4096 cpus in order to accomodate SGI machines. That array then will have the size of 4096 * 8 = 32k

We currently support a dynamic schem for the possible nodes :

cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +

	nr_node_ids * sizeof(struct kmem_list3 *);

We could have a similar trick to make the real size both depends on nr_node_ids and nr_cpu_ids.

(struct kmem_cache)->array would become a pointer.

Quoted 10 lines · Eric Dumazet

Quoted 4 lines

Slab's kmem_cache is configured with an array of NR_CPUS which is the maximum nr of cpus supported. Some distros support 4096 cpus in order to accomodate SGI machines. That array then will have the size of 4096 * 8 = 32k

We currently support a dynamic schem for the possible nodes :

cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +

	nr_node_ids * sizeof(struct kmem_list3 *);

Quoted 4 lines

We could have a similar trick to make the real size both depends on nr_node_ids and nr_cpu_ids.

(struct kmem_cache)->array would become a pointer.

We should be making it a per cpu pointer like slub then. I looked at what it would take to do so a couple of month ago but it was quite invasive.

[PATCH] slab: remove one NR_CPUS dependency

Quoted 1 line · Eric Dumazet

[PATCH] slab: remove one NR_CPUS dependency

Ok simple enough.

Acked-by Christoph Lameter <[email protected]>

Note that adding ____cacheline_aligned_in_smp on nodelists[] actually helps performance, as all following fields are readonly after kmem_cache setup.

Quoted 4 lines

Note that adding ____cacheline_aligned_in_smp on nodelists[] actually helps performance, as all following fields are readonly after kmem_cache setup.

Well but that is not addresssing the same issue. Could you separate that out?

The other question that follows from this is then: Does that alignment compensate for the loss of performance due to the additional lookup in hot code paths and the additional cacheline reference required?

The per node pointers are lower priority in terms of performance than the per cpu pointers. I'd rather have the per node pointers requiring an additional lookup. Less impact on hot code paths.

In fact resulting code is smaller, because most fields are now with < 127 offset (x86 assembly code can use shorter instructions)

Before patch :

# size mm/slab.o
   text	   data	    bss	    dec	    hex	filename
  22605	 361665	     32	 384302	  5dd2e	mm/slab.o

After patch :

# size mm/slab.o
   text	   data	    bss	    dec	    hex	filename
  22347	 328929	  32800	 384076	  5dc4c	mm/slab.o

Quoted 4 lines

The per node pointers are lower priority in terms of performance than the per cpu pointers. I'd rather have the per node pointers requiring an additional lookup. Less impact on hot code paths.

Sure. I'll post a V2 to have CPU array before NODE array.

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter <[email protected]>

Quoted 1 line · Eric Dumazet

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 4 lines · Christoph Lameter

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter <[email protected]>

Applied, thanks! Do we still want the __GFP_REPEAT patch from Konstantin though?

Quoted 11 lines · Eric Dumazet

Quoted 1 line

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 4 lines · Christoph Lameter

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter <[email protected]>

Applied, thanks! Do we still want the __GFP_REPEAT patch from Konstantin though?

Those with 4k cpus will be thankful I guess.

Quoted 9 lines · Pekka Enberg

Quoted 9 lines · Eric Dumazet

Quoted 1 line

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 4 lines · Christoph Lameter

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter <[email protected]>

Quoted 2 lines

Quoted 2 lines

Applied, thanks! Do we still want the __GFP_REPEAT patch from Konstantin though?

Quoted 1 line · Christoph Lameter

Those with 4k cpus will be thankful I guess.

OTOH, I'm slightly worried that it might mask a real problem with GFP_KERNEL not being aggressive enough. Mel?

Quoted 13 lines · Pekka Enberg

Quoted 11 lines · Pekka Enberg

Quoted 11 lines · Eric Dumazet

Quoted 1 line

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 4 lines · Christoph Lameter

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter<[email protected]>

Applied, thanks! Do we still want the __GFP_REPEAT patch from Konstantin though?

Quoted 5 lines · Christoph Lameter

Quoted 1 line

Those with 4k cpus will be thankful I guess.

OTOH, I'm slightly worried that it might mask a real problem with GFP_KERNEL not being aggressive enough. Mel?

I think that's already been demonstrated here, yes. It's just waiting for another obscure workload to trigger it.

Quoted 9 lines · Pekka Enberg

Quoted 9 lines · Eric Dumazet

Quoted 1 line

Quoted 1 line

[PATCH v2] slab: shrinks sizeof(struct kmem_cache)

Quoted 4 lines · Christoph Lameter

This will solve the issue for small nr_cpu_ids but those with 4k cpus will still have the issue.

Acked-by Christoph Lameter <[email protected]>

Quoted 2 lines

Quoted 2 lines

Applied, thanks! Do we still want the __GFP_REPEAT patch from Konstantin though?

Quoted 1 line · Christoph Lameter

Those with 4k cpus will be thankful I guess.

OTOH, I'm slightly worried that it might mask a real problem with GFP_KERNEL not being aggressive enough. Mel?

Quoted 12 lines · Christoph Lameter

Quoted 10 lines · Eric Dumazet

Quoted 4 lines

Slab's kmem_cache is configured with an array of NR_CPUS which is the maximum nr of cpus supported. Some distros support 4096 cpus in order to accomodate SGI machines. That array then will have the size of 4096 * 8 = 32k

We currently support a dynamic schem for the possible nodes :

cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +

	nr_node_ids * sizeof(struct kmem_list3 *);

Quoted 8 lines

Quoted 4 lines

We could have a similar trick to make the real size both depends on nr_node_ids and nr_cpu_ids.

(struct kmem_cache)->array would become a pointer.

We should be making it a per cpu pointer like slub then. I looked at what it would take to do so a couple of month ago but it was quite invasive.

Lets try this first patch, simple enough : No need to setup percpu data for a one time use structure...

[PATCH] slab: remove one NR_CPUS dependency

Reduce high order allocations in do_tune_cpucache() for some setups. (NR_CPUS=4096 -> we need 64KB)

Signed-off-by Eric Dumazet<[email protected]>

CC Pekka Enberg<[email protected]>

CC Christoph Lameter<[email protected]>

---
  mm/slab.c |    5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

mm/slab.c +1 -1

index d96e223..862bd12 100644
@@ -3933,7 +3933,7 @@ fail:
 
  struct ccupdate_struct {
  	struct kmem_cache *cachep;
-	struct array_cache *new[NR_CPUS];
+	struct array_cache *new[0];
  };
 
  static void do_ccupdate_local(void *info)

It seems someone forgot this patch,

Quoted 4 lines

[PATCH] slab: remove one NR_CPUS dependency

Reduce high order allocations in do_tune_cpucache() for some setups. (NR_CPUS=4096 ->  we need 64KB)

We should be making it a per cpu pointer like slub then. I looked at what it would take to do so a couple of month ago but it was quite invasive.

Quoted 3 lines · Christoph Lameter

We should be making it a per cpu pointer like slub then. I looked at what it would take to do so a couple of month ago but it was quite invasive.

I took a look at this too, but using percpu data would consume more memory, because percpu allocator allocates memory blobs for all possible cpus, while current code handles online/offline cpu nicely.