All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: blaisorblade@yahoo.it
Cc: Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org,
	Linux Memory Management <linux-mm@kvack.org>
Subject: Re: [patch 11/14] remap_file_pages protection support: pte_present should not trigger on PTE_FILE PROTNONE ptes
Date: Tue, 02 May 2006 13:53:28 +1000	[thread overview]
Message-ID: <4456D7B8.2000004@yahoo.com.au> (raw)
In-Reply-To: <20060430173025.752423000@zion.home.lan>

blaisorblade@yahoo.it wrote:
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> pte_present(pte) implies that pte_pfn(pte) is valid. Normally even with a
> _PAGE_PROTNONE pte this holds, but not when such a PTE is installed by
> the new install_file_pte; previously it didn't store protections, only file
> offsets, with the patches it also stores protections, and can set
> _PAGE_PROTNONE|_PAGE_FILE.

Why is this combination useful? Can't you just drop the _PAGE_FILE from
_PAGE_PROTNONE ptes?

> 
> zap_pte_range, when acting on such a pte, calls vm_normal_page and gets
> &mem_map[0], does page_remove_rmap, and we're easily in trouble, because it
> happens to find a page with mapcount == 0. And it BUGs on this!
> 
> I've seen this trigger easily and repeatably on UML on 2.6.16-rc3. This was
> likely avoided in the past by the PageReserved test - page 0 *had* to be
> reserved on i386 (dunno on UML).
> 
> Implementation follows for UML and i386.
> 
> To avoid additional overhead, I also considered adding likely() for
> _PAGE_PRESENT and unlikely() for the rest, but I'm uncertain about validity of
> possible [un]likely(pte_present()) occurrences.

Not present pages are likely to be pretty common when unmapping.

I don't like this patch much.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

WARNING: multiple messages have this Message-ID (diff)
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: blaisorblade@yahoo.it
Cc: Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org,
	Linux Memory Management <linux-mm@kvack.org>
Subject: Re: [patch 11/14] remap_file_pages protection support: pte_present should not trigger on PTE_FILE PROTNONE ptes
Date: Tue, 02 May 2006 13:53:28 +1000	[thread overview]
Message-ID: <4456D7B8.2000004@yahoo.com.au> (raw)
In-Reply-To: <20060430173025.752423000@zion.home.lan>

blaisorblade@yahoo.it wrote:
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> pte_present(pte) implies that pte_pfn(pte) is valid. Normally even with a
> _PAGE_PROTNONE pte this holds, but not when such a PTE is installed by
> the new install_file_pte; previously it didn't store protections, only file
> offsets, with the patches it also stores protections, and can set
> _PAGE_PROTNONE|_PAGE_FILE.

Why is this combination useful? Can't you just drop the _PAGE_FILE from
_PAGE_PROTNONE ptes?

> 
> zap_pte_range, when acting on such a pte, calls vm_normal_page and gets
> &mem_map[0], does page_remove_rmap, and we're easily in trouble, because it
> happens to find a page with mapcount == 0. And it BUGs on this!
> 
> I've seen this trigger easily and repeatably on UML on 2.6.16-rc3. This was
> likely avoided in the past by the PageReserved test - page 0 *had* to be
> reserved on i386 (dunno on UML).
> 
> Implementation follows for UML and i386.
> 
> To avoid additional overhead, I also considered adding likely() for
> _PAGE_PRESENT and unlikely() for the rest, but I'm uncertain about validity of
> possible [un]likely(pte_present()) occurrences.

Not present pages are likely to be pretty common when unmapping.

