linux-next ia64 build problems in slqb

linux-next ia64 build problems in slqb

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
of .config options:

	CONFIG_SMP=n
	CONFIG_NUMA=y

which leads to the following build problems with mm/slqb.c

  CC      mm/slqb.o
mm/slqb.c: In function ‘__slab_free’:
mm/slqb.c:1735: error: implicit declaration of function ‘slab_free_to_remote’
mm/slqb.c: In function ‘kmem_cache_open’:
mm/slqb.c:2274: error: implicit declaration of function ‘kmem_cache_dyn_array_free’
mm/slqb.c:2275: warning: label ‘error_cpu_array’ defined but not used
mm/slqb.c: In function ‘kmem_cache_destroy’:
mm/slqb.c:2395: error: implicit declaration of function ‘claim_remote_free_list’
mm/slqb.c: In function ‘kmem_cache_init’:
mm/slqb.c:2885: error: ‘per_cpu__kmem_cpu_nodes’ undeclared (first use in this function)
mm/slqb.c:2885: error: (Each undeclared identifier is reported only once
mm/slqb.c:2885: error: for each function it appears in.)
mm/slqb.c:2886: error: ‘kmem_cpu_cache’ undeclared (first use in this function)

-Tony

Re: linux-next ia64 build problems in slqb

[Randy Dunlap]

Luck, Tony wrote:
> The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
> of .config options:
> 
> 	CONFIG_SMP=n
> 	CONFIG_NUMA=y
> 
> which leads to the following build problems with mm/slqb.c
> 
>   CC      mm/slqb.o
> mm/slqb.c: In function ‘__slab_free’:
> mm/slqb.c:1735: error: implicit declaration of function ‘slab_free_to_remote’
> mm/slqb.c: In function ‘kmem_cache_open’:
> mm/slqb.c:2274: error: implicit declaration of function ‘kmem_cache_dyn_array_free’
> mm/slqb.c:2275: warning: label ‘error_cpu_array’ defined but not used
> mm/slqb.c: In function ‘kmem_cache_destroy’:
> mm/slqb.c:2395: error: implicit declaration of function ‘claim_remote_free_list’
> mm/slqb.c: In function ‘kmem_cache_init’:
> mm/slqb.c:2885: error: ‘per_cpu__kmem_cpu_nodes’ undeclared (first use in this function)
> mm/slqb.c:2885: error: (Each undeclared identifier is reported only once
> mm/slqb.c:2885: error: for each function it appears in.)
> mm/slqb.c:2886: error: ‘kmem_cpu_cache’ undeclared (first use in this function)

Does this patch fix it?

http://lkml.org/lkml/2009/4/20/30

RE: linux-next ia64 build problems in slqb

[Luck, Tony]

> Does this patch fix it?
>
> http://lkml.org/lkml/2009/4/20/30

That makes all the compilation errors go away from
the allnoconfig build.  Build still seems OK on
a more conventional .config (tiger_defconfig).

I haven't tried booting this kernel though, so
I don't know whether slqb works or not. Perhaps
I'll get brave and try that tomorrow.

-Tony

Re: linux-next ia64 build problems in slqb

[Pekka Enberg]

Hi Tony,

On Tue, Apr 21, 2009 at 1:08 AM, Luck, Tony <tony.luck@intel.com> wrote:
> The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
> of .config options:
>
>        CONFIG_SMP=n
>        CONFIG_NUMA=y

Yeah, I don't think the combination makes much sense and x86 doesn't
allow it. Can we fix that in ia64 Kconfig? That said, this seems to
come up with every architecture except x86, so if someone is kind
enough to send me a tested fix for slqb, I'll just go ahead and apply
it.

                                   Pekka

Re: linux-next ia64 build problems in slqb

[Paul Mundt]

