Commit f50de2d38 seems to be breaking my oom killer

Commit f50de2d38 seems to be breaking my oom killer

[Will Newton]

Hi,

I'm having some problems on a small embedded box with 24Mb of RAM and
no swap. If a process tries to use large amounts of memory and gets
OOM killed, with 2.6.32 it's fine, but with 2.6.33-rc2 kswapd gets
stuck and the system locks up. The problem appears to have been
introduced with f50de2d38. If I change sleeping_prematurely to skip
the for_each_populated_zone test then OOM killing operates as
expected. I'm guessing it's caused by the new code not allowing kswapd
to schedule when it is required to let the killed task exit. Does that
sound plausible?

I'll try and investigate further into what's going on.

Thanks,

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Mel Gorman]

On Thu, Jan 07, 2010 at 12:34:54PM +0000, Will Newton wrote:
> Hi,
> 
> I'm having some problems on a small embedded box with 24Mb of RAM and
> no swap. If a process tries to use large amounts of memory and gets
> OOM killed, with 2.6.32 it's fine, but with 2.6.33-rc2 kswapd gets
> stuck and the system locks up.

By stuck, do you mean it consumes 100% CPU and never goes to sleep?

> The problem appears to have been
> introduced with f50de2d38. If I change sleeping_prematurely to skip
> the for_each_populated_zone test then OOM killing operates as
> expected. I'm guessing it's caused by the new code not allowing kswapd
> to schedule when it is required to let the killed task exit. Does that
> sound plausible?
> 

It's conceivable. The expectation was that the cond_resched() in
balance_pgdat() should have been called at

        if (!all_zones_ok) {
                cond_resched();

But it would appear that if all zones are unreclaimable, all_zones_ok == 1.
It could be looping there indefinitly never calling schedule because it
never reaches the points where cond_resched is called.

> I'll try and investigate further into what's going on.
> 

Can you try the following?

==== CUT HERE ====
vmscan: kswapd should notice that all zones are not ok if they are unreclaimble

In the event all zones are unreclaimble, it is possible for kswapd to
never go to sleep because "all zones are ok even though watermarks are
not reached". It gets into a situation where cond_reched() is not
called.

This patch notes that if all zones are unreclaimable then the zones are
not ok and cond_resched() should be called.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
--- 
 mm/vmscan.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2ad8603..d3c0848 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2022,8 +2022,10 @@ loop_again:
 				break;
 			}
 		}
-		if (i < 0)
+		if (i < 0) {
+			all_zones_ok = 0;
 			goto out;
+		}
 
 		for (i = 0; i <= end_zone; i++) {
 			struct zone *zone = pgdat->node_zones + i;

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Will Newton]

On Thu, Jan 7, 2010 at 1:58 PM, Mel Gorman <mel@csn.ul.ie> wrote:
> On Thu, Jan 07, 2010 at 12:34:54PM +0000, Will Newton wrote:
>> Hi,
>>
>> I'm having some problems on a small embedded box with 24Mb of RAM and
>> no swap. If a process tries to use large amounts of memory and gets
>> OOM killed, with 2.6.32 it's fine, but with 2.6.33-rc2 kswapd gets
>> stuck and the system locks up.
>
> By stuck, do you mean it consumes 100% CPU and never goes to sleep?

I assume so. The system sems locked up so ctrl-c of the process
doesn't work and I can't get in via telnet. Looking where the pc and
return pointer are going via JTAG leads me to believe it's stuck in
kswapd.

>> The problem appears to have been
>> introduced with f50de2d38. If I change sleeping_prematurely to skip
>> the for_each_populated_zone test then OOM killing operates as
>> expected. I'm guessing it's caused by the new code not allowing kswapd
>> to schedule when it is required to let the killed task exit. Does that
>> sound plausible?
>>
>
> It's conceivable. The expectation was that the cond_resched() in
> balance_pgdat() should have been called at
>
>        if (!all_zones_ok) {
>                cond_resched();
>
> But it would appear that if all zones are unreclaimable, all_zones_ok == 1.
> It could be looping there indefinitly never calling schedule because it
> never reaches the points where cond_resched is called.
>
>> I'll try and investigate further into what's going on.
>>
>
> Can you try the following?
>
> ==== CUT HERE ====
> vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
>
> In the event all zones are unreclaimble, it is possible for kswapd to
> never go to sleep because "all zones are ok even though watermarks are
> not reached". It gets into a situation where cond_reched() is not
> called.
>
> This patch notes that if all zones are unreclaimable then the zones are
> not ok and cond_resched() should be called.
>
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>

This fixes the problem, thanks for the quick response!

Tested-by: Will Newton <will.newton@gmail.com>

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Mel Gorman]

