Re: [PATCH linux-2.6.1-rc1-mm1] filemap_fdatawait.patch

[Suparna Bhattacharya]

On Wed, Dec 31, 2003 at 03:42:39PM -0800, Andrew Morton wrote:
> Daniel McNeil <daniel@xxxxxxxx> wrote:
> >
> > The other potential race in filemap_fdatawait() was that it
> > removed the page from the locked list while waiting for a writeback
> > and if there was a 2nd filemap_fdatawait() running on another cpu,
> > it would not wait for the page being written since it would never see
> > it on the list.
> 
> That would only happen if one thread or the other was not running under
> i_sem.   The only path I see doing that is in generic_file_direct_IO()?

Yes, and we should simply fix generic_file_direct_IO to avoid doing so.
We anyway issue filemap_fdatawait later with i_sem held.

The race that we need to worry about is between background writeouts
(which don't take i_sem) and filemap_fdatawrite/filemap_fdatawait - i.e
the first one discussed.

> 
> > +		/*
> > +		 * If the page is locked, it might be in process of being 
> > +		 * setup for writeback but without PG_writeback set 
> > +		 * and with PG_dirty cleared.
> > +		 * (PG_dirty is cleared BEFORE PG_writeback is set)
> > +		 * So, wait for the PG_locked to clear, then start over.
> > +		 */
> > +		if (PageLocked(page)) {
> > +			page_cache_get(page);
> > +			spin_unlock(&mapping->page_lock);
> > +			wait_on_page_locked(page);
> > +			page_cache_release(page);
> > +			goto restart;
> > +		}
> 
> Why is this a problem which needs addressing here?  If some other thread is
> in the process of starting I/O against this page then the page must have
> been clean when this thread ran filemap_fdatawrite()?

This is the same race that we have been discussing (background writer
pulled this page off io_pages, put it on locked pages but hasn't set 
PG_writeback as yet). To me it seemed that Daniel's solution was just an 
alternative to what you proposed - i.e. adding lock_page() to filemap_fdatawait.
I have to think a little about the fix -- AFAICS but we are all talking
about the same (real) problem here.

Regards
Suparna

-- 
Suparna Bhattacharya (suparna@xxxxxxxxxx)
Linux Technology Center
IBM Software Lab, India

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/