From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wu Fengguang Subject: Re: [PATCH 08/10] vmscan: isolated_lru_pages() stop neighbour search if neighbour cannot be isolated Date: Wed, 8 Sep 2010 21:14:04 +0800 Message-ID: <20100908131404.GA12660@localhost> References: <1283770053-18833-1-git-send-email-mel@csn.ul.ie> <1283770053-18833-9-git-send-email-mel@csn.ul.ie> <20100908113734.GC7597@localhost> <20100908125044.GE29263@csn.ul.ie> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "linux-mm@kvack.org" , "linux-fsdevel@vger.kernel.org" , Linux Kernel List , Rik van Riel , Johannes Weiner , Minchan Kim , Andrea Arcangeli , KAMEZAWA Hiroyuki , KOSAKI Motohiro , Dave Chinner , Chris Mason , Christoph Hellwig , Andrew Morton To: Mel Gorman Return-path: Received: from mga11.intel.com ([192.55.52.93]:6213 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752741Ab0IHNOb (ORCPT ); Wed, 8 Sep 2010 09:14:31 -0400 Content-Disposition: inline In-Reply-To: <20100908125044.GE29263@csn.ul.ie> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Sep 08, 2010 at 08:50:44PM +0800, Mel Gorman wrote: > On Wed, Sep 08, 2010 at 07:37:34PM +0800, Wu Fengguang wrote: > > On Mon, Sep 06, 2010 at 06:47:31PM +0800, Mel Gorman wrote: > > > From: KOSAKI Motohiro > > > > > > isolate_lru_pages() does not just isolate LRU tail pages, but also isolate > > > neighbour pages of the eviction page. The neighbour search does not stop even > > > if neighbours cannot be isolated which is excessive as the lumpy reclaim will > > > no longer result in a successful higher order allocation. This patch stops > > > the PFN neighbour pages if an isolation fails and moves on to the next block. > > > > > > Signed-off-by: KOSAKI Motohiro > > > Signed-off-by: Mel Gorman > > > --- > > > mm/vmscan.c | 24 ++++++++++++++++-------- > > > 1 files changed, 16 insertions(+), 8 deletions(-) > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > index 64f9ca5..ff52b46 100644 > > > --- a/mm/vmscan.c > > > +++ b/mm/vmscan.c > > > @@ -1047,14 +1047,18 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > > > continue; > > > > > > /* Avoid holes within the zone. */ > > > - if (unlikely(!pfn_valid_within(pfn))) > > > + if (unlikely(!pfn_valid_within(pfn))) { > > > + nr_lumpy_failed++; > > > break; > > > + } > > > > > > cursor_page = pfn_to_page(pfn); > > > > > > /* Check that we have not crossed a zone boundary. */ > > > - if (unlikely(page_zone_id(cursor_page) != zone_id)) > > > - continue; > > > + if (unlikely(page_zone_id(cursor_page) != zone_id)) { > > > + nr_lumpy_failed++; > > > + break; > > > + } > > > > > > /* > > > * If we don't have enough swap space, reclaiming of > > > @@ -1062,8 +1066,10 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > > > * pointless. > > > */ > > > if (nr_swap_pages <= 0 && PageAnon(cursor_page) && > > > - !PageSwapCache(cursor_page)) > > > - continue; > > > + !PageSwapCache(cursor_page)) { > > > + nr_lumpy_failed++; > > > + break; > > > + } > > > > > > if (__isolate_lru_page(cursor_page, mode, file) == 0) { > > > list_move(&cursor_page->lru, dst); > > > @@ -1074,9 +1080,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > > > nr_lumpy_dirty++; > > > scan++; > > > } else { > > > - if (mode == ISOLATE_BOTH && > > > - page_count(cursor_page)) > > > - nr_lumpy_failed++; > > > + /* the page is freed already. */ > > > + if (!page_count(cursor_page)) > > > + continue; > > > + nr_lumpy_failed++; > > > + break; > > > } > > > } > > > > The many nr_lumpy_failed++ can be moved here: > > > > if (pfn < end_pfn) > > nr_lumpy_failed++; > > > > Because the break stops the loop iterating, is there an advantage to > making it a pfn check instead? I might be misunderstanding your > suggestion. The complete view in my mind is for (; pfn < end_pfn; pfn++) { if (failed 1) break; if (failed 2) break; if (failed 3) break; } if (pfn < end_pfn) nr_lumpy_failed++; Sure it just reduces several lines of code :) Thanks, Fengguang