On Mon, 2008-08-11 at 15:25 +0900, Hisashi Hifumi wrote:

quoted

quoted

quoted

quoted

quoted

I am wondering why we need stronger invalidate hurantees for DIO->
invalidate_inode_pages_range(),which force the page being removed from
page cache? In case of bh is busy due to ext3 writeout,
journal_try_to_free_buffers() could return different error number(EBUSY)
to try_to_releasepage() (instead of EIO).  In that case,  could we just
leave the page in the cache, clean pageuptodate() (to force later buffer
read to read from disk) and then invalidate_complete_page2() return
successfully? Any issue with this way?

My idea is that journal_try_to_free_buffers returns EBUSY if it fails due to
bh busy, and dio write falls back to buffered write. This is easy to fix.

What about the invalidates done after the DIO has already run
non-buffered?

Dio write falls back to buffered IO when writing to a hole on ext3, I 

think. I want to 

quoted

apply this mechanism to fix this issue. When try_to_release_page fails on 

a page 

quoted

due to bh busy, dio write does buffered write, sync_page_range, and 
wait_on_page_writeback, imvalidates page cache to preserve dio semantics. 
Even if page invalidation that is carried out after 

wait_on_page_writeback fails, 

quoted

there is no inconsistency between HDD and page cache.

Sorry, I'm sure I wasn't very clear, I was referencing this code from
mm/filemap.c:

       written = mapping->a_ops->direct_IO(WRITE, iocb, iov, pos, *nr_segs);

       /*
        * Finally, try again to invalidate clean pages which might have been
        * cached by non-direct readahead, or faulted in by get_user_pages()
        * if the source of the write was an mmap'ed region of the file
        * we're writing.  Either one is a pretty crazy thing to do,
        * so we don't support it 100%.  If this invalidation
        * fails, tough, the write still worked...
        */
       if (mapping->nrpages) {
               invalidate_inode_pages2_range(mapping,
                                             pos >> PAGE_CACHE_SHIFT, end);
       }

If this second invalidate fails during a DIO write, we'll have up to
date pages in cache that don't match the data on disk.  It is unlikely
to fail because the conditions that make jbd unable to free a buffer are
rare, but it can still happen with the write combination of mmap usage.

The good news is the second invalidate doesn't make O_DIRECT return
-EIO.  But, it sounds like fixing do_launder_page to always call into
the FS can fix all of these problems.  Am I missing something?

My approach is not implementing do_launder_page for ext3.
It is needed to modify VFS.

My patch is as follows:

Sorry, I'm still not sure why the do_launder_page implementation is a
bad idea.  Clearly Mingming spent quite some time on it in the past, but
given that it could provide a hook for the FS to do expensive operations
to make the page really go away, why not do it?

As far as I can tell, the only current users afs, nfs and fuse.  Pushing
down the PageDirty check to those filesystems should be trivial.

With that said, I don't have strong feelings against falling back to
buffered IO when the invalidate fails.  Maybe Zach remembers something I
don't?

-chris