Re: [patch 5/4] mm: document is_page_cache_freeable()

[Johannes Weiner <hannes@cmpxchg.org>]

On Wed, Jul 22, 2009 at 02:55:12PM -0700, Li, Ming Chun wrote:
> On Wed, 22 Jul 2009, Christoph Lameter wrote:
> 
> > 
> > >  static inline int is_page_cache_freeable(struct page *page)
> > >  {
> > > +	/*
> > > +	 * A freeable page cache page is referenced only by the caller
> > > +	 * that isolated the page, the page cache itself and
> > 
> > The page cache "itself"? This is the radix tree reference right?
> > 
> 
> I think you are right. I had trouble understanding this function, So I 
> looked into it and found out the call path:
> 
>  add_to_page_cache_locked 
>    -> page_cache_get
>     -> atomic_inc(&page->_count) 
> 
> Please correct me if I am wrong.

This is correct.  But this is the purpose of reference counters - you
increase it when you reference the object so that it doesn't get freed
under you.

That's why everybody holding a reference to the page must have its
usage counter increased.  And this includes the page/swap cache, the
LRU lists, the page tables etc.

And I think in that context my comment should be obvious.  Do you need
to know that the page cache is actually managed with radix trees at
this point?

You need to know that the page cache is something holding a reference
to the page so you can meet the requirements that are written above
remove_from_page_cache() - which you are about to call.

I added the comment to document that magic `compare with 2' in there.
If more is needed, I am glad to help - but right now I don't really
think I know what the issue is with this patch?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>