On Thu, Jan 07, 2010 at 02:15:42PM +0000, Will Newton wrote:
> On Thu, Jan 7, 2010 at 1:58 PM, Mel Gorman <mel@csn.ul.ie> wrote:
> > On Thu, Jan 07, 2010 at 12:34:54PM +0000, Will Newton wrote:
> >> Hi,
> >>
> >> I'm having some problems on a small embedded box with 24Mb of RAM and
> >> no swap. If a process tries to use large amounts of memory and gets
> >> OOM killed, with 2.6.32 it's fine, but with 2.6.33-rc2 kswapd gets
> >> stuck and the system locks up.
> >
> > By stuck, do you mean it consumes 100% CPU and never goes to sleep?
> 
> I assume so. The system sems locked up so ctrl-c of the process
> doesn't work and I can't get in via telnet. Looking where the pc and
> return pointer are going via JTAG leads me to believe it's stuck in
> kswapd.
> 

Sounds like kswapd is stuck in an infinite loop and I'm guessing your
machine has just one CPU so the task to be killed never gets onto the
CPU.

> >> The problem appears to have been
> >> introduced with f50de2d38. If I change sleeping_prematurely to skip
> >> the for_each_populated_zone test then OOM killing operates as
> >> expected. I'm guessing it's caused by the new code not allowing kswapd
> >> to schedule when it is required to let the killed task exit. Does that
> >> sound plausible?
> >>
> >
> > It's conceivable. The expectation was that the cond_resched() in
> > balance_pgdat() should have been called at
> >
> >        if (!all_zones_ok) {
> >                cond_resched();
> >
> > But it would appear that if all zones are unreclaimable, all_zones_ok == 1.
> > It could be looping there indefinitly never calling schedule because it
> > never reaches the points where cond_resched is called.
> >
> >> I'll try and investigate further into what's going on.
> >>
> >
> > Can you try the following?
> >
> > ==== CUT HERE ====
> > vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> >
> > In the event all zones are unreclaimble, it is possible for kswapd to
> > never go to sleep because "all zones are ok even though watermarks are
> > not reached". It gets into a situation where cond_reched() is not
> > called.
> >
> > This patch notes that if all zones are unreclaimable then the zones are
> > not ok and cond_resched() should be called.
> >
> > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> 
> This fixes the problem, thanks for the quick response!
> 
> Tested-by: Will Newton <will.newton@gmail.com>
> 

Perfect. Thanks for reporting and testing.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Minchan Kim]

Hi, Mel 

On Thu, 7 Jan 2010 13:58:31 +0000
Mel Gorman <mel@csn.ul.ie> wrote:

> vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> 
> In the event all zones are unreclaimble, it is possible for kswapd to
> never go to sleep because "all zones are ok even though watermarks are
> not reached". It gets into a situation where cond_reched() is not
> called.
> 
> This patch notes that if all zones are unreclaimable then the zones are
> not ok and cond_resched() should be called.
> 
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> --- 
>  mm/vmscan.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 2ad8603..d3c0848 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2022,8 +2022,10 @@ loop_again:
>  				break;
>  			}
>  		}
> -		if (i < 0)
> +		if (i < 0) {
> +			all_zones_ok = 0;
>  			goto out;
> +		}
>  
>  		for (i = 0; i <= end_zone; i++) {
>  			struct zone *zone = pgdat->node_zones + i;
> 
> --

Nice catch!
Don't we care following as although it is rare case?

---
                for (i = 0; i <= end_zone; i++) {
                        struct zone *zone = pgdat->node_zones + i; 
                        int nr_slab;
                        int nid, zid; 

                        if (!populated_zone(zone))
                                continue;

                        if (zone_is_all_unreclaimable(zone) &&
                                        priority != DEF_PRIORITY)
                                continue;  <==== here

---

And while I review all_zones_ok'usage in balance_pgdat, 
I feel it's not consistent and rather confused. 
How about this?

== CUT_HERE ==

Re: Commit f50de2d38 seems to be breaking my oom killer

[KOSAKI Motohiro]

> Hi, Mel 
> 
> On Thu, 7 Jan 2010 13:58:31 +0000
> Mel Gorman <mel@csn.ul.ie> wrote:
> 
> > vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> > 
> > In the event all zones are unreclaimble, it is possible for kswapd to
> > never go to sleep because "all zones are ok even though watermarks are
> > not reached". It gets into a situation where cond_reched() is not
> > called.
> > 
> > This patch notes that if all zones are unreclaimable then the zones are
> > not ok and cond_resched() should be called.
> > 
> > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > --- 
> >  mm/vmscan.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 2ad8603..d3c0848 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -2022,8 +2022,10 @@ loop_again:
> >  				break;
> >  			}
> >  		}
> > -		if (i < 0)
> > +		if (i < 0) {
> > +			all_zones_ok = 0;
> >  			goto out;
> > +		}
> >  
> >  		for (i = 0; i <= end_zone; i++) {
> >  			struct zone *zone = pgdat->node_zones + i;
> > 
> > --
> 
> Nice catch!
> Don't we care following as although it is rare case?
> 
> ---
>                 for (i = 0; i <= end_zone; i++) {
>                         struct zone *zone = pgdat->node_zones + i; 
>                         int nr_slab;
>                         int nid, zid; 
> 
>                         if (!populated_zone(zone))
>                                 continue;
> 
>                         if (zone_is_all_unreclaimable(zone) &&
>                                         priority != DEF_PRIORITY)
>                                 continue;  <==== here
> 
> ---
> 
> And while I review all_zones_ok'usage in balance_pgdat, 
> I feel it's not consistent and rather confused. 
> How about this?

Can you please read my patch?



--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[KOSAKI Motohiro]

> > Hi, Mel 
> > 
> > On Thu, 7 Jan 2010 13:58:31 +0000
> > Mel Gorman <mel@csn.ul.ie> wrote:
> > 
> > > vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> > > 
> > > In the event all zones are unreclaimble, it is possible for kswapd to
> > > never go to sleep because "all zones are ok even though watermarks are
> > > not reached". It gets into a situation where cond_reched() is not
> > > called.
> > > 
> > > This patch notes that if all zones are unreclaimable then the zones are
> > > not ok and cond_resched() should be called.
> > > 
> > > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > > --- 
> > >  mm/vmscan.c |    4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 2ad8603..d3c0848 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -2022,8 +2022,10 @@ loop_again:
> > >  				break;
> > >  			}
> > >  		}
> > > -		if (i < 0)
> > > +		if (i < 0) {
> > > +			all_zones_ok = 0;
> > >  			goto out;
> > > +		}
> > >  
> > >  		for (i = 0; i <= end_zone; i++) {
> > >  			struct zone *zone = pgdat->node_zones + i;
> > > 
> > > --
> > 
> > Nice catch!
> > Don't we care following as although it is rare case?
> > 
> > ---
> >                 for (i = 0; i <= end_zone; i++) {
> >                         struct zone *zone = pgdat->node_zones + i; 
> >                         int nr_slab;
> >                         int nid, zid; 
> > 
> >                         if (!populated_zone(zone))
> >                                 continue;
> > 
> >                         if (zone_is_all_unreclaimable(zone) &&
> >                                         priority != DEF_PRIORITY)
> >                                 continue;  <==== here
> > 
> > ---
> > 
> > And while I review all_zones_ok'usage in balance_pgdat, 
> > I feel it's not consistent and rather confused. 
> > How about this?
> 
> Can you please read my patch?

Grr. I'm sorry. such thread don't CCed LKML.
cut-n-past here.


----------------------------------------
Umm..
This code looks a bit risky. Please imazine asymmetric numa. If the system has
very small node, its nude have unreclaimable state at almost time.

Thus, if all zones in the node are unreclaimable, It should be slept. To retry balance_pgdat()
is meaningless. this is original intention, I think.

So why can't we write following?

From c00d7bb29552d3aa4d934b5007f3d52ade5f2dfd Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 8 Jan 2010 08:36:05 +0900
Subject: [PATCH] vmscan: kswapd don't retry balance_pgdat() if all zones are unreclaimable

Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
double check it should be asleep) can cause kswapd to enter an infinite
loop if running on a single-CPU system. If all zones are unreclaimble,
sleeping_prematurely return 1 and kswapd will call balance_pgdat()
again. but it's totally meaningless, balance_pgdat() doesn't anything
against unreclaimable zone!

Cc: Mel Gorman <mel@csn.ul.ie>
Reported-by: Will Newton <will.newton@gmail.com>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2bbee91..56327d5 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1922,6 +1922,9 @@ static int sleeping_prematurely(pg_data_t *pgdat, int order, long remaining)
 		if (!populated_zone(zone))
 			continue;
 
+		if (zone_is_all_unreclaimable(zone))
+			continue;
+
 		if (!zone_watermark_ok(zone, order, high_wmark_pages(zone),
 								0, 0))
 			return 1;
-- 
1.6.5.2







--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Minchan Kim]

