lkml.org 
[lkml]   [2007]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 3/6] mm: fix fault vs invalidate race for linear mappings
    On Wed, 21 Feb 2007 05:50:05 +0100 (CET) Nick Piggin <npiggin@suse.de> wrote:

    > Fix the race between invalidate_inode_pages and do_no_page.
    >
    > Andrea Arcangeli identified a subtle race between invalidation of
    > pages from pagecache with userspace mappings, and do_no_page.
    >
    > The issue is that invalidation has to shoot down all mappings to the
    > page, before it can be discarded from the pagecache. Between shooting
    > down ptes to a particular page, and actually dropping the struct page
    > from the pagecache, do_no_page from any process might fault on that
    > page and establish a new mapping to the page just before it gets
    > discarded from the pagecache.
    >
    > The most common case where such invalidation is used is in file
    > truncation. This case was catered for by doing a sort of open-coded
    > seqlock between the file's i_size, and its truncate_count.
    >
    > Truncation will decrease i_size, then increment truncate_count before
    > unmapping userspace pages; do_no_page will read truncate_count, then
    > find the page if it is within i_size, and then check truncate_count
    > under the page table lock and back out and retry if it had
    > subsequently been changed (ptl will serialise against unmapping, and
    > ensure a potentially updated truncate_count is actually visible).
    >
    > Complexity and documentation issues aside, the locking protocol fails
    > in the case where we would like to invalidate pagecache inside i_size.
    > do_no_page can come in anytime and filemap_nopage is not aware of the
    > invalidation in progress (as it is when it is outside i_size). The
    > end result is that dangling (->mapping == NULL) pages that appear to
    > be from a particular file may be mapped into userspace with nonsense
    > data. Valid mappings to the same place will see a different page.
    >
    > Andrea implemented two working fixes, one using a real seqlock,
    > another using a page->flags bit. He also proposed using the page lock
    > in do_no_page, but that was initially considered too heavyweight.
    > However, it is not a global or per-file lock, and the page cacheline
    > is modified in do_no_page to increment _count and _mapcount anyway, so
    > a further modification should not be a large performance hit.
    > Scalability is not an issue.
    >
    > This patch implements this latter approach. ->nopage implementations
    > return with the page locked if it is possible for their underlying
    > file to be invalidated (in that case, they must set a special vm_flags
    > bit to indicate so). do_no_page only unlocks the page after setting
    > up the mapping completely. invalidation is excluded because it holds
    > the page lock during invalidation of each page (and ensures that the
    > page is not mapped while holding the lock).
    >
    > This also allows significant simplifications in do_no_page, because
    > we have the page locked in the right place in the pagecache from the
    > start.
    >

    Why was truncate_inode_pages_range() altered to unmap the page if it got
    mapped again?

    Oh. Because the unmap_mapping_range() call got removed from vmtruncate().
    Why? (Please send suitable updates to the changelog).

    I guess truncate of a mmapped area isn't sufficiently common to worry about
    the inefficiency of this change.

    Lots of memory barriers got removed in memory.c, unchangeloggedly.

    Gratuitous renaming of locals in do_no_page() makes the change hard to
    review. Should have been a separate patch.

    In fact, the patch would have been heaps clearer if that renaming had been
    a separate patch.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-03-07 07:39    [W:4.250 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site