On Fri, Aug 20, 2010 at 07:22:16PM +0900, Minchan Kim wrote:
> On Fri, Aug 20, 2010 at 6:35 PM, Mel Gorman <mel@xxxxxxxxx> wrote:
> > On Fri, Aug 20, 2010 at 01:34:47PM +0800, Wu Fengguang wrote:
> >> You do run lots of tasks: kernel_stack=1880kB.
> >>
> >> And you have lots of free memory, page reclaim has never run, so
> >> inactive_anon=0. This is where compaction is different from vmscan.
> >> In vmscan, inactive_anon is reasonably large, and will only be
> >> compared directly with isolated_anon.
> >>
> >
> > True, the key observation here was that compaction is being run via the
> > proc trigger. Normally it would be run as part of the direct reclaim
> > path when kswapd would already be awake. too_many_isolated() needs to be
> > different for compaction to take the whole system into account. What
> > would be the best alternative? Here is one possibility. A reasonable
> > alternative would be that when inactive < active that isolated can't be
> > more than num_online_cpus() * 2 (i.e. one compactor per online cpu).
> >
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 94cce51..1e000b7 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -215,14 +215,16 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
> > static bool too_many_isolated(struct zone *zone)
> > {
> >
> > - unsigned long inactive, isolated;
> > + unsigned long active, inactive, isolated;
> >
> > + active = zone_page_state(zone, NR_ACTIVE_FILE) +
> > + zone_page_state(zone, NR_INACTIVE_ANON);
> > inactive = zone_page_state(zone, NR_INACTIVE_FILE) +
> > zone_page_state(zone, NR_INACTIVE_ANON);
> > isolated = zone_page_state(zone, NR_ISOLATED_FILE) +
> > zone_page_state(zone, NR_ISOLATED_ANON);
> >
> > - return isolated > inactive;
> > + return (inactive > active) ? isolated > inactive : false;
> > }
> >
> > /*
> >
>
> 1. active : 1000 inactive : 1000
> 2. parallel reclaiming -> active : 1000 inactive : 500 isolated : 500
> 3. too_many_isolated return false.
>
> But in this case, there are already many isolated pages. So it should
> return true.
>
> How about this?
> too_many_isolated()
> {
> return (isolated > nr_zones * nr_nodes * nr_online_cpu *
> SWAP_CLUSTER_MAX);
> }
Above utterly not good.
How about this?
>From 560e8898295c663f02aede07b3d55880eba16c69 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan.kim@xxxxxxxxx>
Date: Mon, 23 Aug 2010 00:20:44 +0900
Subject: [PATCH] compaction: handle active and inactive fairly in too_many_isolated
Iram reported compaction's too_many_isolated loops forever.
(http://www.spinics.net/lists/linux-mm/msg08123.html)
The meminfo of situation happened was inactive anon is zero.
That's because the system has no memory pressure until then.
While all anon pages was in active lru, compaction could select
active lru as well as inactive lru. That's different things
with vmscan's isolated. So we has been two too_many_isolated.
While compaction can isolated pages in both active and inactive,
current implementation of too_many_isolated only considers inactive.
It made Iram's problem.
This patch handles active and inactie with fair.
That's because we can't expect where from and how many compaction would
isolated pages.
This patch changes (nr_isolated > nr_inactive) with
nr_isolated > (nr_active + nr_inactive) / 2.
Cc: Mel Gorman <mel@xxxxxxxxx>
Cc: Wu Fengguang <fengguang.wu@xxxxxxxxx>
Signed-off-by: Minchan Kim <minchan.kim@xxxxxxxxx>
---
mm/compaction.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 94cce51..0864839 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -214,15 +214,16 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
/* Similar to reclaim, but different enough that they don't share logic */
static bool too_many_isolated(struct zone *zone)
{
-
- unsigned long inactive, isolated;
+ unsigned long active, inactive, isolated;
inactive = zone_page_state(zone, NR_INACTIVE_FILE) +
zone_page_state(zone, NR_INACTIVE_ANON);
+ active = zone_page_state(zone, NR_ACTIVE_FILE) +
+ zone_page_state(zone, NR_ACTIVE_ANON);
isolated = zone_page_state(zone, NR_ISOLATED_FILE) +
zone_page_state(zone, NR_ISOLATED_ANON);
-
- return isolated > inactive;
+
+ return isolated > (inactive + active) / 2;
}
/*
--
1.7.0.5
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>