mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christoph Lameter <clameter@sgi.com>
To: stable@kernel.org
Cc: "Hugh Dickins" <hugh@veritas.com>,
	"Olivér Pintér" <oliver.pntr@gmail.com>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Willy Tarreau" <w@1wt.eu>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] slub: fix leakage
Date: Mon, 5 Nov 2007 11:15:43 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0711051114560.14913@schroedinger.engr.sgi.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0711051901330.28390@blonde.wat.veritas.com>

On Mon, 5 Nov 2007, Hugh Dickins wrote:

> Okay, I wanted to make the point, but I've no wish to hold up your fix
> (and removing code, particularly code that has given trouble, is always
> welcome).  Please go ahead - thanks.

Here is the fix against 2.6.23:

SLUB: Fix memory leak by not reusing cpu_slab

Fix the memory leak that may occur when we attempt to reuse a cpu_slab
that was allocated while we reenabled interrupts in order to be able to
grow a slab cache. The per cpu freelist may contain objects and in that
situation we may overwrite the per cpu freelist pointer loosing objects.
This only occurs if we find that the concurrently allocated slab fits
our allocation needs.

If we simply always deactivate the slab then the freelist will be properly
reintegrated and the memory leak will go away.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 mm/slub.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

Index: linux-2.6.23/mm/slub.c
===================================================================
--- linux-2.6.23.orig/mm/slub.c	2007-10-09 13:31:38.000000000 -0700
+++ linux-2.6.23/mm/slub.c	2007-11-05 11:09:49.000000000 -0800
@@ -1501,28 +1501,8 @@ new_slab:
 	page = new_slab(s, gfpflags, node);
 	if (page) {
 		cpu = smp_processor_id();
-		if (s->cpu_slab[cpu]) {
-			/*
-			 * Someone else populated the cpu_slab while we
-			 * enabled interrupts, or we have gotten scheduled
-			 * on another cpu. The page may not be on the
-			 * requested node even if __GFP_THISNODE was
-			 * specified. So we need to recheck.
-			 */
-			if (node == -1 ||
-				page_to_nid(s->cpu_slab[cpu]) == node) {
-				/*
-				 * Current cpuslab is acceptable and we
-				 * want the current one since its cache hot
-				 */
-				discard_slab(s, page);
-				page = s->cpu_slab[cpu];
-				slab_lock(page);
-				goto load_freelist;
-			}
-			/* New slab does not fit our expectations */
+		if (s->cpu_slab[cpu])
 			flush_slab(s, s->cpu_slab[cpu], cpu);
-		}
 		slab_lock(page);
 		SetSlabFrozen(page);
 		s->cpu_slab[cpu] = page;

  reply	other threads:[~2007-11-05 19:15 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-03 17:10 Hugh Dickins
2007-11-03 17:11 ` [PATCH 2/2] slub: fix Objects count Hugh Dickins
2007-11-03 18:26   ` Christoph Lameter
2007-11-03 19:09     ` Hugh Dickins
2007-11-03 19:33       ` Christoph Lameter
2007-11-03 20:03         ` Hugh Dickins
2007-11-03 17:27 ` [PATCH 1/2] slub: fix leakage Olivér Pintér
2007-11-03 17:50   ` Hugh Dickins
2007-11-03 18:52     ` Hugh Dickins
2007-11-03 19:39       ` Hugh Dickins
2007-11-03 19:47         ` Christoph Lameter
2007-11-03 19:52           ` Hugh Dickins
2007-11-03 19:54             ` Christoph Lameter
2007-11-03 20:10               ` Hugh Dickins
2007-11-03 20:26                 ` Christoph Lameter
2007-11-04 11:18                   ` Hugh Dickins
2007-11-05 18:45                     ` Christoph Lameter
2007-11-05 19:09                       ` Hugh Dickins
2007-11-05 19:15                         ` Christoph Lameter [this message]
2007-11-05 19:23                           ` Christoph Lameter
2007-11-05 19:26                           ` Linus Torvalds
2007-11-05 19:31                             ` Christoph Lameter
2007-11-05 19:36                               ` Linus Torvalds
2007-11-06  6:15                       ` Jeff Chua
2007-11-06  7:58                         ` Hugh Dickins
2007-11-03 17:38 ` Willy Tarreau
2007-11-03 17:55   ` Hugh Dickins
2007-11-03 17:59     ` Willy Tarreau
2007-11-03 18:31 ` Christoph Lameter
2007-11-03 18:48 ` Christoph Lameter
2007-11-03 18:51   ` Christoph Lameter
2007-11-03 19:04   ` Hugh Dickins
2007-11-03 19:26     ` Christoph Lameter
2007-11-03 19:35       ` Hugh Dickins

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=Pine.LNX.4.64.0711051114560.14913@schroedinger.engr.sgi.com \
    --to=clameter@sgi.com \
    --cc=akpm@linux-foundation.org \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oliver.pntr@gmail.com \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=w@1wt.eu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®