Re: invalidate_inode_pages in 2.5.32/3

[Trond Myklebust <trond.myklebust@fys.uio.no>]

>>>>> " " == Andrew Morton <akpm@zip.com.au> writes:

    >> 'rpciod' process by means of a callback. Since 'rpciod' is
    >> responsible for completing all asynchronous I/O, then
    >> truncate_inode_pages() would deadlock.

     > But if there are such pages, invalidate_inode_pages() would not
     > have removed them anyway?

It should not have to. If the page is locked because it is being paged
in, then we're guaranteed that the data is fresh from the server. If
it is locked because we are writing, then ditto.

     > Trond, there are very good reasons why it broke.  Those pages
     > are visible to the whole world via global data structures -
     > both the page LRUs and via the superblocks->inodes walk.  Those
     > things exist for legitimate purposes, and it is legitimate for
     > async threads of control to take a reference on those pages
     > while playing with them.

I don't buy that explanation w.r.t. readdir: those pages should always
be clean. The global data structures might walk them in order to throw
them out, but that should be a bonus here.
In any case it seems a bit far fetched that they should be pinning
*all* the readdir pages...

     > I suspect we can just remove the page_count() test from
     > invalidate and that will fix everything up.  That will give
     > stronger invalidate and anything which doesn't like that is
     > probably buggy wrt truncate anyway.

I never liked the page_count() test, but Linus overruled me because of
the concern for consistency w.r.t. shared mmap(). Is the latter case
resolved now?

I notice for instance that you've added mapping->releasepage().
What should we be doing for that?

Cheers,
  Trond