I don't like this patch much.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
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:[~2006-05-02 10:45 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-30 17:29 [patch 00/14] remap_file_pages protection support blaisorblade
2006-04-30 17:29 ` [patch 01/14] Fix comment about remap_file_pages blaisorblade
2006-04-30 17:29 ` [patch 02/14] remap_file_pages protection support: add needed macros blaisorblade
2006-04-30 17:29 ` [patch 03/14] remap_file_pages protection support: handle MANYPROTS VMAs blaisorblade
2006-04-30 17:29 ` [patch 04/14] remap_file_pages protection support: disallow mprotect() on manyprots mappings blaisorblade
2006-04-30 17:29 ` [patch 05/14] remap_file_pages protection support: cleanup syscall checks blaisorblade
2006-04-30 17:29 ` [patch 06/14] remap_file_pages protection support: enhance syscall interface blaisorblade
2006-04-30 17:30 ` [patch 07/14] remap_file_pages protection support: support private vma for MAP_POPULATE blaisorblade
2006-04-30 17:30 ` [patch 08/14] remap_file_pages protection support: use FAULT_SIGSEGV for protection checking blaisorblade
2006-04-30 17:30 ` [patch 09/14] remap_file_pages protection support: fix race condition with concurrent faults on same address space blaisorblade
2006-04-30 17:30 ` [patch 10/14] remap_file_pages protection support: fix get_user_pages() on VM_MANYPROTS vmas blaisorblade
2006-04-30 17:30 ` [patch 11/14] remap_file_pages protection support: pte_present should not trigger on PTE_FILE PROTNONE ptes blaisorblade
2006-05-02  3:53   ` Nick Piggin [this message]
2006-05-02  3:53     ` Nick Piggin
2006-05-03  1:29     ` Blaisorblade
2006-05-03  1:29       ` Blaisorblade
2006-05-06 10:03       ` Nick Piggin
2006-05-06 10:03         ` Nick Piggin
2006-05-07 17:50         ` Blaisorblade
2006-05-07 17:50           ` Blaisorblade
2006-04-30 17:30 ` [patch 12/14] remap_file_pages protection support: also set VM_NONLINEAR on nonuniform VMAs blaisorblade
2006-04-30 17:30 ` [patch 13/14] remap_file_pages protection support: uml, i386, x64 bits blaisorblade
2006-04-30 17:30 ` [patch 14/14] remap_file_pages protection support: adapt to uml peculiarities blaisorblade
2006-05-02  3:45 ` [patch 00/14] remap_file_pages protection support Nick Piggin
2006-05-02  3:45   ` Nick Piggin
2006-05-02  3:56   ` Nick Piggin
2006-05-02  3:56     ` Nick Piggin
2006-05-02 11:24     ` Ingo Molnar
2006-05-02 11:24       ` Ingo Molnar
2006-05-02 12:19       ` Nick Piggin
2006-05-02 12:19         ` Nick Piggin
2006-05-02 17:16   ` Lee Schermerhorn
2006-05-02 17:16     ` Lee Schermerhorn
2006-05-03  1:20     ` Blaisorblade
2006-05-03  1:20       ` Blaisorblade
2006-05-03 14:35       ` Lee Schermerhorn
2006-05-03 14:35         ` Lee Schermerhorn
2006-05-03  0:25   ` Blaisorblade
2006-05-03  0:25     ` Blaisorblade
2006-05-06 16:05     ` Ulrich Drepper
2006-05-07  4:22       ` Nick Piggin
2006-05-07  4:22         ` Nick Piggin
2006-05-13 14:13         ` Nick Piggin
2006-05-13 18:19           ` Valerie Henson
2006-05-13 18:19             ` Valerie Henson
2006-05-13 22:54             ` Valerie Henson
2006-05-13 22:54               ` Valerie Henson
2006-05-16 13:30             ` Nick Piggin
2006-05-16 13:30               ` Nick Piggin
2006-05-16 13:51               ` Andreas Mohr
2006-05-16 13:51                 ` Andreas Mohr
2006-05-16 16:31                 ` Valerie Henson
2006-05-16 16:31                   ` Valerie Henson
2006-05-16 16:47                   ` Andreas Mohr
2006-05-16 16:47                     ` Andreas Mohr
2006-05-17  3:25                     ` Nick Piggin
2006-05-17  3:25                       ` Nick Piggin
2006-05-17  6:10                     ` Blaisorblade
2006-05-17  6:10                       ` Blaisorblade
2006-05-16 16:33               ` Valerie Henson
2006-05-16 16:33                 ` Valerie Henson
2006-05-03  0:44   ` Blaisorblade
2006-05-03  0:44     ` Blaisorblade
2006-05-06  9:06     ` Nick Piggin
2006-05-06  9:06       ` Nick Piggin
2006-05-06 15:26       ` Ulrich Drepper
2006-05-02 10:21 ` Arjan van de Ven
2006-05-02 23:46   ` Valerie Henson
2006-05-03  0:26   ` Blaisorblade
2006-05-03  1:44     ` Ulrich Drepper

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=4456D7B8.2000004@yahoo.com.au \
    --to=nickpiggin@yahoo.com.au \
    --cc=akpm@osdl.org \
    --cc=blaisorblade@yahoo.it \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.