All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	Sachin Sant <sachinp@in.ibm.com>,
	linuxppc-dev@ozlabs.org, linux-next@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Christoph Lameter <cl@linux-foundation.org>
Subject: Re: Next April 28: boot failure on PowerPC with SLQB
Date: Thu, 30 Apr 2009 15:05:42 +0200	[thread overview]
Message-ID: <20090430130542.GF6900@wotan.suse.de> (raw)
In-Reply-To: <1241090429.19252.7.camel@penberg-laptop>

On Thu, Apr 30, 2009 at 02:20:29PM +0300, Pekka Enberg wrote:
> On Thu, 2009-04-30 at 13:18 +0200, Nick Piggin wrote:
> > OK thanks. So I think we have 2 problems. One with MAX_ORDER <= 9
> > that is fixed by the previous patch, and another which is probably
> > due to having no memory on node 0 which I will take another look
> > at now.
> > 
> > We can merge the previous patch now, though.
> 
> Hmm, I'll bet this BUG_ON triggers for Stephen.
> 
> diff --git a/mm/slqb.c b/mm/slqb.c
> index a651843..e4b3859 100644
> --- a/mm/slqb.c
> +++ b/mm/slqb.c
> @@ -1391,6 +1391,7 @@ static noinline void *__slab_alloc_page(struct kmem_cache *s,
>  		struct kmem_cache_node *n;
>  
>  		n = s->node_slab[slqb_page_to_nid(page)];
> +		BUG_ON(!n);
>  		l = &n->list;
>  		page->list = l;

Hmm, this might do it. The following code now passes some stress testing
in a userspace harness wheras before it did not (and was obviously wrong).

---
SLQB: fix dumb early allocation cache

The dumb early allocation cache had a bug where it could allow allocation
to go past the end of a page, which could cause crashes or random memory
corruption. Fix this and simplify the logic.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
 mm/slqb.c |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Index: linux-2.6/mm/slqb.c
===================================================================
--- linux-2.6.orig/mm/slqb.c
+++ linux-2.6/mm/slqb.c
@@ -2185,8 +2185,11 @@ static void *kmem_cache_dyn_array_alloc(
 {
 	size_t size = sizeof(void *) * ids;
 
+	BUG_ON(!size);
+
 	if (unlikely(!slab_is_available())) {
 		static void *nextmem;
+		static size_t nextleft;
 		void *ret;
 
 		/*
@@ -2194,16 +2197,16 @@ static void *kmem_cache_dyn_array_alloc(
 		 * never get freed by definition so we can do it rather
 		 * simply.
 		 */
-		if (!nextmem) {
-			nextmem = alloc_pages_exact(size, GFP_KERNEL);
-			if (!nextmem)
-				return NULL;
+		if (size > nextleft) {
+                        nextmem = alloc_pages_exact(size, GFP_KERNEL);
+                        if (!nextmem)
+                                return NULL;
+			nextleft = roundup(size, PAGE_SIZE);
 		}
+
 		ret = nextmem;
-		nextmem = (void *)((unsigned long)ret + size);
-		if ((unsigned long)ret >> PAGE_SHIFT !=
-				(unsigned long)nextmem >> PAGE_SHIFT)
-			nextmem = NULL;
+		nextleft -= size;
+		nextmem += size;
 		memset(ret, 0, size);
 		return ret;
 	} else {

WARNING: multiple messages have this Message-ID (diff)
From: Nick Piggin <npiggin@suse.de>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	Christoph Lameter <cl@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linuxppc-dev@ozlabs.org, linux-next@vger.kernel.org
Subject: Re: Next April 28: boot failure on PowerPC with SLQB
Date: Thu, 30 Apr 2009 15:05:42 +0200	[thread overview]
Message-ID: <20090430130542.GF6900@wotan.suse.de> (raw)
In-Reply-To: <1241090429.19252.7.camel@penberg-laptop>

On Thu, Apr 30, 2009 at 02:20:29PM +0300, Pekka Enberg wrote:
> On Thu, 2009-04-30 at 13:18 +0200, Nick Piggin wrote:
> > OK thanks. So I think we have 2 problems. One with MAX_ORDER <= 9
> > that is fixed by the previous patch, and another which is probably
> > due to having no memory on node 0 which I will take another look
> > at now.
> > 
> > We can merge the previous patch now, though.
> 
> Hmm, I'll bet this BUG_ON triggers for Stephen.
> 
> diff --git a/mm/slqb.c b/mm/slqb.c
> index a651843..e4b3859 100644
> --- a/mm/slqb.c
> +++ b/mm/slqb.c
> @@ -1391,6 +1391,7 @@ static noinline void *__slab_alloc_page(struct kmem_cache *s,
>  		struct kmem_cache_node *n;
>  
>  		n = s->node_slab[slqb_page_to_nid(page)];
> +		BUG_ON(!n);
>  		l = &n->list;
>  		page->list = l;

Hmm, this might do it. The following code now passes some stress testing
in a userspace harness wheras before it did not (and was obviously wrong).

---
SLQB: fix dumb early allocation cache

The dumb early allocation cache had a bug where it could allow allocation
to go past the end of a page, which could cause crashes or random memory
corruption. Fix this and simplify the logic.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
 mm/slqb.c |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Index: linux-2.6/mm/slqb.c
===================================================================
--- linux-2.6.orig/mm/slqb.c
+++ linux-2.6/mm/slqb.c
@@ -2185,8 +2185,11 @@ static void *kmem_cache_dyn_array_alloc(
 {
 	size_t size = sizeof(void *) * ids;
 
+	BUG_ON(!size);
+
 	if (unlikely(!slab_is_available())) {
 		static void *nextmem;
+		static size_t nextleft;
 		void *ret;
 
 		/*
@@ -2194,16 +2197,16 @@ static void *kmem_cache_dyn_array_alloc(
 		 * never get freed by definition so we can do it rather
 		 * simply.
 		 */
-		if (!nextmem) {
-			nextmem = alloc_pages_exact(size, GFP_KERNEL);
-			if (!nextmem)
-				return NULL;
+		if (size > nextleft) {
+                        nextmem = alloc_pages_exact(size, GFP_KERNEL);
+                        if (!nextmem)
+                                return NULL;
+			nextleft = roundup(size, PAGE_SIZE);
 		}
+
 		ret = nextmem;
-		nextmem = (void *)((unsigned long)ret + size);
-		if ((unsigned long)ret >> PAGE_SHIFT !=
-				(unsigned long)nextmem >> PAGE_SHIFT)
-			nextmem = NULL;
+		nextleft -= size;
+		nextmem += size;
 		memset(ret, 0, size);
 		return ret;
 	} else {

  parent reply	other threads:[~2009-04-30 13:06 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-28  6:53 linux-next: Tree for April 28 Stephen Rothwell
2009-04-28 11:10 ` Next April 28: boot failure on PowerPC with SLQB Sachin Sant
2009-04-28 11:22   ` Pekka Enberg
2009-04-28 11:22     ` Pekka Enberg
2009-04-29  7:04     ` Nick Piggin
2009-04-29  7:04       ` Nick Piggin
2009-04-29 11:36     ` Nick Piggin
2009-04-29 11:36       ` Nick Piggin
2009-04-29 16:26       ` Sachin Sant
2009-04-29 16:26         ` Sachin Sant
2009-04-30  3:21         ` Stephen Rothwell
2009-04-30  3:21           ` Stephen Rothwell
2009-04-30  3:21           ` Stephen Rothwell
2009-04-30  4:11         ` Nick Piggin
2009-04-30  4:11           ` Nick Piggin
2009-04-30  5:36           ` Sachin Sant
2009-04-30  5:36             ` Sachin Sant
2009-04-30  6:03             ` Nick Piggin
2009-04-30  6:03               ` Nick Piggin
2009-04-30  6:42               ` Sachin Sant
2009-04-30  6:42                 ` Sachin Sant
2009-04-30  6:41             ` Nick Piggin
2009-04-30  6:41               ` Nick Piggin
2009-04-30  9:47               ` Sachin Sant
2009-04-30  9:47                 ` Sachin Sant
2009-04-30 10:35                 ` Nick Piggin
2009-04-30 10:35                   ` Nick Piggin
2009-04-30 10:38                   ` Pekka Enberg
2009-04-30 10:38                     ` Pekka Enberg
2009-04-30 11:00                     ` Stephen Rothwell
2009-04-30 11:00                       ` Stephen Rothwell
2009-04-30 11:10                       ` Pekka Enberg
2009-04-30 11:10                         ` Pekka Enberg
2009-04-30 11:18                       ` Nick Piggin
2009-04-30 11:18                         ` Nick Piggin
2009-04-30 11:20                         ` Pekka Enberg
2009-04-30 11:20                           ` Pekka Enberg
2009-04-30 11:26                           ` Nick Piggin
2009-04-30 11:26                             ` Nick Piggin
2009-04-30 13:05                           ` Nick Piggin [this message]
2009-04-30 13:05                             ` Nick Piggin
2009-04-30 14:00                             ` Stephen Rothwell
2009-04-30 14:00                               ` Stephen Rothwell
2009-04-30 14:10                               ` Nick Piggin
2009-04-30 14:10                                 ` Nick Piggin
2009-05-03 11:51                                 ` Pekka Enberg
2009-05-03 11:51                                   ` Pekka Enberg
2009-04-30 14:10                             ` Anton Vorontsov
2009-04-30 14:10                               ` Anton Vorontsov
2009-05-03 11:51                               ` Pekka Enberg
2009-04-28 15:22 ` [PATCH] lockd: fix FILE_LOCKING=n build error Randy Dunlap
2009-04-28 15:38   ` Felix Blyakher
2009-04-28 17:21   ` J. Bruce Fields
2009-04-28 17:51     ` Randy Dunlap
2009-04-28 18:15       ` Trond Myklebust
2009-04-28 21:01         ` [PATCH v2] " Randy Dunlap
2009-04-28 21:40           ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090430130542.GF6900@wotan.suse.de \
    --to=npiggin@suse.de \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=penberg@cs.helsinki.fi \
    --cc=sachinp@in.ibm.com \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.