linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Lameter <cl@linux.com>
To: kosaki.motohiro@gmail.com
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Andrew Morton <akpm@google.com>, Dave Jones <davej@redhat.com>,
	Mel Gorman <mgorman@suse.de>,
	stable@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 2/6] mempolicy: remove all mempolicy sharing
Date: Mon, 11 Jun 2012 10:02:17 -0500 (CDT)	[thread overview]
Message-ID: <alpine.DEB.2.00.1206110944120.31180@router.home> (raw)
In-Reply-To: <1339406250-10169-3-git-send-email-kosaki.motohiro@gmail.com>

Some more attempts to cleanup changelogs:

> The problem was created by a reference count imbalance. Example, In following case,
> mbind(addr, len) try to replace mempolicies of vma1 and vma2 and then they will
> be share the same mempolicy, and the new mempolicy has MPOL_F_SHARED flag.

The bug that we saw <where ? details?> was created by a refcount
imbalance. If mbind() replaces the memory policies of vma1 and vma and
they share the same shared mempolicy (MPOL_F_SHARED set) then an imbalance
may occur.

>   +-------------------+-------------------+
>   |     vma1          |     vma2(shmem)   |
>   +-------------------+-------------------+
>   |                                       |
>  addr                                 addr+len
>
> Look at alloc_pages_vma(), it uses get_vma_policy() and mpol_cond_put() pair
> for maintaining mempolicy refcount. The current rule is, get_vma_policy() does
> NOT increase a refcount if the policy is not attached shmem vma and mpol_cond_put()
> DOES decrease a refcount if mpol has MPOL_F_SHARED.

alloc_pages_vma() uses the two function get_vma_policy() and
mpol_cond_put() to maintain the refcount on the memory policies. However,
the current rule is that get_vma_policy() does *not* increase the refcount
if the policy is not attached to a shm vma. mpol_cond_put *does* decrease
the refcount if the memory policy has MPOL_F_SHARED set.

> In above case, vma1 is not shmem vma and vma->policy has MPOL_F_SHARED! then,
> get_vma_policy() doesn't increase a refcount and mpol_cond_put() decrease a
> refcount whenever alloc_page_vma() is called.
>
> The bug was introduced by commit 52cd3b0740 (mempolicy: rework mempolicy Reference
> Counting) at 4 years ago.
>
> More unfortunately mempolicy has one another serious broken. Currently,
> mempolicy rebind logic (it is called from cpuset rebinding) ignore a refcount
> of mempolicy and override it forcibly. Thus, any mempolicy sharing may
> cause mempolicy corruption. The bug was introduced by commit 68860ec10b
> (cpusets: automatic numa mempolicy rebinding) at 7 years ago.

Memory policies have another issue. Currently the mempolicy rebind logic
used for cpuset rebinding ignores the refcount of memory policies.
Therefore, any memory policy sharing can cause refcount mismatches. The
bug was ...

> To disable policy sharing solves user visible breakage and this patch does it.
> Maybe, we need to rewrite MPOL_F_SHARED and mempolicy rebinding code and aim
> to proper cow logic eventually, but I think this is good first step.

Disabling policy sharing solves the breakage and that is how this patch
fixes the issue for now. Rewriting the shared policy handling with proper
COW logic support will be necessary to cleanly address the
problem and allow proper sharing of memory policies.

--
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-06-11 15:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-11  9:17 [PATCH 0/6][resend] mempolicy memory corruption fixlet kosaki.motohiro
2012-06-11  9:17 ` [PATCH 1/6] Revert "mm: mempolicy: Let vma_merge and vma_split handle vma->vm_policy linkages" kosaki.motohiro
2012-06-11 14:43   ` Christoph Lameter
2012-06-11  9:17 ` [PATCH 2/6] mempolicy: remove all mempolicy sharing kosaki.motohiro
2012-06-11 15:02   ` Christoph Lameter [this message]
2012-06-12 16:46     ` KOSAKI Motohiro
2012-06-12 13:55   ` Mel Gorman
2012-06-12 16:45     ` KOSAKI Motohiro
2012-06-11  9:17 ` [PATCH 3/6] mempolicy: fix a race in shared_policy_replace() kosaki.motohiro
2012-06-11  9:17 ` [PATCH 4/6] mempolicy: fix refcount leak in mpol_set_shared_policy() kosaki.motohiro
2012-06-11  9:17 ` [PATCH 5/6] mempolicy: fix a memory corruption by refcount imbalance in alloc_pages_vma() kosaki.motohiro
2012-06-11 13:33   ` Ben Hutchings
2012-06-11 15:24     ` KOSAKI Motohiro
2012-06-12 14:20   ` Mel Gorman
2012-06-12 16:31     ` KOSAKI Motohiro
2012-06-11  9:17 ` [PATCH 6/6] MAINTAINERS: Added MEMPOLICY entry kosaki.motohiro
2012-06-11 15:01 ` [PATCH 0/6][resend] mempolicy memory corruption fixlet Christoph Lameter
2012-07-31 12:33 ` Josh Boyer
2012-08-06 19:32   ` KOSAKI Motohiro
2012-08-15 11:40     ` Josh Boyer
2012-08-15 20:20       ` 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.2.00.1206110944120.31180@router.home \
    --to=cl@linux.com \
    --cc=akpm@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=kosaki.motohiro@gmail.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 \
    /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