We see page allocation failure warnings on the mempool_alloc() path.
See this lkml posting for example:
http://lkml.org/lkml/2008/10/27/100
The cause is that on NUMA, alloc_slabmgmt() clears __GFP_NOWARN,
together with __GFP_THISNODE and __GFP_NORETRY. But AFAICS it really
only wants to clear __GFP_THISNODE.
Does this patch looks good?
Warning: it's completely untested.
Miklos
---
mm/slab.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6/mm/slab.c
===================================================================
--- linux-2.6.orig/mm/slab.c 2008-10-24 12:40:34.000000000 +0200
+++ linux-2.6/mm/slab.c 2008-11-24 22:17:04.000000000 +0100
@@ -2609,7 +2609,8 @@ static struct slab *alloc_slabmgmt(struc
if (OFF_SLAB(cachep)) {
/* Slab management obj is off-slab. */
slabp = kmem_cache_alloc_node(cachep->slabp_cache,
- local_flags & ~GFP_THISNODE, nodeid);
+ local_flags & ~__GFP_THISNODE,
+ nodeid);
if (!slabp)
return NULL;
} else {
On Mon, 24 Nov 2008, Miklos Szeredi wrote:
> We see page allocation failure warnings on the mempool_alloc() path.
> See this lkml posting for example:
>
> http://lkml.org/lkml/2008/10/27/100
>
> The cause is that on NUMA, alloc_slabmgmt() clears __GFP_NOWARN,
> together with __GFP_THISNODE and __GFP_NORETRY. But AFAICS it really
> only wants to clear __GFP_THISNODE.
>
> Does this patch looks good?
Yes, it does but looking at mm/slab.c history I think we want something
like the following instead. Christoph?
P.S. First one to test it gets a fabulous prize of a Tested-by tag in the
patch description! How cool is that?
Pekka
>From 717b2d7bd2398e948d1dfbd2e83bd6a8403b68da Mon Sep 17 00:00:00 2001
From: Pekka Enberg <[email protected]>
Date: Tue, 25 Nov 2008 00:33:28 +0200
Subject: [PATCH] slab: remove GFP_THISNODE clearing from alloc_slabmgmt()
Commit 6cb062296f73e74768cca2f3eaf90deac54de02d ("Categorize GFP flags")
left one call-site in alloc_slabmgmt() to clear GFP_THISNODE instead of
GFP_CONSTRAINT_MASK. Unfortunately, that ends up clearing __GFP_NOWARN
and __GFP_NORETRY as well which is not what we want. As the only caller
of alloc_slabmgmt() already clears GFP_CONSTRAINT_MASK before passing
local_flags to it, we can just remove the clearing of GFP_THISNODE.
This patch should fix spurious page allocation failure warnings on the
mempool_alloc() path. See the following URL for an example:
http://lkml.org/lkml/2008/10/27/100
Reported-by: Miklos Szeredi <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
mm/slab.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/mm/slab.c b/mm/slab.c
index 918f04f..98d3024 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2608,7 +2608,7 @@ static struct slab *alloc_slabmgmt(struct kmem_cache *cachep, void *objp,
if (OFF_SLAB(cachep)) {
/* Slab management obj is off-slab. */
slabp = kmem_cache_alloc_node(cachep->slabp_cache,
- local_flags & ~GFP_THISNODE, nodeid);
+ local_flags, nodeid);
if (!slabp)
return NULL;
} else {
--
1.5.6.4
On Tue, 25 Nov 2008, Pekka J Enberg wrote:
> Yes, it does but looking at mm/slab.c history I think we want something
> like the following instead. Christoph?
Right.
On Tue, 25 Nov 2008, Pekka J Enberg wrote:
> > Yes, it does but looking at mm/slab.c history I think we want something
> > like the following instead. Christoph?
On Tue, 2008-11-25 at 12:58 -0600, Christoph Lameter wrote:
> Right.
OK, even though no tester showed up, I went ahead and merged my version
of the patch as it's a pretty obvious one-liner fix.
On Wed, 26 Nov 2008, Pekka Enberg wrote:
> On Tue, 25 Nov 2008, Pekka J Enberg wrote:
> > > Yes, it does but looking at mm/slab.c history I think we want something
> > > like the following instead. Christoph?
>
> On Tue, 2008-11-25 at 12:58 -0600, Christoph Lameter wrote:
> > Right.
>
> OK, even though no tester showed up, I went ahead and merged my version
> of the patch as it's a pretty obvious one-liner fix.
Thanks. I merged your version in the suse tree as well, so there'll
be lots of testers :)
Miklos