(resend) [PATCH] vmscan,memcg: memcg aware swap token

(resend) [PATCH] vmscan,memcg: memcg aware swap token

[KOSAKI Motohiro]

Currently, memcg reclaim can disable swap token even if the swap token
mm doesn't belong in its memory cgroup. It's slightly riskly. If an
admin makes very small mem-cgroup and silly guy runs contenious heavy
memory pressure workloa, whole tasks in the system are going to lose
swap-token and then system may become unresponsive. That's bad.

This patch adds 'memcg' parameter into disable_swap_token(). and if
the parameter doesn't match swap-token, VM doesn't put swap-token.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/memcontrol.h |    6 ++++++
 include/linux/swap.h       |   24 +++++++++++++++++-------
 mm/memcontrol.c            |    2 +-
 mm/thrash.c                |   17 +++++++++++++++++
 mm/vmscan.c                |    4 ++--
 5 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6a0cffd..df572af 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -84,6 +84,7 @@ int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *mem);
 
 extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
 extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p);
+extern struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm);
 
 static inline
 int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup)
@@ -244,6 +245,11 @@ static inline struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
 	return NULL;
 }
 
+static inline struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
+{
+	return NULL;
+}
+
 static inline int mm_match_cgroup(struct mm_struct *mm, struct mem_cgroup *mem)
 {
 	return 1;
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 384eb5f..ccea15d 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -358,21 +358,31 @@ struct backing_dev_info;
 extern struct mm_struct *swap_token_mm;
 extern void grab_swap_token(struct mm_struct *);
 extern void __put_swap_token(struct mm_struct *);
+extern int has_swap_token_memcg(struct mm_struct *mm, struct mem_cgroup *memcg);
 
-static inline int has_swap_token(struct mm_struct *mm)
+static inline
+int has_swap_token(struct mm_struct *mm)
 {
-	return (mm == swap_token_mm);
+	return has_swap_token_memcg(mm, NULL);
 }
 
-static inline void put_swap_token(struct mm_struct *mm)
+static inline
+void put_swap_token_memcg(struct mm_struct *mm, struct mem_cgroup *memcg)
 {
-	if (has_swap_token(mm))
+	if (has_swap_token_memcg(mm, memcg))
 		__put_swap_token(mm);
 }
 
-static inline void disable_swap_token(void)
+static inline
+void put_swap_token(struct mm_struct *mm)
+{
+	return put_swap_token_memcg(mm, NULL);
+}
+
+static inline
+void disable_swap_token(struct mem_cgroup *memcg)
 {
-	put_swap_token(swap_token_mm);
+	put_swap_token_memcg(swap_token_mm, memcg);
 }
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
@@ -500,7 +510,7 @@ static inline int has_swap_token(struct mm_struct *mm)
 	return 0;
 }
 
-static inline void disable_swap_token(void)
+static inline void disable_swap_token(struct mem_cgroup *memcg)
 {
 }
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c2776f1..5683c7a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -735,7 +735,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
 				struct mem_cgroup, css);
 }
 
-static struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
+struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm)
 {
 	struct mem_cgroup *mem = NULL;
 
diff --git a/mm/thrash.c b/mm/thrash.c
index 2372d4e..f892a6e 100644
--- a/mm/thrash.c
+++ b/mm/thrash.c
@@ -21,6 +21,7 @@
 #include <linux/mm.h>
 #include <linux/sched.h>
 #include <linux/swap.h>
+#include <linux/memcontrol.h>
 
 static DEFINE_SPINLOCK(swap_token_lock);
 struct mm_struct *swap_token_mm;
@@ -75,3 +76,19 @@ void __put_swap_token(struct mm_struct *mm)
 		swap_token_mm = NULL;
 	spin_unlock(&swap_token_lock);
 }
