Re: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Wed, 2006-12-20 at 23:15 +0100, Peter Zijlstra wrote:
> I think this is also needed:

NAK

invalidate_inode_pages2() should _not_ be pretending that dirty pages
are clean. This patch is incorrect both for the NFS usage and for the
directIO usage.

In the latter case, if someone has the page mmapped, resulting in the
page getting marked as dirty _after_ a directIO write, then it would be
wrong to discard that data. Only dirty data from _before_ the directIO
write should needs to be discarded (and that is achieved by unmapping,
then cleaning the page prior to the directIO call)...

For the NFS case, the race is a bit more tricky, since you have the
"unstable write" case which means that the page is neither marked as
dirty, nor is entirely clean ('cos we don't know that the server has
committed the data to permanent storage yet).

Cheers
  Trond

> ---
>  mm/truncate.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> Index: linux-2.6/mm/truncate.c
> ===================================================================
> --- linux-2.6.orig/mm/truncate.c
> +++ linux-2.6/mm/truncate.c
> @@ -320,19 +320,14 @@ invalidate_complete_page2(struct address
>  	if (PagePrivate(page) && !try_to_release_page(page, GFP_KERNEL))
>  		return 0;
>  
> +	cancel_dirty_page(page, PAGE_CACHE_SIZE);
>  	lock_page_ref_irq(page);
> -	if (PageDirty(page))
> -		goto failed;
> -
>  	BUG_ON(PagePrivate(page));
>  	__remove_from_page_cache(page);
>  	unlock_page_ref_irq(page);
>  	ClearPageUptodate(page);
>  	page_cache_release(page);	/* pagecache ref */
>  	return 1;
> -failed:
> -	unlock_page_ref_irq(page);
> -	return 0;
>  }
>  
>  /**
> 
> 
> -
> 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/