lkml.org 
[lkml]   [2002]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: MM patches against 2.5.31
On Tue, Aug 27, 2002 at 06:48:50PM +0200, Daniel Phillips wrote:
> On Monday 26 August 2002 22:58, Christian Ehrhardt wrote:
> > > Nope, still don't see it. Whoever hits put_page_testzero frees the page,
> > > secure in the knowlege that there are no other references to it.
> >
> > Well yes, but we cannot remove the page from the lru atomatically
> > at page_cache_release time if we follow your proposal. If you think we can,
> > show me your implementation of page_cache_release and I'll show
> > you where the races are (unless you do everything under the lru_lock
> > of course).
>
> void page_cache_release(struct page *page)
> {
> spin_lock(&pagemap_lru_lock);
> if (PageLRU(page) && page_count(page) == 2) {
> __lru_cache_del(page);
> atomic_dec(&page->count);
> }
> spin_unlock(&pagemap_lru_lock);
> if (put_page_testzero(page))
> __free_pages_ok(page, 0);
> }
>
> This allows the following benign race, with initial page count = 3:
> [ ...]
> Neither holder of a page reference sees the count at 2, and so the page
> is left on the lru with count = 1. This won't happen often and such
> pages will be recovered from the cold end of the list in due course.

Ok, agreed. I think this will work but taking the lru lock each time
is probably not a good idea.

> We could also do this:
>
> void page_cache_release(struct page *page)
> {
> if (page_count(page) == 2) {
> spin_lock(&pagemap_lru_lock);
> if (PageLRU(page) && page_count(page) == 2) {
> __lru_cache_del(page);
> atomic_dec(&page->count);
> }
> spin_unlock(&pagemap_lru_lock);
> }
> if (put_page_testzero(page))
> __free_pages_ok(page, 0);
> }
>
> Which avoids taking the lru lock sometimes in exchange for widening the
> hole through which pages can end up with count = 1 on the lru list.

This sounds like something that is worth trying. I missed that one.


Side note: The BUG in __pagevec_lru_del seems strange. refill_inactive
or shrink_cache could have removed the page from the lru before
__pagevec_lru_del acquired the lru lock.

regards Christian

--
THAT'S ALL FOLKS!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:28    [W:0.091 / U:1.564 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site