On Fri,  8 Jan 2010 13:08:46 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > > Hi, Mel 
> > > 
> > > On Thu, 7 Jan 2010 13:58:31 +0000
> > > Mel Gorman <mel@csn.ul.ie> wrote:
> > > 
> > > > vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> > > > 
> > > > In the event all zones are unreclaimble, it is possible for kswapd to
> > > > never go to sleep because "all zones are ok even though watermarks are
> > > > not reached". It gets into a situation where cond_reched() is not
> > > > called.
> > > > 
> > > > This patch notes that if all zones are unreclaimable then the zones are
> > > > not ok and cond_resched() should be called.
> > > > 
> > > > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > > > --- 
> > > >  mm/vmscan.c |    4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > > index 2ad8603..d3c0848 100644
> > > > --- a/mm/vmscan.c
> > > > +++ b/mm/vmscan.c
> > > > @@ -2022,8 +2022,10 @@ loop_again:
> > > >  				break;
> > > >  			}
> > > >  		}
> > > > -		if (i < 0)
> > > > +		if (i < 0) {
> > > > +			all_zones_ok = 0;
> > > >  			goto out;
> > > > +		}
> > > >  
> > > >  		for (i = 0; i <= end_zone; i++) {
> > > >  			struct zone *zone = pgdat->node_zones + i;
> > > > 
> > > > --
> > > 
> > > Nice catch!
> > > Don't we care following as although it is rare case?
> > > 
> > > ---
> > >                 for (i = 0; i <= end_zone; i++) {
> > >                         struct zone *zone = pgdat->node_zones + i; 
> > >                         int nr_slab;
> > >                         int nid, zid; 
> > > 
> > >                         if (!populated_zone(zone))
> > >                                 continue;
> > > 
> > >                         if (zone_is_all_unreclaimable(zone) &&
> > >                                         priority != DEF_PRIORITY)
> > >                                 continue;  <==== here
> > > 
> > > ---
> > > 
> > > And while I review all_zones_ok'usage in balance_pgdat, 
> > > I feel it's not consistent and rather confused. 
> > > How about this?
> > 
> > Can you please read my patch?
> 
> Grr. I'm sorry. such thread don't CCed LKML.
> cut-n-past here.
> 
> 
> ----------------------------------------
> Umm..
> This code looks a bit risky. Please imazine asymmetric numa. If the system has
> very small node, its nude have unreclaimable state at almost time.
> 
> Thus, if all zones in the node are unreclaimable, It should be slept. To retry balance_pgdat()
> is meaningless. this is original intention, I think.
> 
> So why can't we write following?
> 
> From c00d7bb29552d3aa4d934b5007f3d52ade5f2dfd Mon Sep 17 00:00:00 2001
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Date: Fri, 8 Jan 2010 08:36:05 +0900
> Subject: [PATCH] vmscan: kswapd don't retry balance_pgdat() if all zones are unreclaimable
> 
> Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
> double check it should be asleep) can cause kswapd to enter an infinite
> loop if running on a single-CPU system. If all zones are unreclaimble,
> sleeping_prematurely return 1 and kswapd will call balance_pgdat()
> again. but it's totally meaningless, balance_pgdat() doesn't anything
> against unreclaimable zone!
> 
> Cc: Mel Gorman <mel@csn.ul.ie>
> Reported-by: Will Newton <will.newton@gmail.com>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

Looks good than mine. Thnaks, Kosaki. 
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>


-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Rik van Riel]

