Re: Checking page_count(page) in invalidate_complete_page

Chuck Lever
2006-09-28T16:41:59Z

Andrew Morton wrote:
> On Thu, 28 Sep 2006 12:26:36 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
>
>> Hi Nick-
>>
>> Nick Piggin wrote:
>>>> The page references I'm noting are:
>>>>
>>>> 1. add_to_page_cache_lru 1 -> 2
>>>>
>>>> 2. do_generic_mapping_read, released in nfs_do_filldir
>>>>
>>>> 3. read_cache_page, released in nfs_readdir
>>>>
>>>> 4. pagevec_lookup (from invalidate_inode_pages2_range) 2 -> 3
>>>>
>>>> The odd thing is some of the pages for the directory are released, so
>>>> they must have a ref count of 2 *after* the pagevec_lookup. Looks
>>>> like more investigation is needed.
>>>
>>> Well they could get flushed out of the lru add pagevecs which drops
>>> their count. Just to
>>> test that theory you could run lru_add_drain_all() at the start of the
>>> invalidate function.
>>> That's not going to actually fix the problem, but it might help 2.6.18
>>> limp along when
>>> combined with some other work.
>> I've verified that the pages that fail to be invalidated are indeed
>> languishing in the per-CPU lru-add pagevec. Adding a call to
>> lru_add_drain_all() to the invalidate_inode_pages2() path does fix the
>> NFS readdir problem.
>>
>> I think a call to lru_add_drain_all() belongs in both the
>> invalidate_inode_pages() and the invalidate_inode_pages2() path. Do you
>> agree?
>
> Yes. But the page-pinned-by-vmscan scenario which you earlier identified
> can still happen, I believe.

Yes it can. From what I gather from the conversation between you and
Nick, that is a much harder problem to solve. Given the debugging I've
done, I don't think I was hitting that case, even though I still believe
it is possible in the current code.

In this case, however, it seems like the non-empty LRU pagevecs will be
a pretty common show-stopper for invalidate_inode_pages. It will hit
NFS directories and files quite a bit I would expect because the average
size of these objects is only a few pages.