Messages in this thread | | | Date | Wed, 14 Feb 2007 14:00:50 -0800 | From | Andrew Morton <> | Subject | Re: [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/
| |