Re: [PATCH] Make invalidate_inode_pages2() work again

Andrew Morton
2006-09-26T01:42:13Z

On Mon, 25 Sep 2006 21:20:13 -0400
Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> On Tue, 2006-09-26 at 09:59 +1000, Nick Piggin wrote:
> > Chuck Lever wrote:
> >
> > >A recent change to fix a problem with invalidate_inode_pages() has weakened
> > >the behavior of invalidate_inode_pages2() inadvertently. Add a flag to
> > >tell the helper routines when stronger invalidation semantics are desired.
> > >
> >
> > Question: if invalidate_inode_pages2 cares about not invalidating dirty
> > pages, how can one avoid the page_count check and it still be correct
> > (ie. not randomly lose dirty bits in some situations)?
>
> Tests of page_count _suck_ 'cos they are 100% non-specific. Is there no
> way to set a page flag or something to indicate that the page may have
> been remapped while we were sleeping?

Its a question of "what are these functions supposed to do"?

I'd suggest:

invalidate_inode_pages() -> best-effort, remove-it-if-it-isn't-busy

truncate_inode_pages() -> guaranteed, data-destroying takedown.

invalidate_inode_pages2() -> Somewhere in between. Any takers?

I'd suggest "guaranteed, non-data-destroying takedown". Maybe. So it
doesn't remove dirty pages, but it does remove otherwise-busy pages.

As definitions go, that really sucks.

I think testing page_count() makes sense for invalidate_inode_pages(),
because that page is clearly in use by someone for something and we
shouldn't go and whip it out of pagecache under that someone's feet. It
is, after all, "pinned".

For invalidate_inode_pages2(), proper behaviour would be to block until
whoever is busying that page stops being busy on it.

I perhaps we could do a wake_up(page_waitqueue(page)) in vmscan when it
drops the ref on a page. But that would mean that
invalidate_inode_pages2() would get permanently stuck on a
permanently-pinned page.

It's a bit of a pickle. Perhaps we just add the
invalidate_complete_page2(). That re-adds the direct-io race, which is
fixable by locking the page in the pagefault handler. argh.