From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <20020828131445.25959.qmail@thales.mathematik.uni-ulm.de> From: "Christian Ehrhardt" Date: Wed, 28 Aug 2002 15:14:45 +0200 Subject: Re: MM patches against 2.5.31 References: <3D644C70.6D100EA5@zip.com.au> <20020826205858.6612.qmail@thales.mathematik.uni-ulm.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org Return-Path: To: Daniel Phillips Cc: Andrew Morton , lkml , "linux-mm@kvack.org" List-ID: 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, 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/