linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Minchan Kim <minchan.kim@gmail.com>
To: Rik van Riel <riel@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	lwoodman@redhat.com, akpm@linux-foundation.org,
	Lee Schermerhorn <Lee.Schermerhorn@hp.com>,
	aarcange@redhat.com
Subject: Re: [PATCH -mm] change anon_vma linking to fix multi-process server scalability issue
Date: Fri, 29 Jan 2010 09:55:04 +0900	[thread overview]
Message-ID: <28c262361001281655x70e5f77awf4d890d20f57ca83@mail.gmail.com> (raw)
In-Reply-To: <4B61C83A.20301@redhat.com>

On Fri, Jan 29, 2010 at 2:24 AM, Rik van Riel <riel@redhat.com> wrote:
>>> -void vma_adjust(struct vm_area_struct *vma, unsigned long start,
>>> +int vma_adjust(struct vm_area_struct *vma, unsigned long start,
>>>        unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert)
>>>  {
>>>        struct mm_struct *mm = vma->vm_mm;
>>> @@ -542,6 +541,29 @@ again:                     remove_next = 1 + (end>
>>>  next->vm_end);
>>>                }
>>>        }
>>>
>>> +       /*
>>> +        * When changing only vma->vm_end, we don't really need
>>> +        * anon_vma lock.
>>> +        */
>>> +       if (vma->anon_vma&&  (insert || importer || start !=
>>> vma->vm_start))
>>> +               anon_vma = vma->anon_vma;
>>> +       if (anon_vma) {
>>> +               /*
>>> +                * Easily overlooked: when mprotect shifts the boundary,
>>> +                * make sure the expanding vma has anon_vma set if the
>>> +                * shrinking vma had, to cover any anon pages imported.
>>> +                */
>>> +               if (importer&&  !importer->anon_vma) {
>>> +                       /* Block reverse map lookups until things are set
>>> up. */
>>> +                       importer->vm_flags |= VM_LOCK_RMAP;
>>> +                       if (anon_vma_clone(importer, vma)) {
>>> +                               importer->vm_flags&= ~VM_LOCK_RMAP;
>>> +                               return -ENOMEM;
>>
>> If we fail in here during progressing on next vmas in case of mprotect
>> case 6,
>> the previous vmas would become inconsistent state.
>
> I've re-read the code, but I don't see what you are referring
> to.  If vma_adjust bails out early, no VMAs will be adjusted
> and all the VMAs will stay the way they were before mprotect
> was called.
>
> What am I overlooking?

I also look at the code more detail and found me wrong.
In mprotect case 6,  the importer is fixed as head of vmas while next
is marched
on forward. So anon_vma_clone is just called once at first time.
So as what you said, It's no problem.
Totally, my mistake. Sorry for that, Rik.

-- 
Kind regards,
Minchan Kim

--
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-01-29  0:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-28  5:20 [PATCH -mm] change anon_vma linking to fix multi-process server scalability issue Rik van Riel
2010-01-28  6:43 ` [PATCH -mm] rmap: remove obsolete check from __page_check_anon_rmap Rik van Riel
2010-02-01 15:26   ` Minchan Kim
2010-01-28  6:43 ` [PATCH -mm] rmap: move exclusively owned pages to own anon_vma in do_wp_page Rik van Riel
2010-02-01 15:25   ` Minchan Kim
2010-02-01 16:33     ` Rik van Riel
2010-01-28 16:37 ` [PATCH -mm] change anon_vma linking to fix multi-process server scalability issue Minchan Kim
2010-01-28 17:24   ` Rik van Riel
2010-01-29  0:55     ` Minchan Kim [this message]
2010-01-29  1:18       ` Rik van Riel
2010-01-29 23:14 ` Andrew Morton
2010-01-29 23:57   ` Rik van Riel
2010-01-30  0:22   ` [PATCH -mm] further cleanups to " Rik van Riel
2010-01-30  0:34   ` [PATCH -mm] remove VM_LOCK_RMAP code Rik van Riel
2010-02-01  6:15     ` Nick Piggin
2010-02-01 15:55       ` Rik van Riel
2010-02-02  6:44         ` Nick Piggin

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=28c262361001281655x70e5f77awf4d890d20f57ca83@mail.gmail.com \
    --to=minchan.kim@gmail.com \
    --cc=Lee.Schermerhorn@hp.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lwoodman@redhat.com \
    --cc=riel@redhat.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;
as well as URLs for NNTP newsgroup(s).