linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Lameter <cl@linux-foundation.org>
To: Robin Holt <holt@sgi.com>
Cc: linux-mm@kvack.org, aarcange@redhat.com,
	Nick Piggin <npiggin@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [Patch] mmu_notifiers destroyed by __mmu_notifier_release() retain extra mm_count.
Date: Thu, 5 Feb 2009 18:54:33 -0500 (EST)	[thread overview]
Message-ID: <alpine.DEB.1.10.0902051844390.17441@qirst.com> (raw)
In-Reply-To: <20090205200214.GN8577@sgi.com>

On Thu, 5 Feb 2009, Robin Holt wrote:

> On Thu, Feb 05, 2009 at 02:30:29PM -0500, Christoph Lameter wrote:
> > The drop of the refcount needs to occur  after the last use of
> > data in the mmstruct because mmdrop() may free the mmstruct.
>
> Not this time.  We are being called from process termination and the
> calling function is assured to hold one reference count.

Maybe add a comment that says that this is a requirement for the
caller? mmdrop() has logic to free the mmstruct.

One also needs to wonder why we acquire the refcount for the mmu
notifier on the mmstruct at all. Maybe remove the

	atomic_inc()

from mmu_notifier_register() instead? Looks strange there especially since
we have a BUG_ON there as well that verifies that the number of refcount
is already above 0.

How about this patch instead?


Subject: mmu_notifier: Remove superfluous increase of the mm refcount

The mm refcount is handled by the caller of mmu_notifier_register and
mmu_notifier_unregister(). There is no need to increase the refcount.
Increasing the refcount led to a memory leak.

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

Index: linux-2.6/mm/mmu_notifier.c
===================================================================
--- linux-2.6.orig/mm/mmu_notifier.c	2009-02-05 17:55:27.000000000 -0600
+++ linux-2.6/mm/mmu_notifier.c	2009-02-05 17:55:31.000000000 -0600
@@ -167,7 +167,6 @@
 		mm->mmu_notifier_mm = mmu_notifier_mm;
 		mmu_notifier_mm = NULL;
 	}
-	atomic_inc(&mm->mm_count);

 	/*
 	 * Serialize the update against mmu_notifier_unregister. A



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2009-02-05 23:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-05 17:23 Robin Holt
2009-02-05 19:30 ` Christoph Lameter
2009-02-05 20:02   ` Robin Holt
2009-02-05 23:54     ` Christoph Lameter [this message]
2009-02-06  1:38       ` Andrea Arcangeli
2009-02-06  1:44         ` Andrea Arcangeli
2009-02-06 12:58           ` Robin Holt
2009-02-06 16:56             ` Andrea Arcangeli
2009-02-05 21:20 ` Andrea Arcangeli
2009-02-05 22:25 ` Andrew Morton

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=alpine.DEB.1.10.0902051844390.17441@qirst.com \
    --to=cl@linux-foundation.org \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=holt@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@suse.de \
    /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