On 01/07/2010 11:08 PM, KOSAKI Motohiro wrote:

> Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
> double check it should be asleep) can cause kswapd to enter an infinite
> loop if running on a single-CPU system. If all zones are unreclaimble,
> sleeping_prematurely return 1 and kswapd will call balance_pgdat()
> again. but it's totally meaningless, balance_pgdat() doesn't anything
> against unreclaimable zone!
>
> Cc: Mel Gorman<mel@csn.ul.ie>
> Reported-by: Will Newton<will.newton@gmail.com>
> Signed-off-by: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed.

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Mel Gorman]

On Fri, Jan 08, 2010 at 01:08:46PM +0900, KOSAKI Motohiro wrote:
> > > Hi, Mel 
> > > 
> > > On Thu, 7 Jan 2010 13:58:31 +0000
> > > Mel Gorman <mel@csn.ul.ie> wrote:
> > > 
> > > > vmscan: kswapd should notice that all zones are not ok if they are unreclaimble
> > > > 
> > > > In the event all zones are unreclaimble, it is possible for kswapd to
> > > > never go to sleep because "all zones are ok even though watermarks are
> > > > not reached". It gets into a situation where cond_reched() is not
> > > > called.
> > > > 
> > > > This patch notes that if all zones are unreclaimable then the zones are
> > > > not ok and cond_resched() should be called.
> > > > 
> > > > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > > > --- 
> > > >  mm/vmscan.c |    4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > > index 2ad8603..d3c0848 100644
> > > > --- a/mm/vmscan.c
> > > > +++ b/mm/vmscan.c
> > > > @@ -2022,8 +2022,10 @@ loop_again:
> > > >  				break;
> > > >  			}
> > > >  		}
> > > > -		if (i < 0)
> > > > +		if (i < 0) {
> > > > +			all_zones_ok = 0;
> > > >  			goto out;
> > > > +		}
> > > >  
> > > >  		for (i = 0; i <= end_zone; i++) {
> > > >  			struct zone *zone = pgdat->node_zones + i;
> > > > 
> > > > --
> > > 
> > > Nice catch!
> > > Don't we care following as although it is rare case?
> > > 
> > > ---
> > >                 for (i = 0; i <= end_zone; i++) {
> > >                         struct zone *zone = pgdat->node_zones + i; 
> > >                         int nr_slab;
> > >                         int nid, zid; 
> > > 
> > >                         if (!populated_zone(zone))
> > >                                 continue;
> > > 
> > >                         if (zone_is_all_unreclaimable(zone) &&
> > >                                         priority != DEF_PRIORITY)
> > >                                 continue;  <==== here
> > > 
> > > ---
> > > 
> > > And while I review all_zones_ok'usage in balance_pgdat, 
> > > I feel it's not consistent and rather confused. 
> > > How about this?
> > 
> > Can you please read my patch?
> 
> Grr. I'm sorry. such thread don't CCed LKML.
> cut-n-past here.
> 
> 
> ----------------------------------------
> Umm..
> This code looks a bit risky. Please imazine asymmetric numa. If the system has
> very small node, its nude have unreclaimable state at almost time.
> 
> Thus, if all zones in the node are unreclaimable, It should be slept. To retry balance_pgdat()
> is meaningless. this is original intention, I think.
> 
> So why can't we write following?
> 
> From c00d7bb29552d3aa4d934b5007f3d52ade5f2dfd Mon Sep 17 00:00:00 2001
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Date: Fri, 8 Jan 2010 08:36:05 +0900
> Subject: [PATCH] vmscan: kswapd don't retry balance_pgdat() if all zones are unreclaimable
> 
> Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
> double check it should be asleep) can cause kswapd to enter an infinite
> loop if running on a single-CPU system. If all zones are unreclaimble,
> sleeping_prematurely return 1 and kswapd will call balance_pgdat()
> again. but it's totally meaningless, balance_pgdat() doesn't anything
> against unreclaimable zone!
> 

Sure, that would be a safer check in the face of very small NUMA nodes.
It could do with a comment explaining why unreclaimable zones are being skipped
but it's no biggie.  Will, can you confirm this patch also fixes your problem.

Kosaki, if Will reports success, can you then report that patch please
for upstreaming?  After today, I'm offline for a week so it'd be at
least 10 days before I'd do it. Thanks

> Cc: Mel Gorman <mel@csn.ul.ie>
> Reported-by: Will Newton <will.newton@gmail.com>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 2bbee91..56327d5 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1922,6 +1922,9 @@ static int sleeping_prematurely(pg_data_t *pgdat, int order, long remaining)
>  		if (!populated_zone(zone))
>  			continue;
>  
> +		if (zone_is_all_unreclaimable(zone))
> +			continue;
> +
>  		if (!zone_watermark_ok(zone, order, high_wmark_pages(zone),
>  								0, 0))
>  			return 1;
> -- 
> 1.6.5.2
> 
> 
> 
> 
> 
> 
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[KOSAKI Motohiro]

> > Umm..
> > This code looks a bit risky. Please imazine asymmetric numa. If the system has
> > very small node, its nude have unreclaimable state at almost time.
> > 
> > Thus, if all zones in the node are unreclaimable, It should be slept. To retry balance_pgdat()
> > is meaningless. this is original intention, I think.
> > 
> > So why can't we write following?
> > 
> > From c00d7bb29552d3aa4d934b5007f3d52ade5f2dfd Mon Sep 17 00:00:00 2001
> > From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Date: Fri, 8 Jan 2010 08:36:05 +0900
> > Subject: [PATCH] vmscan: kswapd don't retry balance_pgdat() if all zones are unreclaimable
> > 
> > Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
> > double check it should be asleep) can cause kswapd to enter an infinite
> > loop if running on a single-CPU system. If all zones are unreclaimble,
> > sleeping_prematurely return 1 and kswapd will call balance_pgdat()
> > again. but it's totally meaningless, balance_pgdat() doesn't anything
> > against unreclaimable zone!
> > 
> 
> Sure, that would be a safer check in the face of very small NUMA nodes.
> It could do with a comment explaining why unreclaimable zones are being skipped
> but it's no biggie.  Will, can you confirm this patch also fixes your problem.
> 
> Kosaki, if Will reports success, can you then report that patch please
> for upstreaming?  After today, I'm offline for a week so it'd be at
> least 10 days before I'd do it. Thanks

