2004-01-11 23:15:25

by Janet Morgan

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

Andrew Morton wrote:

>Andrew Morton <[email protected]> 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?
>
>
>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);
>+ }
> if (PageError(page))
> ret = -EIO;
>
>
>
>
That fixed the problem! Stephen's testcase is running successfully on
2.6.1-mm1 plus your patch -- no more uninitialized data!

Thanks!
-Janet
P.S. Sorry so late testing this (was on vacation).

2004-01-11 23:44:13

by Andrew Morton

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

Janet Morgan <[email protected]> wrote:
>
> >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);
> >+ }
> > if (PageError(page))
> > ret = -EIO;
> >
> >
> >
> >
> That fixed the problem! Stephen's testcase is running successfully on
> 2.6.1-mm1 plus your patch -- no more uninitialized data!

Could you please test 2.6.1-mm2 with that patch? If that works, send the
patch back to me? (I lost it ;))

It still leaves the AIO situation open.

2004-01-12 18:00:20

by Daniel McNeil

Subject: Re: filemap_fdatawait.patch

I've started test with this patch on 2.6.1-mm2 this morning.
I've seen previous runs take 24 hours to see corruption,
so I let you tomorrow how things went.

Daniel

On Sun, 2004-01-11 at 15:44, Andrew Morton wrote:
> Janet Morgan <[email protected]> wrote:
> >
> > >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);
> > >+ }
> > > if (PageError(page))
> > > ret = -EIO;
> > >
> > >
> > >
> > >
> > That fixed the problem! Stephen's testcase is running successfully on
> > 2.6.1-mm1 plus your patch -- no more uninitialized data!
>
> Could you please test 2.6.1-mm2 with that patch? If that works, send the
> patch back to me? (I lost it ;))
>
> It still leaves the AIO situation open.