Re: [PATCH linux-2.6.0-test10-mm1] filemap_fdatawait.patch

[Suparna Bhattacharya <suparna@in.ibm.com>]

On Wed, Dec 31, 2003 at 03:17:36AM -0800, Andrew Morton wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> >
> > Let me actually think about this a bit.
> 
> Nasty.  The same race is present in 2.4.x...
> 
> How's about we start new I/O in filemap_fdatawait() if the page is dirty?
> 

Makes sense to me.
There's a chance that this could explain why Daniel saw exposures even 
with his fix. 

Would be interesting to see his results with your patch.

Though we might as well plug this anyway ?

> 
> diff -puN mm/filemap.c~a mm/filemap.c
> --- 25/mm/filemap.c~a	2003-12-31 03:10:29.000000000 -0800
> +++ 25-akpm/mm/filemap.c	2003-12-31 03:17:05.000000000 -0800
> @@ -206,7 +206,13 @@ restart:
>  		page_cache_get(page);
>  		spin_unlock(&mapping->page_lock);
>  
> -		wait_on_page_writeback(page);
> +		lock_page(page);
> +		if (PageDirty(page) && mapping->a_ops->writepage) {
> +			write_one_page(page, 1);
> +		} else {
> +			wait_on_page_writeback(page);
> +			unlock_page(page);

Would we lose anything if we unlock_page() before wait_on_page_writeback() ?
I was thinking about the corresponding fix in sync_page_range, and it
would make life easier for retry based fs-AIO if we could move the
unlock_page before the wait.

> +		}
>  		if (PageError(page))
>  			ret = -EIO;
>  
> 
> 

Regards
Suparna

-- 
Suparna Bhattacharya (suparna@in.ibm.com)
Linux Technology Center
IBM Software Lab, India