On Tue, Apr 21, 2009 at 08:51:47AM +0300, Pekka Enberg wrote:
> Hi Tony,
> 
> On Tue, Apr 21, 2009 at 1:08 AM, Luck, Tony <tony.luck@intel.com> wrote:
> > The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
> > of .config options:
> >
> > ? ? ? ?CONFIG_SMP=n
> > ? ? ? ?CONFIG_NUMA=y
> 
> Yeah, I don't think the combination makes much sense and x86 doesn't
> allow it. Can we fix that in ia64 Kconfig? That said, this seems to
> come up with every architecture except x86, so if someone is kind
> enough to send me a tested fix for slqb, I'll just go ahead and apply
> it.
> 
This configuration is used on SH quite regularly, so if SLQB breaks, then
that is a regression.

Re: linux-next ia64 build problems in slqb

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 699 bytes --]

On Tue, 21 Apr 2009, Pekka Enberg wrote:

> On Tue, Apr 21, 2009 at 1:08 AM, Luck, Tony <tony.luck@intel.com> wrote:
> > The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
> > of .config options:
> >
> >        CONFIG_SMP=n
> >        CONFIG_NUMA=y
>
> Yeah, I don't think the combination makes much sense and x86 doesn't
> allow it. Can we fix that in ia64 Kconfig? That said, this seems to
> come up with every architecture except x86, so if someone is kind
> enough to send me a tested fix for slqb, I'll just go ahead and apply
> it.

Tony runs tests with this configuration. This is an established
configuration and has been historically supported.

Re: linux-next ia64 build problems in slqb

[Pekka Enberg]

On Tue, Apr 21, 2009 at 1:08 AM, Luck, Tony <tony.luck@intel.com> wrote:
>> > The ia64 "allnoconfig" build ends up with the (possibly dubious) combination
>> > of .config options:
>> >
>> >        CONFIG_SMP=n
>> >        CONFIG_NUMA=y
>>
>> Yeah, I don't think the combination makes much sense and x86 doesn't
>> allow it. Can we fix that in ia64 Kconfig? That said, this seems to
>> come up with every architecture except x86, so if someone is kind
>> enough to send me a tested fix for slqb, I'll just go ahead and apply
>> it.

On Tue, Apr 21, 2009 at 5:35 PM, Christoph Lameter <cl@linux.com> wrote:
> Tony runs tests with this configuration. This is an established
> configuration and has been historically supported.

Interesting. What exactly is an UP NUMA machine? Anyway, I'm more than
happy to apply a tested patch to fix up SLQB. Nick?

RE: linux-next ia64 build problems in slqb

[Luck, Tony]

> Interesting. What exactly is an UP NUMA machine?

UP + NUMA is a special case of memory-only nodes.  There are
some (crazy?) customers with problems that require very large
amounts of memory, but not very much cpu horse power.  They
buy large multi-node systems and populate all the nodes with
as much memory as they can afford, but most nodes get zero
cpus.

The limit case with only one cpu is becoming rare (since
multi-core gives you more than one cpu even if you only
fill a single cpu socket).

N.B. Note that memory only nodes mean that it is a poor
idea for the "free" code to just place returned remote
memory on a queue for the node that it belongs to, and
rely on a "local" cpu processing that queue ... there may
not be any local cpus to do so.

> Anyway, I'm more than
> happy to apply a tested patch to fix up SLQB. Nick?

I'm trying to check whether http://lkml.org/lkml/2009/4/20/30
fixes things.  It certainly solves the complilation problem,
but I'm running into apparently unrelated issues trying to
boot linux-next kernels.

-Tony

Re: linux-next ia64 build problems in slqb

[Pekka Enberg]

Hi Tony,

On Tue, Apr 21, 2009 at 9:45 PM, Luck, Tony <tony.luck@intel.com> wrote:
>> Interesting. What exactly is an UP NUMA machine?
>
> UP + NUMA is a special case of memory-only nodes.  There are
> some (crazy?) customers with problems that require very large
> amounts of memory, but not very much cpu horse power.  They
> buy large multi-node systems and populate all the nodes with
> as much memory as they can afford, but most nodes get zero
> cpus.

Oh, cool. Thanks for the explanation!

On Tue, Apr 21, 2009 at 9:45 PM, Luck, Tony <tony.luck@intel.com> wrote:
>> Anyway, I'm more than
>> happy to apply a tested patch to fix up SLQB. Nick?
>
> I'm trying to check whether http://lkml.org/lkml/2009/4/20/30
> fixes things.  It certainly solves the complilation problem,
> but I'm running into apparently unrelated issues trying to
> boot linux-next kernels.

