On Wed, 2008-08-06 at 15:57 -0700, Mingming Cao wrote:

quoted

quoted

Does it make more sense to fix do_launder_page to call into the FS on
every page, and let the FS check for PageDirty on its own?  That way
invalidate_inode_pages2_range basically gets its own private call into
the FS that says wait around until this page is really free.

  That would certainly work as well. But IMHO waiting for ->writepage()
call to finish isn't really a big deal even in try_to_release_page() if
__GFP_FS (and __GFP_WAIT) is set. The only problem is that there is no
effective way to do so and so Hisashi used that "wait for b_count to drop"
which looks really scary and I don't like it as well.

I was  looking at the comment in invalidate_complete_page2(), which is
now only called from DIO path, it saids

/*
 * This is like invalidate_complete_page(), except it ignores the page's
 * refcount.  We do this because invalidate_inode_pages2() needs
stronger
 * invalidation guarantees, and cannot afford to leave pages behind
because
 * shrink_page_list() has a temp ref on them, or because they're
transiently
 * sitting in the lru_cache_add() pagevecs.
 */


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?

Isn't this similar to the recent splice thread?  It seems like there are
places in the kernel that expect an up to date page they have a
reference against to stay up to date.

-chris