+
+int has_swap_token_memcg(struct mm_struct *mm, struct mem_cgroup *memcg)
+{
+	if (memcg) {
+		struct mem_cgroup *swap_token_memcg;
+
+		/*
+		 * memcgroup reclaim can disable swap token only if token task
+		 * is in the same cgroup.
+		 */
+		swap_token_memcg = try_get_mem_cgroup_from_mm(swap_token_mm);
+		return ((mm == swap_token_mm) && (memcg == swap_token_memcg));
+	} else
+		return (mm == swap_token_mm);
+}
+
diff --git a/mm/vmscan.c b/mm/vmscan.c
index b3a569f..19e179b 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2044,7 +2044,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
 		sc->nr_scanned = 0;
 		if (!priority)
-			disable_swap_token();
+			disable_swap_token(sc->mem_cgroup);
 		shrink_zones(priority, zonelist, sc);
 		/*
 		 * Don't shrink slabs when reclaiming memory from
@@ -2353,7 +2353,7 @@ loop_again:
 
 		/* The swap token gets in the way of swapout... */
 		if (!priority)
-			disable_swap_token();
+			disable_swap_token(NULL);
 
 		all_zones_ok = 1;
 		balanced = 0;
-- 
1.7.3.1




--
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: (resend) [PATCH] vmscan,memcg: memcg aware swap token

[KAMEZAWA Hiroyuki]

On Tue, 26 Apr 2011 16:59:19 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> Currently, memcg reclaim can disable swap token even if the swap token
> mm doesn't belong in its memory cgroup. It's slightly riskly. If an
> admin makes very small mem-cgroup and silly guy runs contenious heavy
> memory pressure workloa, whole tasks in the system are going to lose
> swap-token and then system may become unresponsive. That's bad.
> 
> This patch adds 'memcg' parameter into disable_swap_token(). and if
> the parameter doesn't match swap-token, VM doesn't put swap-token.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Ack. Thank you.

-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: (resend) [PATCH] vmscan,memcg: memcg aware swap token

[Andrew Morton]

On Tue, 26 Apr 2011 16:59:19 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> @@ -75,3 +76,19 @@ void __put_swap_token(struct mm_struct *mm)
>  		swap_token_mm = NULL;
>  	spin_unlock(&swap_token_lock);
>  }
> +
> +int has_swap_token_memcg(struct mm_struct *mm, struct mem_cgroup *memcg)
> +{
> +	if (memcg) {
> +		struct mem_cgroup *swap_token_memcg;
> +
> +		/*
> +		 * memcgroup reclaim can disable swap token only if token task
> +		 * is in the same cgroup.
> +		 */
> +		swap_token_memcg = try_get_mem_cgroup_from_mm(swap_token_mm);
> +		return ((mm == swap_token_mm) && (memcg == swap_token_memcg));
> +	} else
> +		return (mm == swap_token_mm);
> +}

Seems to be missing a css_put()?

Either I'm mistaken or that's a bug.  Perhaps neither of these would
have happened if we'd bothered to document
try_get_mem_cgroup_from_mm().

--
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: (resend) [PATCH] vmscan,memcg: memcg aware swap token

[KOSAKI Motohiro]

> On Tue, 26 Apr 2011 16:59:19 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > @@ -75,3 +76,19 @@ void __put_swap_token(struct mm_struct *mm)
> >  		swap_token_mm = NULL;
> >  	spin_unlock(&swap_token_lock);
> >  }
> > +
> > +int has_swap_token_memcg(struct mm_struct *mm, struct mem_cgroup *memcg)
> > +{
> > +	if (memcg) {
> > +		struct mem_cgroup *swap_token_memcg;
> > +
> > +		/*
> > +		 * memcgroup reclaim can disable swap token only if token task
> > +		 * is in the same cgroup.
> > +		 */
> > +		swap_token_memcg = try_get_mem_cgroup_from_mm(swap_token_mm);
> > +		return ((mm == swap_token_mm) && (memcg == swap_token_memcg));
> > +	} else
> > +		return (mm == swap_token_mm);
> > +}
> 
> Seems to be missing a css_put()?

Yes! Please drop this one. I'll rework it at this weekend.

Thank you for the finding!

> 
> Either I'm mistaken or that's a bug.  Perhaps neither of these would
> have happened if we'd bothered to document
> try_get_mem_cgroup_from_mm().
> 



--
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>