Sure. thanks.


--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Will Newton]

On Fri, Jan 8, 2010 at 4:08 AM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> wrote:
>> > And while I review all_zones_ok'usage in balance_pgdat,
>> > I feel it's not consistent and rather confused.
>> > How about this?
>>
>> Can you please read my patch?
>
> Grr. I'm sorry. such thread don't CCed LKML.
> cut-n-past here.
>
>
> ----------------------------------------
> Umm..
> This code looks a bit risky. Please imazine asymmetric numa. If the system has
> very small node, its nude have unreclaimable state at almost time.
>
> Thus, if all zones in the node are unreclaimable, It should be slept. To retry balance_pgdat()
> is meaningless. this is original intention, I think.
>
> So why can't we write following?

Hi Kosaki,

This patch fixes the problem for me too, thanks!

Tested-by: Will Newton <will.newton@gmail.com>

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Commit f50de2d38 seems to be breaking my oom killer

[Wu Fengguang]

> Subject: [PATCH] vmscan: kswapd don't retry balance_pgdat() if all zones are unreclaimable
> 
> Commit f50de2d3 (vmscan: have kswapd sleep for a short interval and
> double check it should be asleep) can cause kswapd to enter an infinite
> loop if running on a single-CPU system. If all zones are unreclaimble,
> sleeping_prematurely return 1 and kswapd will call balance_pgdat()
> again. but it's totally meaningless, balance_pgdat() doesn't anything
> against unreclaimable zone!
 
Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>

Thanks!

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>