Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755743Ab0K3LVA (ORCPT ); Tue, 30 Nov 2010 06:21:00 -0500 Received: from zene.cmpxchg.org ([85.214.230.12]:50226 "EHLO zene.cmpxchg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755588Ab0K3LU7 (ORCPT ); Tue, 30 Nov 2010 06:20:59 -0500 Date: Tue, 30 Nov 2010 12:20:41 +0100 From: Johannes Weiner To: Minchan Kim Cc: Mel Gorman , Andrew Morton , linux-mm , LKML , Ben Gamari , Wu Fengguang , KOSAKI Motohiro , Nick Piggin Subject: Re: [PATCH 2/3] Reclaim invalidated page ASAP Message-ID: <20101130112041.GC15564@cmpxchg.org> References: <053e6a3308160a8992af5a47fb4163796d033b08.1291043274.git.minchan.kim@gmail.com> <20101129165706.GH13268@csn.ul.ie> <20101129224130.GA1989@barrios-desktop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101129224130.GA1989@barrios-desktop> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3035 Lines: 95 On Tue, Nov 30, 2010 at 07:41:30AM +0900, Minchan Kim wrote: > diff --git a/mm/swap.c b/mm/swap.c > index 19e0812..1f1f435 100644 > --- a/mm/swap.c > +++ b/mm/swap.c > @@ -275,28 +275,51 @@ void add_page_to_unevictable_list(struct page *page) > * into inative list's head. Because the VM expects the page would > * be writeout by flusher. The flusher's writeout is much effective > * than reclaimer's random writeout. > + * > + * If the page isn't page_mapped and dirty/writeback, the page > + * could reclaim asap using PG_reclaim. > + * > + * 1. active, mapped page -> none > + * 2. active, dirty/writeback page -> inactive, head, PG_reclaim > + * 3. inactive, mapped page -> none > + * 4. inactive, dirty/writeback page -> inactive, head, PG_reclaim > + * 5. Others -> none > + * > + * In 4, why it moves inactive's head, the VM expects the page would > + * be writeout by flusher. The flusher's writeout is much effective than > + * reclaimer's random writeout. > */ > static void __lru_deactivate(struct page *page, struct zone *zone) > { > int lru, file; > - unsigned long vm_flags; > + int active = 0; vm_flags is never used in this series. > - if (!PageLRU(page) || !PageActive(page)) > + if (!PageLRU(page)) > return; > - > /* Some processes are using the page */ > if (page_mapped(page)) > return; > - > - file = page_is_file_cache(page); > - lru = page_lru_base_type(page); > - del_page_from_lru_list(zone, page, lru + LRU_ACTIVE); > - ClearPageActive(page); > - ClearPageReferenced(page); > - add_page_to_lru_list(zone, page, lru); > - __count_vm_event(PGDEACTIVATE); > - > - update_page_reclaim_stat(zone, page, file, 0); > + if (PageActive(page)) > + active = 1; active = PageActive(page) > + if (PageWriteback(page) || PageDirty(page)) { > + /* > + * PG_reclaim could be raced with end_page_writeback > + * It can make readahead confusing. But race window > + * is _really_ small and it's non-critical problem. > + */ > + SetPageReclaim(page); > + > + file = page_is_file_cache(page); > + lru = page_lru_base_type(page); > + del_page_from_lru_list(zone, page, lru + active); > + ClearPageActive(page); > + ClearPageReferenced(page); > + add_page_to_lru_list(zone, page, lru); > + if (active) > + __count_vm_event(PGDEACTIVATE); > + update_page_reclaim_stat(zone, page, file, 0); > + } If we lose the race with writeback, the completion handler won't see PG_reclaim, won't move the page, and we have an unwanted clean cache page on the active list. Given the pagevec caching of those pages it could be rather likely that IO completes before the above executes. Shouldn't this be if (PageWriteback() || PageDirty()) { SetPageReclaim() move_to_inactive_head() } else { move_to_inactive_tail() } instead? Hannes -- 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/