[PATCH][BUGFIX] memcg fix zone congestion

[PATCH][BUGFIX] memcg fix zone congestion

[KAMEZAWA Hiroyuki]

ZONE_CONGESTED should be a state of global memory reclaim.
If not, a busy memcg sets this and give unnecessary throttoling in
wait_iff_congested() against memory recalim in other contexts. This makes
system performance bad.

I'll think about "memcg is congested!" flag is required or not, later.
But this fix is required 1st.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/vmscan.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: mmotm-May11/mm/vmscan.c
===================================================================
--- mmotm-May11.orig/mm/vmscan.c
+++ mmotm-May11/mm/vmscan.c
@@ -941,7 +941,8 @@ keep_lumpy:
 	 * back off and wait for congestion to clear because further reclaim
 	 * will encounter the same problem
 	 */
-	if (nr_dirty == nr_congested && nr_dirty != 0)
+	if (scanning_global_lru(sc) &&
+	    nr_dirty == nr_congested && nr_dirty != 0)
 		zone_set_flag(zone, ZONE_CONGESTED);
 
 	free_page_list(&free_pages);

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[Minchan Kim]

On Fri, May 13, 2011 at 12:10 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> ZONE_CONGESTED should be a state of global memory reclaim.
> If not, a busy memcg sets this and give unnecessary throttoling in
> wait_iff_congested() against memory recalim in other contexts. This makes
> system performance bad.
>
> I'll think about "memcg is congested!" flag is required or not, later.
> But this fix is required 1st.

It's reasonable.

>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[Daisuke Nishimura]

On Fri, 13 May 2011 12:10:30 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> 
> ZONE_CONGESTED should be a state of global memory reclaim.
> If not, a busy memcg sets this and give unnecessary throttoling in
> wait_iff_congested() against memory recalim in other contexts. This makes
> system performance bad.
> 
hmm, nice catch.

Just from my curiosity, is there any number of performance improvement by this patch?

Thanks, 
Daisuke Nishimura.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[Ying Han]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Thu, May 12, 2011 at 8:10 PM, KAMEZAWA Hiroyuki <
kamezawa.hiroyu@jp.fujitsu.com> wrote:

>
> ZONE_CONGESTED should be a state of global memory reclaim.
> If not, a busy memcg sets this and give unnecessary throttoling in
> wait_iff_congested() against memory recalim in other contexts. This makes
> system performance bad.
>
> I'll think about "memcg is congested!" flag is required or not, later.
> But this fix is required 1st.
>

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: mmotm-May11/mm/vmscan.c
> ===================================================================
> --- mmotm-May11.orig/mm/vmscan.c
> +++ mmotm-May11/mm/vmscan.c
> @@ -941,7 +941,8 @@ keep_lumpy:
>         * back off and wait for congestion to clear because further reclaim
>         * will encounter the same problem
>         */
> -       if (nr_dirty == nr_congested && nr_dirty != 0)
> +       if (scanning_global_lru(sc) &&
> +           nr_dirty == nr_congested && nr_dirty != 0)
>                zone_set_flag(zone, ZONE_CONGESTED);
>
>        free_page_list(&free_pages);
>
> For memcg, wonder if we should make it per-memcg-per-zone congested.

Acked-by: Ying Han <yinghan@google.com>

--Ying

