linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Tommi Rantala <tt.rantala@gmail.com>,
	Stable <stable@vger.kernel.org>, Andi Kleen <ak@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Dave Jones <davej@redhat.com>, Christoph Lameter <cl@linux.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH 5/5] mempolicy: fix a memory corruption by refcount imbalance in alloc_pages_vma()
Date: Tue, 4 Dec 2012 22:28:33 -0800 (PST)	[thread overview]
Message-ID: <alpine.LNX.2.00.1212042211340.892@eggly.anvils> (raw)
In-Reply-To: <alpine.LNX.2.00.1212042042130.13895@eggly.anvils>

On Tue, 4 Dec 2012, Hugh Dickins wrote:
> 
> Yes, your patch fixes it Mel, but I prefer it as below, with a couple
> of mods: removing the no longer true comment, and leaving shmem_swapin()
> alone with just a comment.  It appears to be the job of the rather weird
> mpol_cond_copy() to drop the reference on the original mempolicy, and
> clear MPOL_F_SHARED so the copy won't need one (it's trying to cope with
> the fact that swapin_readahead will make an unknown number of calls to
> alloc_page_vma).  So I'd rather not add another mpol_cond_put there,
> whose cond will never be met.

Hold on, ignore that patch for now, I think I had my priorities
upside down: it would be better for shmem_swapin() to behave as
you proposed, and we delete the mpol_cond_copy() weirdness instead.

Your 00442ad04a5e changed alloc_pages_vma() to keep its refcounting
in balance, so it now does not matter that swapin_readahead() makes
an unknown number of calls to it: we should simply take a reference
before and drop it after, just as you do in shmem_alloc_page().

I'd still like to revisit alloc_page_vma(), and its refcount
manipulations do now appear redundant; but changing that is not
something I want to get into in a last minute rush.  But getting rid
of mpol_cond_copy() should be safe and clear, I'll test that out now
and reply with an updated patch (or else admit I got confused).

Hugh

--
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:[~2012-12-05  6:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-09 16:58 [PATCH 0/5] Memory policy corruption fixes -stable Mel Gorman
2012-10-09 16:58 ` [PATCH 1/5] revert "mm: mempolicy: Let vma_merge and vma_split handle vma->vm_policy linkages" Mel Gorman
2012-10-09 16:58 ` [PATCH 2/5] mempolicy: remove mempolicy sharing Mel Gorman
2012-10-09 16:58 ` [PATCH 3/5] mempolicy: fix a race in shared_policy_replace() Mel Gorman
2012-10-09 16:58 ` [PATCH 4/5] mempolicy: fix refcount leak in mpol_set_shared_policy() Mel Gorman
2012-10-09 16:58 ` [PATCH 5/5] mempolicy: fix a memory corruption by refcount imbalance in alloc_pages_vma() Mel Gorman
2012-12-04 12:54   ` Tommi Rantala
2012-12-04 14:15     ` Mel Gorman
2012-12-05  5:11       ` Hugh Dickins
2012-12-05  6:28         ` Hugh Dickins [this message]
2012-12-05  7:24           ` [PATCH] tmpfs: fix shared mempolicy leak Hugh Dickins
2012-12-05  9:52             ` Mel Gorman
2012-12-05 20:25               ` Tommi Rantala
2012-12-05 21:59                 ` Hugh Dickins
2012-12-05 22:01                   ` Hugh Dickins
2012-10-10  0:47 ` [PATCH 0/5] Memory policy corruption fixes -stable Greg KH
2012-10-14  9:13 ` Ben Hutchings
  -- strict thread matches above, loose matches on Subject: below --
2012-08-20 16:36 [PATCH 0/5] Memory policy corruption fixes V2 Mel Gorman
2012-08-20 16:36 ` [PATCH 5/5] mempolicy: fix a memory corruption by refcount imbalance in alloc_pages_vma() Mel Gorman
2012-08-20 19:51   ` Christoph Lameter
2012-08-21  7:26     ` Mel Gorman
2012-08-21 15:37       ` Christoph Lameter
2012-09-07 23:06       ` KOSAKI Motohiro

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.LNX.2.00.1212042211340.892@eggly.anvils \
    --to=hughd@google.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=davej@redhat.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=stable@vger.kernel.org \
    --cc=tt.rantala@gmail.com \
    /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