lkml.org 
[lkml]   [2007]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] VM: invalidate_inode_pages2_range() shouldn't fail on page dirty...
    On Mon, 12 Feb 2007 23:43:38 -0800
    Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

    > From: Trond Myklebust <Trond.Myklebust@netapp.com>
    >
    > invalidate_inode_pages2() should not try to fix races between direct_IO and
    > mmap(). It should only be trying to clear out pages that were dirty before
    > the direct_IO write (see generic_file_direct_IO()).
    > Skipping dirty pages should therefore not result in an error.
    >

    This change worries me. It's a very bad situation if we leave dirty
    pagecache sitting over a piece of the file which is about to be either read
    or written via direct-IO. As far as the application is concerned, it
    pretty much guarantees impending data corruption and I do think we need to
    tell the application the bad news and not just pretend that things are all
    OK.

    What problem are we trying to fix here?


    > ---
    >
    > mm/truncate.c | 8 +++++---
    > 1 files changed, 5 insertions(+), 3 deletions(-)
    >
    > diff --git a/mm/truncate.c b/mm/truncate.c
    > index 0f4b6d1..c3ff820 100644
    > --- a/mm/truncate.c
    > +++ b/mm/truncate.c
    > @@ -318,6 +318,8 @@ EXPORT_SYMBOL(invalidate_mapping_pages);
    > * invalidation guarantees, and cannot afford to leave pages behind because
    > * shrink_list() has a temp ref on them, or because they're transiently sitting
    > * in the lru_cache_add() pagevecs.
    > + * Note: this function just skips pages that are dirty without flagging
    > + * an error.
    > */
    > static int
    > invalidate_complete_page2(struct address_space *mapping, struct page *page)
    > @@ -330,7 +332,7 @@ invalidate_complete_page2(struct address
    >
    > write_lock_irq(&mapping->tree_lock);
    > if (PageDirty(page))
    > - goto failed;
    > + goto dirty;
    >
    > BUG_ON(PagePrivate(page));
    > __remove_from_page_cache(page);
    > @@ -338,9 +340,9 @@ invalidate_complete_page2(struct address
    > ClearPageUptodate(page);
    > page_cache_release(page); /* pagecache ref */
    > return 1;
    > -failed:
    > +dirty:
    > write_unlock_irq(&mapping->tree_lock);
    > - return 0;
    > + return 1;
    > }
    >
    > static int do_launder_page(struct address_space *mapping, struct page *page)
    -
    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-02-14 23:05    [W:0.025 / U:30.780 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site