Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754710AbZKMOid (ORCPT ); Fri, 13 Nov 2009 09:38:33 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753375AbZKMOic (ORCPT ); Fri, 13 Nov 2009 09:38:32 -0500 Received: from mail-pw0-f42.google.com ([209.85.160.42]:43406 "EHLO mail-pw0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752312AbZKMOib convert rfc822-to-8bit (ORCPT ); Fri, 13 Nov 2009 09:38:31 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=B9kPYk2DIYAp6ra95NF5E+y4vh1HY+XuVHeXw+eJGTefXp3Ok78WRCXAmPufV2QBKD g7GFN7K5qDexBQb/SuqNmVdDzxFkdrE5Pt1qF12+pKsN0zsT3j0M1TeVfPCn+Mp5Se0E jxmsruF9JJGhNvTK1Qb7mm18gu5I4OtRs4uvU= MIME-Version: 1.0 In-Reply-To: <20091113142608.33B9.A69D9226@jp.fujitsu.com> References: <1258054235-3208-1-git-send-email-mel@csn.ul.ie> <1258054235-3208-6-git-send-email-mel@csn.ul.ie> <20091113142608.33B9.A69D9226@jp.fujitsu.com> Date: Fri, 13 Nov 2009 23:38:37 +0900 Message-ID: <28c262360911130638l7c0becbbsd09db0fd3837ffa5@mail.gmail.com> Subject: Re: [PATCH 5/5] vmscan: Take order into consideration when deciding if kswapd is in trouble From: Minchan Kim To: KOSAKI Motohiro Cc: Mel Gorman , Andrew Morton , Frans Pop , Jiri Kosina , Sven Geggus , Karol Lewandowski , Tobias Oetiker , linux-kernel@vger.kernel.org, "linux-mm@kvack.org\"" , Pekka Enberg , Rik van Riel , Christoph Lameter , Stephan von Krawczynski , "Rafael J. Wysocki" , Kernel Testers List Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6563 Lines: 170 Hi, Koskai. I missed this patch. I noticed this after Mel reply your patch. On Fri, Nov 13, 2009 at 6:54 PM, KOSAKI Motohiro wrote: >> If reclaim fails to make sufficient progress, the priority is raised. >> Once the priority is higher, kswapd starts waiting on congestion. >> However, on systems with large numbers of high-order atomics due to >> crappy network cards, it's important that kswapd keep working in >> parallel to save their sorry ass. >> >> This patch takes into account the order kswapd is reclaiming at before >> waiting on congestion. The higher the order, the longer it is before >> kswapd considers itself to be in trouble. The impact is that kswapd >> works harder in parallel rather than depending on direct reclaimers or >> atomic allocations to fail. >> >> Signed-off-by: Mel Gorman >> --- >> ?mm/vmscan.c | ? 14 ++++++++++++-- >> ?1 files changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index ffa1766..5e200f1 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1946,7 +1946,7 @@ static int sleeping_prematurely(int order, long remaining) >> ?static unsigned long balance_pgdat(pg_data_t *pgdat, int order) >> ?{ >> ? ? ? int all_zones_ok; >> - ? ? int priority; >> + ? ? int priority, congestion_priority; >> ? ? ? int i; >> ? ? ? unsigned long total_scanned; >> ? ? ? struct reclaim_state *reclaim_state = current->reclaim_state; >> @@ -1967,6 +1967,16 @@ static unsigned long balance_pgdat(pg_data_t *pgdat, int order) >> ? ? ? ?*/ >> ? ? ? int temp_priority[MAX_NR_ZONES]; >> >> + ? ? /* >> + ? ? ?* When priority reaches congestion_priority, kswapd will sleep >> + ? ? ?* for a short time while congestion clears. The higher the >> + ? ? ?* order being reclaimed, the less likely kswapd will go to >> + ? ? ?* sleep as high-order allocations are harder to reclaim and >> + ? ? ?* stall direct reclaimers longer >> + ? ? ?*/ >> + ? ? congestion_priority = DEF_PRIORITY - 2; >> + ? ? congestion_priority -= min(congestion_priority, sc.order); > > This calculation mean > > ? ? ? ?sc.order ? ? ? ?congestion_priority ? ? scan-pages > ? ? ? ?--------------------------------------------------------- > ? ? ? ?0 ? ? ? ? ? ? ? 10 ? ? ? ? ? ? ? ? ? ? ?1/1024 * zone-mem > ? ? ? ?1 ? ? ? ? ? ? ? 9 ? ? ? ? ? ? ? ? ? ? ? 1/512 ?* zone-mem > ? ? ? ?2 ? ? ? ? ? ? ? 8 ? ? ? ? ? ? ? ? ? ? ? 1/256 ?* zone-mem > ? ? ? ?3 ? ? ? ? ? ? ? 7 ? ? ? ? ? ? ? ? ? ? ? 1/128 ?* zone-mem > ? ? ? ?4 ? ? ? ? ? ? ? 6 ? ? ? ? ? ? ? ? ? ? ? 1/64 ? * zone-mem > ? ? ? ?5 ? ? ? ? ? ? ? 5 ? ? ? ? ? ? ? ? ? ? ? 1/32 ? * zone-mem > ? ? ? ?6 ? ? ? ? ? ? ? 4 ? ? ? ? ? ? ? ? ? ? ? 1/16 ? * zone-mem > ? ? ? ?7 ? ? ? ? ? ? ? 3 ? ? ? ? ? ? ? ? ? ? ? 1/8 ? ?* zone-mem > ? ? ? ?8 ? ? ? ? ? ? ? 2 ? ? ? ? ? ? ? ? ? ? ? 1/4 ? ?* zone-mem > ? ? ? ?9 ? ? ? ? ? ? ? 1 ? ? ? ? ? ? ? ? ? ? ? 1/2 ? ?* zone-mem > ? ? ? ?10 ? ? ? ? ? ? ?0 ? ? ? ? ? ? ? ? ? ? ? 1 ? ? ?* zone-mem > ? ? ? ?11+ ? ? ? ? ? ? 0 ? ? ? ? ? ? ? ? ? ? ? 1 ? ? ?* zone-mem > > I feel this is too agressive. The intention of this congestion_wait() > is to prevent kswapd use 100% cpu time. but the above promotion seems > break it. I can't understand your point. Mel didn't change the number of scan pages. It denpends on priority. He just added another one to prevent frequent contestion_wait. Still, shrink_zone is called with priority, not congestion_priority. > example, > ia64 have 256MB hugepage (i.e. order=14). it mean kswapd never sleep. Indeed. Good catch. > example2, > order-3 (i.e. PAGE_ALLOC_COSTLY_ORDER) makes one of most inefficent > reclaim, because it doesn't use lumpy recliam. > I've seen 128GB size zone, it mean 1/128 = 1GB. oh well, kswapd definitely > waste cpu time 100%. Above I said, It depends on priority, not congestion_priority. > >> + >> ?loop_again: >> ? ? ? total_scanned = 0; >> ? ? ? sc.nr_reclaimed = 0; >> @@ -2092,7 +2102,7 @@ loop_again: >> ? ? ? ? ? ? ? ?* OK, kswapd is getting into trouble. ?Take a nap, then take >> ? ? ? ? ? ? ? ?* another pass across the zones. >> ? ? ? ? ? ? ? ?*/ >> - ? ? ? ? ? ? if (total_scanned && priority < DEF_PRIORITY - 2) >> + ? ? ? ? ? ? if (total_scanned && priority < congestion_priority) >> ? ? ? ? ? ? ? ? ? ? ? congestion_wait(BLK_RW_ASYNC, HZ/10); > > Instead, How about this? > > > > --- > ?mm/vmscan.c | ? 13 ++++++++++++- > ?1 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 64e4388..937e90d 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1938,6 +1938,7 @@ static unsigned long balance_pgdat(pg_data_t *pgdat, int order) > ? ? ? ? * free_pages == high_wmark_pages(zone). > ? ? ? ? */ > ? ? ? ?int temp_priority[MAX_NR_ZONES]; > + ? ? ? int has_under_min_watermark_zone = 0; Let's make the shorter. How about "under_min_watermark"? > > ?loop_again: > ? ? ? ?total_scanned = 0; > @@ -2057,6 +2058,15 @@ loop_again: > ? ? ? ? ? ? ? ? ? ? ? ?if (total_scanned > SWAP_CLUSTER_MAX * 2 && > ? ? ? ? ? ? ? ? ? ? ? ? ? ?total_scanned > sc.nr_reclaimed + sc.nr_reclaimed / 2) > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sc.may_writepage = 1; > + > + ? ? ? ? ? ? ? ? ? ? ? /* > + ? ? ? ? ? ? ? ? ? ? ? ?* We are still under min water mark. it mean we have > + ? ? ? ? ? ? ? ? ? ? ? ?* GFP_ATOMIC allocation failure risk. Hurry up! > + ? ? ? ? ? ? ? ? ? ? ? ?*/ > + ? ? ? ? ? ? ? ? ? ? ? if (!zone_watermark_ok(zone, order, min_wmark_pages(zone), > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? end_zone, 0)) > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? has_under_min_watermark_zone = 1; > + > ? ? ? ? ? ? ? ?} > ? ? ? ? ? ? ? ?if (all_zones_ok) > ? ? ? ? ? ? ? ? ? ? ? ?break; ? ? ? ? ?/* kswapd: all done */ > @@ -2064,7 +2074,8 @@ loop_again: > ? ? ? ? ? ? ? ? * OK, kswapd is getting into trouble. ?Take a nap, then take > ? ? ? ? ? ? ? ? * another pass across the zones. > ? ? ? ? ? ? ? ? */ > - ? ? ? ? ? ? ? if (total_scanned && priority < DEF_PRIORITY - 2) > + ? ? ? ? ? ? ? if (total_scanned && (priority < DEF_PRIORITY - 2) && > + ? ? ? ? ? ? ? ? ? !has_under_min_watermark_zone) > ? ? ? ? ? ? ? ? ? ? ? ?congestion_wait(BLK_RW_ASYNC, HZ/10); > > ? ? ? ? ? ? ? ?/* > -- > 1.6.2.5 Anyway, Looks good to me. It's more straightforward than Mel's one, I think. Reviewed-by: Minchan Kim -- Kind regards, Minchan Kim -- 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/