linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mel@csn.ul.ie>
To: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Minchan Kim <minchan.kim@gmail.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Christoph Lameter <cl@linux.com>,
	Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: [PATCH 1/2] mm,migration: Prevent rmap_walk_[anon|ksm] seeing the wrong VMA information
Date: Thu, 6 May 2010 14:45:51 +0100	[thread overview]
Message-ID: <20100506134550.GA8704@csn.ul.ie> (raw)
In-Reply-To: <4BE2C6E8.2030609@redhat.com>

On Thu, May 06, 2010 at 09:40:56AM -0400, Rik van Riel wrote:
> On 05/05/2010 01:34 PM, Linus Torvalds wrote:
>
>>   - you always lock the _deepest_ anon_vma you can find.
>
> The emphasis should be on "always" :)
>
>> That means just a single lock. And the "deepest" anon_vma is well-defined
>> for all anon_vma's, because each same_anon_vma chain is always rooted in
>> the original anon_vma that caused it.
>
> It should work, but only if we always take the deepest
> anon_vma lock.
>
> Not just in the migration code, but also in mmap, munmap,
> mprotect (for split_vma), expand_stack, etc...
>
> Otherwise we will still not provide exclusion of migrate
> vs. those events.
>

Are you sure?

I thought this as well but considered a situation something like

root anon_vma          <--- rmap_walk starts here
     anon_vma a
     anon_vma b
     anon_vma c        <--- an munmap/mmap/mprotect/etc here
     anon_vma d
     anon_vma e

The rmap_walk takes the root lock and then locks a, b, c, d and e as it
walks along.

The mSomething event happens on c and takes the lock

if rmap_walk gets there first, it takes the lock and the mSomething
event waits until the full rmap_walk is complete (delayed slightly but
no biggie).

if mSomething gets there first, rmap_walk will wait on taking the lock.
Again, there could be some delays but no biggie.

What am I missing?

> I'm guessing that means changing both anon_vma_lock and
> page_lock_anon_vma to always take the deepest anon_vma
> lock - not introducing a new function that is only called
> by the migration code.
>

That would be the case all right but I'd prefer to have PeterZ's patches
that do full reference counting of anon_vma first instead of introducing
RCU to those paths.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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:[~2010-05-06 13:46 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-05 13:14 [PATCH 0/2] Fix migration races in rmap_walk() V5 Mel Gorman
2010-05-05 13:14 ` [PATCH 1/2] mm,migration: Prevent rmap_walk_[anon|ksm] seeing the wrong VMA information Mel Gorman
2010-05-05 14:34   ` Linus Torvalds
2010-05-05 14:56     ` Mel Gorman
2010-05-05 15:31       ` Linus Torvalds
2010-05-05 15:54         ` Mel Gorman
2010-05-05 16:13           ` Andrea Arcangeli
2010-05-05 19:11             ` Peter Zijlstra
2010-05-05 19:57               ` Andrea Arcangeli
2010-05-21  0:27               ` Andrea Arcangeli
2010-05-06 10:37             ` Mel Gorman
2010-05-05 17:34           ` Linus Torvalds
2010-05-05 17:57             ` Linus Torvalds
2010-05-05 18:14             ` Mel Gorman
2010-05-05 18:34               ` Linus Torvalds
2010-05-06 11:03                 ` Mel Gorman
2010-05-06 13:40             ` Rik van Riel
2010-05-06 13:45               ` Mel Gorman [this message]
2010-05-05 17:53         ` Mel Gorman
2010-05-05 18:02           ` Linus Torvalds
2010-05-05 18:17             ` Mel Gorman
2010-05-06  0:22             ` Mel Gorman
2010-05-06  0:42               ` Linus Torvalds
2010-05-06 10:02                 ` Mel Gorman
2010-05-06 14:15                   ` Linus Torvalds
2010-05-06 14:25                     ` Mel Gorman
2010-05-06  9:47               ` Minchan Kim
2010-05-06  9:54                 ` Mel Gorman
2010-05-06 10:01                   ` Minchan Kim
2010-05-06 10:10                     ` Mel Gorman
2010-05-06 14:06                 ` Linus Torvalds
2010-05-06 15:59                   ` Minchan Kim
2010-05-06  7:38   ` KAMEZAWA Hiroyuki
2010-05-06  9:46     ` Mel Gorman
2010-05-06 23:52       ` KAMEZAWA Hiroyuki
2010-05-07  5:49         ` KAMEZAWA Hiroyuki
2010-05-05 13:14 ` [PATCH 2/2] mm,migration: Fix race between shift_arg_pages and rmap_walk by guaranteeing rmap_walk finds PTEs created within the temporary stack Mel Gorman
  -- strict thread matches above, loose matches on Subject: below --
2010-05-06 15:33 [PATCH 0/2] Fix migration races in rmap_walk() V6 Mel Gorman
2010-05-06 15:33 ` [PATCH 1/2] mm,migration: Prevent rmap_walk_[anon|ksm] seeing the wrong VMA information Mel Gorman
2010-05-06 15:44   ` Rik van Riel
2010-05-06 15:51     ` Mel Gorman
2010-05-06 15:59   ` Linus Torvalds
2010-05-06 17:07     ` Mel Gorman
2010-05-06 23:20 [PATCH 0/2] Fix migration races in rmap_walk() V7 Mel Gorman
2010-05-06 23:20 ` [PATCH 1/2] mm,migration: Prevent rmap_walk_[anon|ksm] seeing the wrong VMA information Mel Gorman
2010-05-07  0:56   ` KAMEZAWA Hiroyuki
2010-05-07 16:26     ` Mel Gorman
2010-05-08 15:39   ` Andrea Arcangeli
2010-05-08 17:02     ` Linus Torvalds
2010-05-08 18:04       ` Andrea Arcangeli
2010-05-08 19:51         ` Linus Torvalds
2010-05-09 19:23     ` Mel Gorman

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=20100506134550.GA8704@csn.ul.ie \
    --to=mel@csn.ul.ie \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan.kim@gmail.com \
    --cc=riel@redhat.com \
    --cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).