[-- Attachment #2: Type: text/html, Size: 2487 bytes --]

Re: [PATCH][BUGFIX] memcg fix zone congestion

[KAMEZAWA Hiroyuki]

On Fri, 13 May 2011 13:15:14 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Fri, 13 May 2011 12:10:30 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > 
> > ZONE_CONGESTED should be a state of global memory reclaim.
> > If not, a busy memcg sets this and give unnecessary throttoling in
> > wait_iff_congested() against memory recalim in other contexts. This makes
> > system performance bad.
> > 
> hmm, nice catch.
> 
> Just from my curiosity, is there any number of performance improvement by this patch?
> 

I guess impact of this patch is very limited. (And I've tested but cannot measure
the score. I need some special application/settings.)

I think, with small memcg, we'll see nr_dirty==nr_congested situaion often and
set ZONE_CONGESTED. But the effect of patch can be seen in the another place.

To see the effect of patch, the system need to reclaim memory eagerly with memory
shortage. In that situation, everything is slow. This patch just removes a extra
burden which is put on by mistake.


Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[Andrew Morton]

On Fri, 13 May 2011 12:10:30 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> 
> ZONE_CONGESTED should be a state of global memory reclaim.
> If not, a busy memcg sets this and give unnecessary throttoling in
> wait_iff_congested() against memory recalim in other contexts. This makes
> system performance bad.
> 
> I'll think about "memcg is congested!" flag is required or not, later.
> But this fix is required 1st.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: mmotm-May11/mm/vmscan.c
> ===================================================================
> --- mmotm-May11.orig/mm/vmscan.c
> +++ mmotm-May11/mm/vmscan.c
> @@ -941,7 +941,8 @@ keep_lumpy:
>  	 * back off and wait for congestion to clear because further reclaim
>  	 * will encounter the same problem
>  	 */
> -	if (nr_dirty == nr_congested && nr_dirty != 0)
> +	if (scanning_global_lru(sc) &&
> +	    nr_dirty == nr_congested && nr_dirty != 0)
>  		zone_set_flag(zone, ZONE_CONGESTED);
>  

nit: which is more probable?  nr_dirty==nr_congested or
scanning_global_lru(sc)?  

If the user is actually _using_ memcg then

--- a/mm/vmscan.c~a
+++ a/mm/vmscan.c
@@ -937,7 +937,7 @@ keep_lumpy:
 	 * back off and wait for congestion to clear because further reclaim
 	 * will encounter the same problem
 	 */
-	if (nr_dirty == nr_congested && nr_dirty != 0)
+	if (nr_dirty == nr_congested && scanning_global_lru(sc) && nr_dirty)
 		zone_set_flag(zone, ZONE_CONGESTED);
 
 	free_page_list(&free_pages);


is more efficient.  If the user isn't using memcg then your patch is faster?

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[KAMEZAWA Hiroyuki]

On Thu, 12 May 2011 22:25:10 -0700
Ying Han <yinghan@google.com> wrote:

> On Thu, May 12, 2011 at 8:10 PM, KAMEZAWA Hiroyuki <
> kamezawa.hiroyu@jp.fujitsu.com> wrote:

> >  mm/vmscan.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: mmotm-May11/mm/vmscan.c
> > ===================================================================
> > --- mmotm-May11.orig/mm/vmscan.c
> > +++ mmotm-May11/mm/vmscan.c
> > @@ -941,7 +941,8 @@ keep_lumpy:
> >         * back off and wait for congestion to clear because further reclaim
> >         * will encounter the same problem
> >         */
> > -       if (nr_dirty == nr_congested && nr_dirty != 0)
> > +       if (scanning_global_lru(sc) &&
> > +           nr_dirty == nr_congested && nr_dirty != 0)
> >                zone_set_flag(zone, ZONE_CONGESTED);
> >
> >        free_page_list(&free_pages);
> >
> > For memcg, wonder if we should make it per-memcg-per-zone congested.
> 

I guess dirty ratio should come 1st. If we don't have it, I think
nr_dirty==nr_congested very easily.

Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH][BUGFIX] memcg fix zone congestion

[KAMEZAWA Hiroyuki]

On Thu, 12 May 2011 23:25:00 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 13 May 2011 12:10:30 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > 
> > ZONE_CONGESTED should be a state of global memory reclaim.
> > If not, a busy memcg sets this and give unnecessary throttoling in
> > wait_iff_congested() against memory recalim in other contexts. This makes
> > system performance bad.
> > 
> > I'll think about "memcg is congested!" flag is required or not, later.
> > But this fix is required 1st.
> > 
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> >  mm/vmscan.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Index: mmotm-May11/mm/vmscan.c
> > ===================================================================
> > --- mmotm-May11.orig/mm/vmscan.c
> > +++ mmotm-May11/mm/vmscan.c
> > @@ -941,7 +941,8 @@ keep_lumpy:
> >  	 * back off and wait for congestion to clear because further reclaim
> >  	 * will encounter the same problem
> >  	 */
> > -	if (nr_dirty == nr_congested && nr_dirty != 0)
> > +	if (scanning_global_lru(sc) &&
> > +	    nr_dirty == nr_congested && nr_dirty != 0)
> >  		zone_set_flag(zone, ZONE_CONGESTED);
> >  
> 
> nit: which is more probable?  nr_dirty==nr_congested or
> scanning_global_lru(sc)?  
> 
> If the user is actually _using_ memcg then
> 

If the user uses memcg, yes, nr_dirty == nr_congested is more probable.
If user doesn't, scanning_global_lru() returns always true.

> --- a/mm/vmscan.c~a
> +++ a/mm/vmscan.c
> @@ -937,7 +937,7 @@ keep_lumpy:
>  	 * back off and wait for congestion to clear because further reclaim
>  	 * will encounter the same problem
>  	 */
> -	if (nr_dirty == nr_congested && nr_dirty != 0)
> +	if (nr_dirty == nr_congested && scanning_global_lru(sc) && nr_dirty)
>  		zone_set_flag(zone, ZONE_CONGESTED);
>  
>  	free_page_list(&free_pages);
> 
> 
> is more efficient.  If the user isn't using memcg then your patch is faster?
> 

Hmm, maybe your fix is always faster. But in many case, if nr_congested == nr_dirty,
nr_dirty == 0 because vmscan just finds clean pages...fast path.
So, nr_dirty should be 1st ?

How about this ?
==

ZONE_CONGESTED should be a state of global memory reclaim.
Changelog:v1->v2
 - fixed the order of conditions.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/vmscan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: mmotm-May11/mm/vmscan.c
===================================================================
--- mmotm-May11.orig/mm/vmscan.c
+++ mmotm-May11/mm/vmscan.c
@@ -941,7 +941,7 @@ keep_lumpy:
 	 * back off and wait for congestion to clear because further reclaim
 	 * will encounter the same problem
 	 */
-	if (nr_dirty == nr_congested && nr_dirty != 0)
+	if (nr_dirty && nr_dirty == nr_congested &&  scanning_global_lru(sc))
 		zone_set_flag(zone, ZONE_CONGESTED);
 
 	free_page_list(&free_pages);





--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>