Great! You can try it on top of the "topic/slqb/core" branch of

  git://git.kernel.org/pub/scm/linux/kernel/git/penberg/slab-2.6.git

if you want. It's basically plain 2.6.30-rc1 plus SLQB.

One minor nit: the patch should define an empty static inline of
claim_remote_free_list() for the !SMP case. I can fix it at my end
before merging, though, if necessary.

                                       Pekka

RE: linux-next ia64 build problems in slqb

[Luck, Tony]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

> > I'm trying to check whether http://lkml.org/lkml/2009/4/20/30
> > fixes things.  It certainly solves the complilation problem,
> > but I'm running into apparently unrelated issues trying to
> > boot linux-next kernels.

My unrelated issue was the new default for CONFIG_SYSFS_DEPRECATED
(since my test machine does have an old distribution, this change
upset my old udev tools and I ended up with no /dev/root :-( )

Turning that back on, the CONFIG_SLQB=y version booted with no
obvious problems.

> Great! You can try it on top of the "topic/slqb/core" branch of
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/penberg/slab-2.6.git
>
> if you want. It's basically plain 2.6.30-rc1 plus SLQB.

Since I got linux-next up & running, I didn't need to try this.

> One minor nit: the patch should define an empty static inline of
> claim_remote_free_list() for the !SMP case. I can fix it at my end
> before merging, though, if necessary.

Agreed.  It would be better to have an empty static inline than
adding the noisy #ifdef SMP around every call to
claim_remote_free_list() ... in fact some such #ifdef can be
removed.

You could tag such a modified patch (attached) as:

Acked-by: Tony Luck <tony.luck@intel.com>

-Tony

[-- Attachment #2: fix-slqb-numa-up-build.patch --]
[-- Type: application/octet-stream, Size: 2283 bytes --]

diff --git a/mm/slqb.c b/mm/slqb.c
index 37949f5..50bf180 100644
--- a/mm/slqb.c
+++ b/mm/slqb.c
@@ -1224,6 +1224,11 @@ static void claim_remote_free_list(struct kmem_cache *s,
 	slqb_stat_inc(l, CLAIM_REMOTE_LIST);
 	slqb_stat_add(l, CLAIM_REMOTE_LIST_OBJECTS, nr);
 }
+#else
+static inline void claim_remote_free_list(struct kmem_cache *s,
+					struct kmem_cache_list *l)
+{
+}
 #endif
 
 /*
@@ -1728,7 +1733,7 @@ static __always_inline void __slab_free(struct kmem_cache *s,
 			flush_free_list(s, l);
 
 	} else {
-#ifdef CONFIG_NUMA
+#ifdef CONFIG_SMP
 		/*
 		 * Freeing an object that was allocated on a remote node.
 		 */
@@ -1937,7 +1942,9 @@ static DEFINE_PER_CPU(struct kmem_cache_node, kmem_cpu_nodes); /* XXX per-nid */
 
 #ifdef CONFIG_NUMA
 static struct kmem_cache kmem_node_cache;
+# ifdef CONFIG_SMP
 static DEFINE_PER_CPU(struct kmem_cache_cpu, kmem_node_cpus);
+# endif
 static DEFINE_PER_CPU(struct kmem_cache_node, kmem_node_nodes); /*XXX per-nid */
 #endif
 
@@ -2270,7 +2277,7 @@ static int kmem_cache_open(struct kmem_cache *s,
 error_nodes:
 	free_kmem_cache_nodes(s);
 error_node_array:
-#ifdef CONFIG_NUMA
+#if defined(CONFIG_NUMA) && defined(CONFIG_SMP)
 	kmem_cache_dyn_array_free(s->node_slab);
 error_cpu_array:
 #endif
@@ -2370,9 +2377,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
 		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
 		struct kmem_cache_list *l = &c->list;
 
-#ifdef CONFIG_SMP
 		claim_remote_free_list(s, l);
-#endif
 		flush_free_list_all(s, l);
 
 		WARN_ON(l->freelist.nr);
@@ -2595,9 +2600,7 @@ static void kmem_cache_trim_percpu(void *arg)
 	struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
 	struct kmem_cache_list *l = &c->list;
 
-#ifdef CONFIG_SMP
 	claim_remote_free_list(s, l);
-#endif
 	flush_free_list(s, l);
 #ifdef CONFIG_SMP
 	flush_remote_free_cache(s, c);
@@ -2881,11 +2884,11 @@ void __init kmem_cache_init(void)
 		n = &per_cpu(kmem_cache_nodes, i);
 		init_kmem_cache_node(&kmem_cache_cache, n);
 		kmem_cache_cache.node_slab[i] = n;
-
+#ifdef CONFIG_SMP
 		n = &per_cpu(kmem_cpu_nodes, i);
 		init_kmem_cache_node(&kmem_cpu_cache, n);
 		kmem_cpu_cache.node_slab[i] = n;
-
+#endif
 		n = &per_cpu(kmem_node_nodes, i);
 		init_kmem_cache_node(&kmem_node_cache, n);
 		kmem_node_cache.node_slab[i] = n;

RE: linux-next ia64 build problems in slqb

[Pekka J Enberg]

Hi Tony,

On Tue, 21 Apr 2009, Luck, Tony wrote:
> > One minor nit: the patch should define an empty static inline of
> > claim_remote_free_list() for the !SMP case. I can fix it at my end
> > before merging, though, if necessary.
> 
> Agreed.  It would be better to have an empty static inline than
> adding the noisy #ifdef SMP around every call to
> claim_remote_free_list() ... in fact some such #ifdef can be
> removed.
> 
> You could tag such a modified patch (attached) as:
> 
> Acked-by: Tony Luck <tony.luck@intel.com>

Thanks for the help! I went and merged the following patch and I hope I 
got all the patch attributions right. Paul, does this work for you as well?

			Pekka

>From d46f661ed791312ba008f862a601179c5c9f1e9c Mon Sep 17 00:00:00 2001
From: Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
Date: Wed, 22 Apr 2009 09:50:15 +0300
Subject: [PATCH] SLQB: Fix UP + NUMA build

This patch fixes the following build breakage which happens when CONFIG_NUMA is
enabled but CONFIG_SMP is disabled:

    CC      mm/slqb.o
  mm/slqb.c: In function '__slab_free':
  mm/slqb.c:1735: error: implicit declaration of function 'slab_free_to_remote'
  mm/slqb.c: In function 'kmem_cache_open':
  mm/slqb.c:2274: error: implicit declaration of function 'kmem_cache_dyn_array_free'
  mm/slqb.c:2275: warning: label 'error_cpu_array' defined but not used
  mm/slqb.c: In function 'kmem_cache_destroy':
  mm/slqb.c:2395: error: implicit declaration of function 'claim_remote_free_list'
  mm/slqb.c: In function 'kmem_cache_init':
  mm/slqb.c:2885: error: 'per_cpu__kmem_cpu_nodes' undeclared (first use in this function)
  mm/slqb.c:2885: error: (Each undeclared identifier is reported only once
  mm/slqb.c:2885: error: for each function it appears in.)
  mm/slqb.c:2886: error: 'kmem_cpu_cache' undeclared (first use in this function)
  make[1]: *** [mm/slqb.o] Error 1
  make: *** [mm] Error 2

As x86 Kconfig doesn't even allow this combination, one is tempted to think
it's an architecture Kconfig bug. But as it turns out, it's a perfecly 
valid configuration. Tony Luck explains:

  UP + NUMA is a special case of memory-only nodes.  There are some (crazy?)
  customers with problems that require very large amounts of memory, but not very
  much cpu horse power.  They buy large multi-node systems and populate all the
  nodes with as much memory as they can afford, but most nodes get zero cpus.

So lets fix that up.

[ tony.luck@intel.com: #ifdef cleanups ]
Signed-off-by: Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
Acked-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slqb.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/slqb.c b/mm/slqb.c
index 37949f5..0300a6d 100644
--- a/mm/slqb.c
+++ b/mm/slqb.c
@@ -1224,6 +1224,11 @@ static void claim_remote_free_list(struct kmem_cache *s,
 	slqb_stat_inc(l, CLAIM_REMOTE_LIST);
 	slqb_stat_add(l, CLAIM_REMOTE_LIST_OBJECTS, nr);
 }
+#else
+static inline void claim_remote_free_list(struct kmem_cache *s,
+					struct kmem_cache_list *l)
+{
+}
 #endif
 
 /*
@@ -1728,7 +1733,7 @@ static __always_inline void __slab_free(struct kmem_cache *s,
 			flush_free_list(s, l);
 
 	} else {
-#ifdef CONFIG_NUMA
+#ifdef CONFIG_SMP
 		/*
 		 * Freeing an object that was allocated on a remote node.
 		 */
@@ -1937,7 +1942,9 @@ static DEFINE_PER_CPU(struct kmem_cache_node, kmem_cpu_nodes); /* XXX per-nid */
 
 #ifdef CONFIG_NUMA
 static struct kmem_cache kmem_node_cache;
+#ifdef CONFIG_SMP
 static DEFINE_PER_CPU(struct kmem_cache_cpu, kmem_node_cpus);
+#endif
 static DEFINE_PER_CPU(struct kmem_cache_node, kmem_node_nodes); /*XXX per-nid */
 #endif
 
@@ -2270,7 +2277,7 @@ static int kmem_cache_open(struct kmem_cache *s,
 error_nodes:
 	free_kmem_cache_nodes(s);
 error_node_array:
-#ifdef CONFIG_NUMA
+#if defined(CONFIG_NUMA) && defined(CONFIG_SMP)
 	kmem_cache_dyn_array_free(s->node_slab);
 error_cpu_array:
 #endif
@@ -2370,9 +2377,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
 		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
 		struct kmem_cache_list *l = &c->list;
 
-#ifdef CONFIG_SMP
 		claim_remote_free_list(s, l);
-#endif
 		flush_free_list_all(s, l);
 
 		WARN_ON(l->freelist.nr);
@@ -2595,9 +2600,7 @@ static void kmem_cache_trim_percpu(void *arg)
 	struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
 	struct kmem_cache_list *l = &c->list;
 
-#ifdef CONFIG_SMP
 	claim_remote_free_list(s, l);
-#endif
 	flush_free_list(s, l);
 #ifdef CONFIG_SMP
 	flush_remote_free_cache(s, c);
@@ -2881,11 +2884,11 @@ void __init kmem_cache_init(void)
 		n = &per_cpu(kmem_cache_nodes, i);
 		init_kmem_cache_node(&kmem_cache_cache, n);
 		kmem_cache_cache.node_slab[i] = n;
-
+#ifdef CONFIG_SMP
 		n = &per_cpu(kmem_cpu_nodes, i);
 		init_kmem_cache_node(&kmem_cpu_cache, n);
 		kmem_cpu_cache.node_slab[i] = n;
-
+#endif
 		n = &per_cpu(kmem_node_nodes, i);
 		init_kmem_cache_node(&kmem_node_cache, n);
 		kmem_node_cache.node_slab[i] = n;
-- 
1.5.6.3

Re: linux-next ia64 build problems in slqb

[Paul Mundt]

On Wed, Apr 22, 2009 at 10:02:06AM +0300, Pekka J Enberg wrote:
> On Tue, 21 Apr 2009, Luck, Tony wrote:
> > > One minor nit: the patch should define an empty static inline of
> > > claim_remote_free_list() for the !SMP case. I can fix it at my end
> > > before merging, though, if necessary.
> > 
> > Agreed.  It would be better to have an empty static inline than
> > adding the noisy #ifdef SMP around every call to
> > claim_remote_free_list() ... in fact some such #ifdef can be
> > removed.
> > 
> > You could tag such a modified patch (attached) as:
> > 
> > Acked-by: Tony Luck <tony.luck@intel.com>
> 
> Thanks for the help! I went and merged the following patch and I hope I 
> got all the patch attributions right. Paul, does this work for you as well?
> 
Yup, works fine for me, thanks for taking care of this.

Acked-by: Paul Mundt <lethal@linux-sh.org>