From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Fri, 27 Aug 2004 17:16:41 -0300 From: Marcelo Tosatti Subject: Re: refill_inactive_zone question Message-ID: <20040827201641.GD3332@logos.cnet> References: <20040827190714.GB3332@logos.cnet> 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: Hugh Dickins Cc: linux-mm@kvack.org, akpm@osdl.org List-ID: On Fri, Aug 27, 2004 at 10:17:35PM +0100, Hugh Dickins wrote: > On Fri, 27 Aug 2004, Marcelo Tosatti wrote: > > > > Is it possible to have AnonPages without a mapping to them? I dont think so. > > It was impossible, but my "remove page_map_lock" patches had to change that. > > > Can't the check "if (total_swap_pages == 0 && PageAnon(page))" be moved > > inside "if (page_mapped(page))" ? > > Yes: it's like that in -mm, and I believe now in Linus' bk tree too. > > Hugh Hi Hugh, Oh thanks! I see that. So you just dropped the bit spinlocked and changed mapcount to an atomic variable, right? Cool. Do you have any numbers on big SMP systems for that change? Talking about refill_inactive_zone(), the next stage of the algorithm: while (!list_empty(&l_active)) { page = lru_to_page(&l_active); prefetchw_prev_lru_page(page, &l_active, flags); if (TestSetPageLRU(page)) BUG(); BUG_ON(!PageActive(page)); list_move(&page->lru, &zone->active_list); pgmoved++; if (!pagevec_add(&pvec, page)) { zone->nr_active += pgmoved; pgmoved = 0; spin_unlock_irq(&zone->lru_lock); __pagevec_release(&pvec); spin_lock_irq(&zone->lru_lock); } } Several things: 1) __pagevec_release does lru_add_drain(), which moves pages in the deferred lru queues (active & inactive) to the actual lists. But at that point in refill_inactive() thats not a direct benefit (we already moved scanned the inactive list at the beginning of the algo). So, we could remove that lru_add_drain from refill_inactive_zone->__pagevec_release. The bad part of doing unecessary lru_add_drain's is that we minimize the chance from the queue growing big. And the queues growing big means we batch the list moving, better cache locality. Is there any good reason for doing that lru_add_drain from the refill_inactive_zone() callchain? 2) before calling __pagevec_release we drop the zone lock, to lock it again at __pagevec_release->release_pages. Acquiring locks is usually more expensive then it seems (thanks Paul McKenney!), release_pages handles pagevec's containing pages from different zones, but we know all pages on this pagevec are on the same zone. Couldnt it all be under the zone lock? 3) What happens if that __pagevec_release frees one or more pages page (and deletes it/them from the LRU list)? We will still count those pages in "pgmoved" which will then be accounted in zone->nr_active. Whoops. Hope I'm full of shit. -- 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: aart@kvack.org