diff mbox

[v3] mm: make expand_downwards symmetrical to expand_upwards

Message ID 1303401997.4025.8.camel@mulgrave.site (mailing list archive)
State Not Applicable
Headers show

Commit Message

James Bottomley April 21, 2011, 4:06 p.m. UTC
On Wed, 2011-04-20 at 14:42 -0700, David Rientjes wrote:
> On Wed, 20 Apr 2011, Christoph Lameter wrote:
> 
> > There is barely any testing going on at all of this since we have had this
> > issue for more than 5 years and have not noticed it. The absence of bug
> > reports therefore proves nothing. Code inspection of the VM shows
> > that this is an issue that arises in multiple subsystems and that we have
> > VM_BUG_ONs in the page allocator that should trigger for these situations.
> > 
> > Usage of DISCONTIGMEM and !NUMA is not safe and should be flagged as such.
> > 
> 
> We don't actually have any bug reports in front of us that show anything 
> else in the VM other than slub has issues with this configuration, so 
> marking them as broken is probably premature.  The parisc config that 
> triggered this debugging enables CONFIG_SLAB by default, so it probably 
> has gone unnoticed just because nobody other than James has actually tried 
> it on hppa64.
> 
> Let's see if KOSAKI-san's fixes to Kconfig (even though I'd prefer the 
> simpler and implicit "config NUMA def_bool ARCH_DISCONTIGMEM_ENABLE" over 
> his config NUMA) and my fix to parisc to set the bit in N_NORMAL_MEMORY 
> so that CONFIG_SLUB initializes kmem_cache_node correctly works and then 
> address issues in the core VM as they arise.  Presumably someone has been 
> running DISCONTIGMEM on hppa64 in the past five years without issues with 
> defconfig, so the issue here may just be slub.

Actually, we can fix slub.  As far as all my memory hammer tests go, the
one liner below is the actual fix (it just forces slub get_node() to
return the zero node always on !NUMA).  That, as far as a code
inspection goes, seems to make SLUB as good as SLAB ... as long as
no-one uses hugepages or VM DEBUG, which, I think we've demonstrated, is
the case for all the current DISCONTIGMEM users.

I think either the above or just marking slub broken in DISCONTIGMEM & !
NUMA is sufficient for stable.  The fix is getting urgent, because
debian (which is what most of our users are running) has made SLUB the
default allocator, which is why we're now starting to run into these
panic reports.

The set memory range fix looks good for a backport too ... at least the
page cache is now no-longer reluctant to use my upper 1GB ...

I worry a bit more about backporting the selection of NUMA as a -stable
fix because it's a larger change (and requires changes to all the
architectures, since NUMA is an arch local Kconfig variable)

James

----



--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Rientjes April 21, 2011, 10:19 p.m. UTC | #1
On Thu, 21 Apr 2011, James Bottomley wrote:

> diff --git a/mm/slub.c b/mm/slub.c
> index 94d2a33..243bd9c 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -235,7 +235,11 @@ int slab_is_available(void)
>  
>  static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
>  {
> +#ifdef CONFIG_NUMA
>  	return s->node[node];
> +#else
> +	return s->node[0];
> +#endif
>  }
>  
>  /* Verify that a pointer has an address that is valid within a slab page */

Looks like parisc may have been just fine before 7340cc84141d (slub: 
reduce differences between SMP and NUMA), which was merged into 2.6.37?
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley April 21, 2011, 10:31 p.m. UTC | #2
On Thu, 2011-04-21 at 15:19 -0700, David Rientjes wrote:
> On Thu, 21 Apr 2011, James Bottomley wrote:
> 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 94d2a33..243bd9c 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -235,7 +235,11 @@ int slab_is_available(void)
> >  
> >  static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
> >  {
> > +#ifdef CONFIG_NUMA
> >  	return s->node[node];
> > +#else
> > +	return s->node[0];
> > +#endif
> >  }
> >  
> >  /* Verify that a pointer has an address that is valid within a slab page */
> 
> Looks like parisc may have been just fine before 7340cc84141d (slub: 
> reduce differences between SMP and NUMA), which was merged into 2.6.37?

That's possible.  I've had no bug reports from the debian 2.6.32 kernel,
which is the only other one that has SLUB by default.  The m68k guys
seem to think this is the cause of their problems too.

But the basic fact is that all our testing has been done on SLAB.  It
wasn't until debian asked us to looks at a 2.6.38 kernel that I
accidentally picked up SLUB by importing their config into my build
environment.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/mm/slub.c b/mm/slub.c
index 94d2a33..243bd9c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -235,7 +235,11 @@  int slab_is_available(void)
 
 static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
 {
+#ifdef CONFIG_NUMA
 	return s->node[node];
+#else
+	return s->node[0];
+#endif
 }
 
 /* Verify that a pointer has an address that is valid within a slab page */