[RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[KAMEZAWA Hiroyuki]

Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h

I made a brief trial to use static_branch() for that function. At doing that,
I thought it will be better to implement generic cgroup functions rather
than having memory cgroup's its own one.

This series consists of 3 patches
1 .... implement cgroup_xxxx_disabled() in generic.
2 .... use jump_label for cgroup_xxxx_disabled()
3 .... remove mem_cgroup_disabled() in memcontrol.c

And I post this series for getting review/comments.
I'm not sure patches for using jump_label is worth to be merged.

I did a test to run a loop
	while(-) {
		mmap(1M)
		touch all pages
		munmap()
	}

and measured performance score in ROOT cgroup. Here,

(Before patch)
   182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
   192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
       761,483,416 cache-references                                             [49.98%]
           159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
    33,253,084,874 branches                                                     [33.34%]
       109,796,792 branch-misses             #    0.33% of all branches         [33.34%]

      58.289265709 seconds time elapsed

(After patch)
 Performance counter stats for './malloc 1':

   183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
   191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
       798,635,028 cache-references                                             [49.98%]
            95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
    32,755,318,286 branches                                                     [33.34%]
        77,774,624 branch-misses             #    0.24% of all branches         [33.34%]

      58.332356996 seconds time elapsed

There is no differece in 'time' ;) 
But I got an impression that 'branch' score gets better in several tests.

Thanks,
-Kame

P.S. maybe I can replace 'do_swap_account' with jump_label, too.



--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[KAMEZAWA Hiroyuki]

Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h

I made a brief trial to use static_branch() for that function. At doing that,
I thought it will be better to implement generic cgroup functions rather
than having memory cgroup's its own one.

This series consists of 3 patches
1 .... implement cgroup_xxxx_disabled() in generic.
2 .... use jump_label for cgroup_xxxx_disabled()
3 .... remove mem_cgroup_disabled() in memcontrol.c

And I post this series for getting review/comments.
I'm not sure patches for using jump_label is worth to be merged.

I did a test to run a loop
	while(-) {
		mmap(1M)
		touch all pages
		munmap()
	}

and measured performance score in ROOT cgroup. Here,

(Before patch)
   182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
   192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
       761,483,416 cache-references                                             [49.98%]
           159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
    33,253,084,874 branches                                                     [33.34%]
       109,796,792 branch-misses             #    0.33% of all branches         [33.34%]

      58.289265709 seconds time elapsed

(After patch)
 Performance counter stats for './malloc 1':

   183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
   191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
       798,635,028 cache-references                                             [49.98%]
            95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
    32,755,318,286 branches                                                     [33.34%]
        77,774,624 branch-misses             #    0.24% of all branches         [33.34%]

      58.332356996 seconds time elapsed

There is no differece in 'time' ;) 
But I got an impression that 'branch' score gets better in several tests.

Thanks,
-Kame

P.S. maybe I can replace 'do_swap_account' with jump_label, too.

[RFC][PATCH 1/3] add cgroup_xxxx_disabled functions

[KAMEZAWA Hiroyuki]

From 1a9e228c2a28bb918ea8cb9aecc11fd80444acaa Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Date: Wed, 23 Nov 2011 15:36:22 +0900
Subject: [PATCH 1/3] add cgroup_xxxx_disabled() functions

Now, memory cgroup uses mem_cgroup_disabled() for checking
cgroup is disabled or not. This patch adds cgroup_xxxx_disabled()
functions for all subsys. After all works, mem_cgroup_disabled()
can be disappeared.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
---
 include/linux/cgroup.h        |   18 ++++++++++++++++++
 include/linux/cgroup_subsys.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 1b7f9d5..9eaa6fe 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -526,6 +526,16 @@ struct cgroup_subsys {
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
+#define SUBSYS(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return _x ## _subsys.disabled;}
+#define SUBSYS_UNDEFINED(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return true;}
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+#undef SUBSYS_UNDEFINED
+
 static inline struct cgroup_subsys_state *cgroup_subsys_state(
 	struct cgroup *cgrp, int subsys_id)
 {
@@ -655,6 +665,14 @@ static inline int cgroup_attach_task_current_cg(struct task_struct *t)
 	return 0;
 }
 
+#define SUBSYS(_x)
+#define SUBSYS_UNDEFINED(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return true;}
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+#undef SUBSYS_UNDEFINED
+
 #endif /* !CONFIG_CGROUPS */
 
 #endif /* _LINUX_CGROUP_H */
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index ac663c1..f3eaee8 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -9,58 +9,98 @@
 
 #ifdef CONFIG_CPUSETS
 SUBSYS(cpuset)
+#else
+#ifdef SUBSYS_DEFINED
+SUBSYS_UNDEFINED(cpuset)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_DEBUG
 SUBSYS(debug)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(debug)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_SCHED
 SUBSYS(cpu_cgroup)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(cpu_cgroup)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_CPUACCT
 SUBSYS(cpuacct)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(cpuacct)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
 SUBSYS(mem_cgroup)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(mem_cgroup)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_DEVICE
 SUBSYS(devices)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(devices)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_FREEZER
 SUBSYS(freezer)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(freezer)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_NET_CLS_CGROUP
 SUBSYS(net_cls)
+#else
+#ifdef SUBSYS_DEFINED
+SUBSYS_UNDEFINED(net_cls)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_BLK_CGROUP
 SUBSYS(blkio)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(blkio)
+#endif
 #endif
 
 #ifdef CONFIG_CGROUP_PERF
 SUBSYS(perf)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(perf)
+#endif
 #endif
 
 /* */
-- 
1.7.4.1


--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH 1/3] add cgroup_xxxx_disabled functions

[KAMEZAWA Hiroyuki]

>From 1a9e228c2a28bb918ea8cb9aecc11fd80444acaa Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Wed, 23 Nov 2011 15:36:22 +0900
Subject: [PATCH 1/3] add cgroup_xxxx_disabled() functions

Now, memory cgroup uses mem_cgroup_disabled() for checking
cgroup is disabled or not. This patch adds cgroup_xxxx_disabled()
functions for all subsys. After all works, mem_cgroup_disabled()
can be disappeared.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/cgroup.h        |   18 ++++++++++++++++++
 include/linux/cgroup_subsys.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 1b7f9d5..9eaa6fe 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -526,6 +526,16 @@ struct cgroup_subsys {
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
+#define SUBSYS(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return _x ## _subsys.disabled;}
+#define SUBSYS_UNDEFINED(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return true;}
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+#undef SUBSYS_UNDEFINED
+
 static inline struct cgroup_subsys_state *cgroup_subsys_state(
 	struct cgroup *cgrp, int subsys_id)
 {
@@ -655,6 +665,14 @@ static inline int cgroup_attach_task_current_cg(struct task_struct *t)
 	return 0;
 }
 
+#define SUBSYS(_x)
+#define SUBSYS_UNDEFINED(_x)\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return true;}
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+#undef SUBSYS_UNDEFINED
+
 #endif /* !CONFIG_CGROUPS */
 
 #endif /* _LINUX_CGROUP_H */
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index ac663c1..f3eaee8 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -9,58 +9,98 @@
 
 #ifdef CONFIG_CPUSETS
 SUBSYS(cpuset)
+#else
+#ifdef SUBSYS_DEFINED
+SUBSYS_UNDEFINED(cpuset)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_DEBUG
 SUBSYS(debug)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(debug)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_SCHED
 SUBSYS(cpu_cgroup)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(cpu_cgroup)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_CPUACCT
 SUBSYS(cpuacct)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(cpuacct)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR
 SUBSYS(mem_cgroup)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(mem_cgroup)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_DEVICE
 SUBSYS(devices)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(devices)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_CGROUP_FREEZER
 SUBSYS(freezer)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(freezer)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_NET_CLS_CGROUP
 SUBSYS(net_cls)
+#else
+#ifdef SUBSYS_DEFINED
+SUBSYS_UNDEFINED(net_cls)
+#endif
 #endif
 
 /* */
 
 #ifdef CONFIG_BLK_CGROUP
 SUBSYS(blkio)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(blkio)
+#endif
 #endif
 
 #ifdef CONFIG_CGROUP_PERF
 SUBSYS(perf)
+#else
+#ifdef SUBSYS_UNDEFINED
+SUBSYS_UNDEFINED(perf)
+#endif
 #endif
 
 /* */
-- 
1.7.4.1

[RFC][PATCH 2/3] use static_branch for cgroup_xxxx_disabled

[KAMEZAWA Hiroyuki]

From 7eae9a2735bf8df8a7e12f10dd7c27e729760196 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Date: Wed, 23 Nov 2011 15:53:18 +0900
Subject: [PATCH 2/3] use static_branch for cgroup_xxx_disabled

The behavior of cgroup_xxx_disable is determined at boot.
Then, using static_brach() will allow us binary patching rather
than checking global variable.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
---
 include/linux/cgroup.h |    8 ++++++++
 kernel/cgroup.c        |   22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 9eaa6fe..511cb6a 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -16,6 +16,7 @@
 #include <linux/prio_heap.h>
 #include <linux/rwsem.h>
 #include <linux/idr.h>
+#include <linux/jump_label.h>
 
 #ifdef CONFIG_CGROUPS
 
@@ -526,9 +527,16 @@ struct cgroup_subsys {
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
+#ifdef CONFIG_JUMP_LABEL
+#define SUBSYS(_x)\
+	extern struct jump_label_key cgroup_ ## _x ## _disabled_key;\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return static_branch(&cgroup_ ## _x ## _disabled_key);}
+#else
 #define SUBSYS(_x)\
 	static inline bool cgroup_ ## _x ## _disabled(void){\
 		return _x ## _subsys.disabled;}
+#endif
 #define SUBSYS_UNDEFINED(_x)\
 	static inline bool cgroup_ ## _x ## _disabled(void){\
 		return true;}
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d9d5648..28d4430 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -75,7 +75,7 @@ static DEFINE_MUTEX(cgroup_mutex);
 static struct cgroup_subsys *subsys[CGROUP_SUBSYS_COUNT] = {
 #include <linux/cgroup_subsys.h>
 };
-
+#undef SUBSYS
 #define MAX_CGROUP_ROOT_NAMELEN 64
 
 /*
@@ -4776,6 +4776,25 @@ static void cgroup_release_agent(struct work_struct *work)
 	raw_spin_unlock(&release_list_lock);
 	mutex_unlock(&cgroup_mutex);
 }
+#ifdef CONFIG_JUMP_LABEL
+#define SUBSYS(_x)\
+	struct jump_label_key cgroup_ ## _x ## _disable_key;
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+
+static void cgroup_subsys_disable(void)
+{
+#define SUBSYS(_x)\
+	if ( _x ## _subsys.disabled)\
+		jump_label_inc(&cgroup_ ## _x ## _disable_key);
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+}
+#else
+static void cgroup_subsys_disable(void)
+{
+}
+#endif
 
 static int __init cgroup_disable(char *str)
 {
@@ -4800,6 +4819,7 @@ static int __init cgroup_disable(char *str)
 			}
 		}
 	}
+	cgroup_subsys_disable();
 	return 1;
 }
 __setup("cgroup_disable=", cgroup_disable);
-- 
1.7.4.1


--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH 2/3] use static_branch for cgroup_xxxx_disabled

[KAMEZAWA Hiroyuki]

>From 7eae9a2735bf8df8a7e12f10dd7c27e729760196 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Wed, 23 Nov 2011 15:53:18 +0900
Subject: [PATCH 2/3] use static_branch for cgroup_xxx_disabled

The behavior of cgroup_xxx_disable is determined at boot.
Then, using static_brach() will allow us binary patching rather
than checking global variable.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/cgroup.h |    8 ++++++++
 kernel/cgroup.c        |   22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 9eaa6fe..511cb6a 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -16,6 +16,7 @@
 #include <linux/prio_heap.h>
 #include <linux/rwsem.h>
 #include <linux/idr.h>
+#include <linux/jump_label.h>
 
 #ifdef CONFIG_CGROUPS
 
@@ -526,9 +527,16 @@ struct cgroup_subsys {
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
+#ifdef CONFIG_JUMP_LABEL
+#define SUBSYS(_x)\
+	extern struct jump_label_key cgroup_ ## _x ## _disabled_key;\
+	static inline bool cgroup_ ## _x ## _disabled(void){\
+		return static_branch(&cgroup_ ## _x ## _disabled_key);}
+#else
 #define SUBSYS(_x)\
 	static inline bool cgroup_ ## _x ## _disabled(void){\
 		return _x ## _subsys.disabled;}
+#endif
 #define SUBSYS_UNDEFINED(_x)\
 	static inline bool cgroup_ ## _x ## _disabled(void){\
 		return true;}
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d9d5648..28d4430 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -75,7 +75,7 @@ static DEFINE_MUTEX(cgroup_mutex);
 static struct cgroup_subsys *subsys[CGROUP_SUBSYS_COUNT] = {
 #include <linux/cgroup_subsys.h>
 };
-
+#undef SUBSYS
 #define MAX_CGROUP_ROOT_NAMELEN 64
 
 /*
@@ -4776,6 +4776,25 @@ static void cgroup_release_agent(struct work_struct *work)
 	raw_spin_unlock(&release_list_lock);
 	mutex_unlock(&cgroup_mutex);
 }
+#ifdef CONFIG_JUMP_LABEL
+#define SUBSYS(_x)\
+	struct jump_label_key cgroup_ ## _x ## _disable_key;
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+
+static void cgroup_subsys_disable(void)
+{
+#define SUBSYS(_x)\
+	if ( _x ## _subsys.disabled)\
+		jump_label_inc(&cgroup_ ## _x ## _disable_key);
+#include <linux/cgroup_subsys.h>
+#undef SUBSYS
+}
+#else
+static void cgroup_subsys_disable(void)
+{
+}
+#endif
 
 static int __init cgroup_disable(char *str)
 {
@@ -4800,6 +4819,7 @@ static int __init cgroup_disable(char *str)
 			}
 		}
 	}
+	cgroup_subsys_disable();
 	return 1;
 }
 __setup("cgroup_disable=", cgroup_disable);
-- 
1.7.4.1

[RFC][PATCH 3/3] replace mem_cgroup_disabled

[KAMEZAWA Hiroyuki]

I'll rebase this onto mmotm this is based on mainline git tree.
==
From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Date: Wed, 23 Nov 2011 16:42:59 +0900
Subject: [PATCH 3/3] replace mem_cgroup_disabled().

cgroup provires cgroup_xxxx_disabled() functions for checking
subsys is diabled by boot option or not. Make use of it instead
of using private function.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
---
 include/linux/memcontrol.h |   12 ------------
 kernel/cgroup.c            |    4 ++--
 mm/memcontrol.c            |   32 ++++++++++++++++----------------
 mm/page_cgroup.c           |    4 ++--
 4 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index b87068a..fa5712e 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -124,13 +124,6 @@ extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
 extern int do_swap_account;
 #endif
 
-static inline bool mem_cgroup_disabled(void)
-{
-	if (mem_cgroup_subsys.disabled)
-		return true;
-	return false;
-}
-
 void mem_cgroup_update_page_stat(struct page *page,
 				 enum mem_cgroup_page_stat_item idx,
 				 int val);
@@ -291,11 +284,6 @@ static inline void mem_cgroup_record_reclaim_priority(struct mem_cgroup *memcg,
 {
 }
 
-static inline bool mem_cgroup_disabled(void)
-{
-	return true;
-}
-
 static inline int
 mem_cgroup_inactive_anon_is_low(struct mem_cgroup *memcg, struct zone *zone)
 {
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 28d4430..e5c33f5 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
 }
 #ifdef CONFIG_JUMP_LABEL
 #define SUBSYS(_x)\
-	struct jump_label_key cgroup_ ## _x ## _disable_key;
+	struct jump_label_key cgroup_ ## _x ## _disabled_key;
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
@@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
 {
 #define SUBSYS(_x)\
 	if ( _x ## _subsys.disabled)\
-		jump_label_inc(&cgroup_ ## _x ## _disable_key);
+		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..594af98 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	pc = lookup_page_cgroup(page);
 	/* can happen while we handle swapcache. */
@@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
 	struct page_cgroup *pc;
 	enum lru_list lru = page_lru(page);
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	pc = lookup_page_cgroup(page);
@@ -972,7 +972,7 @@ void mem_cgroup_rotate_lru_list(struct page *page, enum lru_list lru)
 	struct mem_cgroup_per_zone *mz;
 	struct page_cgroup *pc;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	pc = lookup_page_cgroup(page);
@@ -992,7 +992,7 @@ void mem_cgroup_add_lru_list(struct page *page, enum lru_list lru)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	pc = lookup_page_cgroup(page);
 	VM_BUG_ON(PageCgroupAcctLRU(pc));
@@ -1081,7 +1081,7 @@ static void mem_cgroup_lru_add_after_commit(struct page *page)
 void mem_cgroup_move_lists(struct page *page,
 			   enum lru_list from, enum lru_list to)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	mem_cgroup_del_lru_list(page, from);
 	mem_cgroup_add_lru_list(page, to);
@@ -1180,7 +1180,7 @@ mem_cgroup_get_reclaim_stat_from_page(struct page *page)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return NULL;
 
 	pc = lookup_page_cgroup(page);
@@ -2530,7 +2530,7 @@ void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail)
 	struct page_cgroup *tail_pc = lookup_page_cgroup(tail);
 	unsigned long flags;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	/*
 	 * We have no races with charge/uncharge but will have races with
@@ -2730,7 +2730,7 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
 int mem_cgroup_newpage_charge(struct page *page,
 			      struct mm_struct *mm, gfp_t gfp_mask)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 	/*
 	 * If already mapped, we don't have to account.
@@ -2773,7 +2773,7 @@ int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
 	struct mem_cgroup *memcg = NULL;
 	int ret;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 	if (PageCompound(page))
 		return 0;
@@ -2823,7 +2823,7 @@ int mem_cgroup_try_charge_swapin(struct mm_struct *mm,
 
 	*ptr = NULL;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 
 	if (!do_swap_account)
@@ -2853,7 +2853,7 @@ static void
 __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *ptr,
 					enum charge_type ctype)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	if (!ptr)
 		return;
@@ -2903,7 +2903,7 @@ void mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *ptr)
 
 void mem_cgroup_cancel_charge_swapin(struct mem_cgroup *memcg)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	if (!memcg)
 		return;
@@ -2974,7 +2974,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
 	unsigned int nr_pages = 1;
 	struct page_cgroup *pc;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return NULL;
 
 	if (PageSwapCache(page))
@@ -3237,7 +3237,7 @@ int mem_cgroup_prepare_migration(struct page *page,
 	*ptr = NULL;
 
 	VM_BUG_ON(PageTransHuge(page));
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 
 	pc = lookup_page_cgroup(page);
@@ -3379,7 +3379,7 @@ static struct page_cgroup *lookup_page_cgroup_used(struct page *page)
 
 bool mem_cgroup_bad_page_check(struct page *page)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return false;
 
 	return lookup_page_cgroup_used(page) != NULL;
@@ -4853,7 +4853,7 @@ static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg)
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 static void __init enable_swap_cgroup(void)
 {
-	if (!mem_cgroup_disabled() && really_do_swap_account)
+	if (!cgroup_mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
index 2d123f9..a7bf01d 100644
--- a/mm/page_cgroup.c
+++ b/mm/page_cgroup.c
@@ -87,7 +87,7 @@ void __init page_cgroup_init_flatmem(void)
 
 	int nid, fail;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	for_each_online_node(nid)  {
@@ -302,7 +302,7 @@ void __init page_cgroup_init(void)
 	unsigned long pfn;
 	int nid;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	for_each_node_state(nid, N_HIGH_MEMORY) {
-- 
1.7.4.1


--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH 3/3] replace mem_cgroup_disabled

[KAMEZAWA Hiroyuki]

I'll rebase this onto mmotm this is based on mainline git tree.
==
>From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Wed, 23 Nov 2011 16:42:59 +0900
Subject: [PATCH 3/3] replace mem_cgroup_disabled().

cgroup provires cgroup_xxxx_disabled() functions for checking
subsys is diabled by boot option or not. Make use of it instead
of using private function.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 include/linux/memcontrol.h |   12 ------------
 kernel/cgroup.c            |    4 ++--
 mm/memcontrol.c            |   32 ++++++++++++++++----------------
 mm/page_cgroup.c           |    4 ++--
 4 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index b87068a..fa5712e 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -124,13 +124,6 @@ extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
 extern int do_swap_account;
 #endif
 
-static inline bool mem_cgroup_disabled(void)
-{
-	if (mem_cgroup_subsys.disabled)
-		return true;
-	return false;
-}
-
 void mem_cgroup_update_page_stat(struct page *page,
 				 enum mem_cgroup_page_stat_item idx,
 				 int val);
@@ -291,11 +284,6 @@ static inline void mem_cgroup_record_reclaim_priority(struct mem_cgroup *memcg,
 {
 }
 
-static inline bool mem_cgroup_disabled(void)
-{
-	return true;
-}
-
 static inline int
 mem_cgroup_inactive_anon_is_low(struct mem_cgroup *memcg, struct zone *zone)
 {
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 28d4430..e5c33f5 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
 }
 #ifdef CONFIG_JUMP_LABEL
 #define SUBSYS(_x)\
-	struct jump_label_key cgroup_ ## _x ## _disable_key;
+	struct jump_label_key cgroup_ ## _x ## _disabled_key;
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 
@@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
 {
 #define SUBSYS(_x)\
 	if ( _x ## _subsys.disabled)\
-		jump_label_inc(&cgroup_ ## _x ## _disable_key);
+		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
 #include <linux/cgroup_subsys.h>
 #undef SUBSYS
 }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..594af98 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	pc = lookup_page_cgroup(page);
 	/* can happen while we handle swapcache. */
@@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
 	struct page_cgroup *pc;
 	enum lru_list lru = page_lru(page);
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	pc = lookup_page_cgroup(page);
@@ -972,7 +972,7 @@ void mem_cgroup_rotate_lru_list(struct page *page, enum lru_list lru)
 	struct mem_cgroup_per_zone *mz;
 	struct page_cgroup *pc;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	pc = lookup_page_cgroup(page);
@@ -992,7 +992,7 @@ void mem_cgroup_add_lru_list(struct page *page, enum lru_list lru)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	pc = lookup_page_cgroup(page);
 	VM_BUG_ON(PageCgroupAcctLRU(pc));
@@ -1081,7 +1081,7 @@ static void mem_cgroup_lru_add_after_commit(struct page *page)
 void mem_cgroup_move_lists(struct page *page,
 			   enum lru_list from, enum lru_list to)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	mem_cgroup_del_lru_list(page, from);
 	mem_cgroup_add_lru_list(page, to);
@@ -1180,7 +1180,7 @@ mem_cgroup_get_reclaim_stat_from_page(struct page *page)
 	struct page_cgroup *pc;
 	struct mem_cgroup_per_zone *mz;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return NULL;
 
 	pc = lookup_page_cgroup(page);
@@ -2530,7 +2530,7 @@ void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail)
 	struct page_cgroup *tail_pc = lookup_page_cgroup(tail);
 	unsigned long flags;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	/*
 	 * We have no races with charge/uncharge but will have races with
@@ -2730,7 +2730,7 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
 int mem_cgroup_newpage_charge(struct page *page,
 			      struct mm_struct *mm, gfp_t gfp_mask)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 	/*
 	 * If already mapped, we don't have to account.
@@ -2773,7 +2773,7 @@ int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
 	struct mem_cgroup *memcg = NULL;
 	int ret;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 	if (PageCompound(page))
 		return 0;
@@ -2823,7 +2823,7 @@ int mem_cgroup_try_charge_swapin(struct mm_struct *mm,
 
 	*ptr = NULL;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 
 	if (!do_swap_account)
@@ -2853,7 +2853,7 @@ static void
 __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *ptr,
 					enum charge_type ctype)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	if (!ptr)
 		return;
@@ -2903,7 +2903,7 @@ void mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *ptr)
 
 void mem_cgroup_cancel_charge_swapin(struct mem_cgroup *memcg)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 	if (!memcg)
 		return;
@@ -2974,7 +2974,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
 	unsigned int nr_pages = 1;
 	struct page_cgroup *pc;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return NULL;
 
 	if (PageSwapCache(page))
@@ -3237,7 +3237,7 @@ int mem_cgroup_prepare_migration(struct page *page,
 	*ptr = NULL;
 
 	VM_BUG_ON(PageTransHuge(page));
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return 0;
 
 	pc = lookup_page_cgroup(page);
@@ -3379,7 +3379,7 @@ static struct page_cgroup *lookup_page_cgroup_used(struct page *page)
 
 bool mem_cgroup_bad_page_check(struct page *page)
 {
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return false;
 
 	return lookup_page_cgroup_used(page) != NULL;
@@ -4853,7 +4853,7 @@ static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg)
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 static void __init enable_swap_cgroup(void)
 {
-	if (!mem_cgroup_disabled() && really_do_swap_account)
+	if (!cgroup_mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
index 2d123f9..a7bf01d 100644
--- a/mm/page_cgroup.c
+++ b/mm/page_cgroup.c
@@ -87,7 +87,7 @@ void __init page_cgroup_init_flatmem(void)
 
 	int nid, fail;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	for_each_online_node(nid)  {
@@ -302,7 +302,7 @@ void __init page_cgroup_init(void)
 	unsigned long pfn;
 	int nid;
 
-	if (mem_cgroup_disabled())
+	if (cgroup_mem_cgroup_disabled())
 		return;
 
 	for_each_node_state(nid, N_HIGH_MEMORY) {
-- 
1.7.4.1

Re: [RFC][PATCH 3/3] replace mem_cgroup_disabled

[Glauber Costa]

On 11/23/2011 06:34 AM, KAMEZAWA Hiroyuki wrote:
> I'll rebase this onto mmotm this is based on mainline git tree.
> ==
>  From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 23 Nov 2011 16:42:59 +0900
> Subject: [PATCH 3/3] replace mem_cgroup_disabled().
>
> cgroup provires cgroup_xxxx_disabled() functions for checking
> subsys is diabled by boot option or not. Make use of it instead
> of using private function.
>
> Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> ---
>   include/linux/memcontrol.h |   12 ------------
>   kernel/cgroup.c            |    4 ++--
>   mm/memcontrol.c            |   32 ++++++++++++++++----------------
>   mm/page_cgroup.c           |    4 ++--
>   4 files changed, 20 insertions(+), 32 deletions(-)
>

> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 28d4430..e5c33f5 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
>   }
>   #ifdef CONFIG_JUMP_LABEL
>   #define SUBSYS(_x)\
> -	struct jump_label_key cgroup_ ## _x ## _disable_key;
> +	struct jump_label_key cgroup_ ## _x ## _disabled_key;
>   #include<linux/cgroup_subsys.h>
>   #undef SUBSYS

I have the impression this is just churn. Can you call it disabled_key
from the beginning ?

> @@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
>   {
>   #define SUBSYS(_x)\
>   	if ( _x ## _subsys.disabled)\
> -		jump_label_inc(&cgroup_ ## _x ## _disable_key);
> +		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
>   #include<linux/cgroup_subsys.h>
>   #undef SUBSYS
>   }
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6aff93c..594af98 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
>   	struct page_cgroup *pc;
>   	struct mem_cgroup_per_zone *mz;
>
> -	if (mem_cgroup_disabled())
> +	if (cgroup_mem_cgroup_disabled())
>   		return;
>   	pc = lookup_page_cgroup(page);
>   	/* can happen while we handle swapcache. */
> @@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
>   	struct page_cgroup *pc;
>   	enum lru_list lru = page_lru(page);
>
> -	if (mem_cgroup_disabled())
> +	if (cgroup_mem_cgroup_disabled())
>   		return;
>
>   	pc = lookup_page_cgroup(page);

In many cases, this will be just a useless function call. Wouldn't it be 
better if in the disabled case we would not even call those functions? 
It may help some fast paths.

We could define inline versions in memcontrol.h in place of the function 
signatures themselves, and have those to test for the static branch.

Maybe a macro can be used to make it less laborious?

Re: [RFC][PATCH 3/3] replace mem_cgroup_disabled

[Glauber Costa]

On 11/23/2011 06:34 AM, KAMEZAWA Hiroyuki wrote:
> I'll rebase this onto mmotm this is based on mainline git tree.
> ==
>  From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> Date: Wed, 23 Nov 2011 16:42:59 +0900
> Subject: [PATCH 3/3] replace mem_cgroup_disabled().
>
> cgroup provires cgroup_xxxx_disabled() functions for checking
> subsys is diabled by boot option or not. Make use of it instead
> of using private function.
>
> Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> ---
>   include/linux/memcontrol.h |   12 ------------
>   kernel/cgroup.c            |    4 ++--
>   mm/memcontrol.c            |   32 ++++++++++++++++----------------
>   mm/page_cgroup.c           |    4 ++--
>   4 files changed, 20 insertions(+), 32 deletions(-)
>

> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 28d4430..e5c33f5 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
>   }
>   #ifdef CONFIG_JUMP_LABEL
>   #define SUBSYS(_x)\
> -	struct jump_label_key cgroup_ ## _x ## _disable_key;
> +	struct jump_label_key cgroup_ ## _x ## _disabled_key;
>   #include<linux/cgroup_subsys.h>
>   #undef SUBSYS

I have the impression this is just churn. Can you call it disabled_key
from the beginning ?

> @@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
>   {
>   #define SUBSYS(_x)\
>   	if ( _x ## _subsys.disabled)\
> -		jump_label_inc(&cgroup_ ## _x ## _disable_key);
> +		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
>   #include<linux/cgroup_subsys.h>
>   #undef SUBSYS
>   }
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6aff93c..594af98 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
>   	struct page_cgroup *pc;
>   	struct mem_cgroup_per_zone *mz;
>
> -	if (mem_cgroup_disabled())
> +	if (cgroup_mem_cgroup_disabled())
>   		return;
>   	pc = lookup_page_cgroup(page);
>   	/* can happen while we handle swapcache. */
> @@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
>   	struct page_cgroup *pc;
>   	enum lru_list lru = page_lru(page);
>
> -	if (mem_cgroup_disabled())
> +	if (cgroup_mem_cgroup_disabled())
>   		return;
>
>   	pc = lookup_page_cgroup(page);

In many cases, this will be just a useless function call. Wouldn't it be 
better if in the disabled case we would not even call those functions? 
It may help some fast paths.

We could define inline versions in memcontrol.h in place of the function 
signatures themselves, and have those to test for the static branch.

Maybe a macro can be used to make it less laborious?

Re: [RFC][PATCH 3/3] replace mem_cgroup_disabled

[KAMEZAWA Hiroyuki]

On Wed, 23 Nov 2011 08:43:15 -0200
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> On 11/23/2011 06:34 AM, KAMEZAWA Hiroyuki wrote:
> > I'll rebase this onto mmotm this is based on mainline git tree.
> > ==
> >  From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
> > Date: Wed, 23 Nov 2011 16:42:59 +0900
> > Subject: [PATCH 3/3] replace mem_cgroup_disabled().
> >
> > cgroup provires cgroup_xxxx_disabled() functions for checking
> > subsys is diabled by boot option or not. Make use of it instead
> > of using private function.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
> > ---
> >   include/linux/memcontrol.h |   12 ------------
> >   kernel/cgroup.c            |    4 ++--
> >   mm/memcontrol.c            |   32 ++++++++++++++++----------------
> >   mm/page_cgroup.c           |    4 ++--
> >   4 files changed, 20 insertions(+), 32 deletions(-)
> >
> 
> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 28d4430..e5c33f5 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
> >   }
> >   #ifdef CONFIG_JUMP_LABEL
> >   #define SUBSYS(_x)\
> > -	struct jump_label_key cgroup_ ## _x ## _disable_key;
> > +	struct jump_label_key cgroup_ ## _x ## _disabled_key;
> >   #include<linux/cgroup_subsys.h>
> >   #undef SUBSYS
> 
> I have the impression this is just churn. Can you call it disabled_key
> from the beginning ?
> 

Ouch, I made a mistake at commiting and fix to previous commit sneaked
into here. Thank you for review, I'll do fix.


> > @@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
> >   {
> >   #define SUBSYS(_x)\
> >   	if ( _x ## _subsys.disabled)\
> > -		jump_label_inc(&cgroup_ ## _x ## _disable_key);
> > +		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
> >   #include<linux/cgroup_subsys.h>
> >   #undef SUBSYS
> >   }
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 6aff93c..594af98 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
> >   	struct page_cgroup *pc;
> >   	struct mem_cgroup_per_zone *mz;
> >
> > -	if (mem_cgroup_disabled())
> > +	if (cgroup_mem_cgroup_disabled())
> >   		return;
> >   	pc = lookup_page_cgroup(page);
> >   	/* can happen while we handle swapcache. */
> > @@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
> >   	struct page_cgroup *pc;
> >   	enum lru_list lru = page_lru(page);
> >
> > -	if (mem_cgroup_disabled())
> > +	if (cgroup_mem_cgroup_disabled())
> >   		return;
> >
> >   	pc = lookup_page_cgroup(page);
> 
> In many cases, this will be just a useless function call. Wouldn't it be 
> better if in the disabled case we would not even call those functions? 
> It may help some fast paths.
> 
Hm ? diassembled code is like this.

Dump of assembler code for function mem_cgroup_rotate_reclaimable_page:
   0xffffffff8116c460 <+0>:     push   %rbp
   0xffffffff8116c461 <+1>:     mov    %rsp,%rbp
   0xffffffff8116c464 <+4>:     sub    $0x10,%rsp
   0xffffffff8116c468 <+8>:     mov    %rbx,(%rsp)
   0xffffffff8116c46c <+12>:    mov    %r12,0x8(%rsp)
   0xffffffff8116c471 <+17>:    callq  0xffffffff815c59c0
   0xffffffff8116c476 <+22>:    mov    (%rdi),%rax
   0xffffffff8116c479 <+25>:    mov    $0x4,%ebx
   0xffffffff8116c47e <+30>:    mov    %rdi,%r12
   0xffffffff8116c481 <+33>:    test   $0x100000,%eax
   0xffffffff8116c486 <+38>:    jne    0xffffffff8116c4a6 <mem_cgroup_rotate_reclaimable_page+70>
   0xffffffff8116c488 <+40>:    mov    (%rdi),%rax
   0xffffffff8116c48b <+43>:    and    $0x80000,%eax
   0xffffffff8116c490 <+48>:    cmp    $0x1,%rax
   0xffffffff8116c494 <+52>:    mov    (%rdi),%rax
   0xffffffff8116c497 <+55>:    sbb    %ebx,%ebx
   0xffffffff8116c499 <+57>:    and    $0x2,%ebx
   0xffffffff8116c49c <+60>:    and    $0x40,%eax
   0xffffffff8116c49f <+63>:    cmp    $0x1,%rax
   0xffffffff8116c4a3 <+67>:    sbb    $0xffffffffffffffff,%ebx
   0xffffffff8116c4a6 <+70>:    jmpq   0xffffffff8116c4ab <mem_cgroup_rotate_reclaimable_page+75>
   0xffffffff8116c4ab <+75>:    mov    %r12,%rdi
   0xffffffff8116c4ae <+78>:    callq  0xffffffff81173740 <lookup_page_cgroup>

0xffffffff8116c4a6  is the jump label I used.

jump labels in static inline functions seems to work as expected.
and no function calls. This seems good.

Thanks,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 3/3] replace mem_cgroup_disabled

[KAMEZAWA Hiroyuki]

On Wed, 23 Nov 2011 08:43:15 -0200
Glauber Costa <glommer@parallels.com> wrote:

> On 11/23/2011 06:34 AM, KAMEZAWA Hiroyuki wrote:
> > I'll rebase this onto mmotm this is based on mainline git tree.
> > ==
> >  From 2da35fd8eab3a8c2ca80d7aa5dfd4276a23ebf57 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Wed, 23 Nov 2011 16:42:59 +0900
> > Subject: [PATCH 3/3] replace mem_cgroup_disabled().
> >
> > cgroup provires cgroup_xxxx_disabled() functions for checking
> > subsys is diabled by boot option or not. Make use of it instead
> > of using private function.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> >   include/linux/memcontrol.h |   12 ------------
> >   kernel/cgroup.c            |    4 ++--
> >   mm/memcontrol.c            |   32 ++++++++++++++++----------------
> >   mm/page_cgroup.c           |    4 ++--
> >   4 files changed, 20 insertions(+), 32 deletions(-)
> >
> 
> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 28d4430..e5c33f5 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -4778,7 +4778,7 @@ static void cgroup_release_agent(struct work_struct *work)
> >   }
> >   #ifdef CONFIG_JUMP_LABEL
> >   #define SUBSYS(_x)\
> > -	struct jump_label_key cgroup_ ## _x ## _disable_key;
> > +	struct jump_label_key cgroup_ ## _x ## _disabled_key;
> >   #include<linux/cgroup_subsys.h>
> >   #undef SUBSYS
> 
> I have the impression this is just churn. Can you call it disabled_key
> from the beginning ?
> 

Ouch, I made a mistake at commiting and fix to previous commit sneaked
into here. Thank you for review, I'll do fix.


> > @@ -4786,7 +4786,7 @@ static void cgroup_subsys_disable(void)
> >   {
> >   #define SUBSYS(_x)\
> >   	if ( _x ## _subsys.disabled)\
> > -		jump_label_inc(&cgroup_ ## _x ## _disable_key);
> > +		jump_label_inc(&cgroup_ ## _x ## _disabled_key);
> >   #include<linux/cgroup_subsys.h>
> >   #undef SUBSYS
> >   }
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 6aff93c..594af98 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -916,7 +916,7 @@ void mem_cgroup_del_lru_list(struct page *page, enum lru_list lru)
> >   	struct page_cgroup *pc;
> >   	struct mem_cgroup_per_zone *mz;
> >
> > -	if (mem_cgroup_disabled())
> > +	if (cgroup_mem_cgroup_disabled())
> >   		return;
> >   	pc = lookup_page_cgroup(page);
> >   	/* can happen while we handle swapcache. */
> > @@ -952,7 +952,7 @@ void mem_cgroup_rotate_reclaimable_page(struct page *page)
> >   	struct page_cgroup *pc;
> >   	enum lru_list lru = page_lru(page);
> >
> > -	if (mem_cgroup_disabled())
> > +	if (cgroup_mem_cgroup_disabled())
> >   		return;
> >
> >   	pc = lookup_page_cgroup(page);
> 
> In many cases, this will be just a useless function call. Wouldn't it be 
> better if in the disabled case we would not even call those functions? 
> It may help some fast paths.
> 
Hm ? diassembled code is like this.

Dump of assembler code for function mem_cgroup_rotate_reclaimable_page:
   0xffffffff8116c460 <+0>:     push   %rbp
   0xffffffff8116c461 <+1>:     mov    %rsp,%rbp
   0xffffffff8116c464 <+4>:     sub    $0x10,%rsp
   0xffffffff8116c468 <+8>:     mov    %rbx,(%rsp)
   0xffffffff8116c46c <+12>:    mov    %r12,0x8(%rsp)
   0xffffffff8116c471 <+17>:    callq  0xffffffff815c59c0
   0xffffffff8116c476 <+22>:    mov    (%rdi),%rax
   0xffffffff8116c479 <+25>:    mov    $0x4,%ebx
   0xffffffff8116c47e <+30>:    mov    %rdi,%r12
   0xffffffff8116c481 <+33>:    test   $0x100000,%eax
   0xffffffff8116c486 <+38>:    jne    0xffffffff8116c4a6 <mem_cgroup_rotate_reclaimable_page+70>
   0xffffffff8116c488 <+40>:    mov    (%rdi),%rax
   0xffffffff8116c48b <+43>:    and    $0x80000,%eax
   0xffffffff8116c490 <+48>:    cmp    $0x1,%rax
   0xffffffff8116c494 <+52>:    mov    (%rdi),%rax
   0xffffffff8116c497 <+55>:    sbb    %ebx,%ebx
   0xffffffff8116c499 <+57>:    and    $0x2,%ebx
   0xffffffff8116c49c <+60>:    and    $0x40,%eax
   0xffffffff8116c49f <+63>:    cmp    $0x1,%rax
   0xffffffff8116c4a3 <+67>:    sbb    $0xffffffffffffffff,%ebx
   0xffffffff8116c4a6 <+70>:    jmpq   0xffffffff8116c4ab <mem_cgroup_rotate_reclaimable_page+75>
   0xffffffff8116c4ab <+75>:    mov    %r12,%rdi
   0xffffffff8116c4ae <+78>:    callq  0xffffffff81173740 <lookup_page_cgroup>

0xffffffff8116c4a6  is the jump label I used.

jump labels in static inline functions seems to work as expected.
and no function calls. This seems good.

Thanks,
-Kame

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[Glauber Costa]

On 11/23/2011 06:28 AM, KAMEZAWA Hiroyuki wrote:
>
> Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
>
> I made a brief trial to use static_branch() for that function. At doing that,
> I thought it will be better to implement generic cgroup functions rather
> than having memory cgroup's its own one.
>
> This series consists of 3 patches
> 1 .... implement cgroup_xxxx_disabled() in generic.
> 2 .... use jump_label for cgroup_xxxx_disabled()
> 3 .... remove mem_cgroup_disabled() in memcontrol.c
>
> And I post this series for getting review/comments.
> I'm not sure patches for using jump_label is worth to be merged.
>
> I did a test to run a loop
> 	while(-) {
> 		mmap(1M)
> 		touch all pages
> 		munmap()
> 	}
>
> and measured performance score in ROOT cgroup. Here,
>
> (Before patch)
>     182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
>     192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
>         761,483,416 cache-references                                             [49.98%]
>             159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
>      33,253,084,874 branches                                                     [33.34%]
>         109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
>
>        58.289265709 seconds time elapsed
>
> (After patch)
>   Performance counter stats for './malloc 1':
>
>     183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
>     191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
>         798,635,028 cache-references                                             [49.98%]
>              95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
>      32,755,318,286 branches                                                     [33.34%]
>          77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
>
>        58.332356996 seconds time elapsed
>
> There is no differece in 'time' ;)
> But I got an impression that 'branch' score gets better in several tests.
>

branch and cache misses are a lot smaller as well. I think this is a win.

--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[Glauber Costa]

On 11/23/2011 06:28 AM, KAMEZAWA Hiroyuki wrote:
>
> Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
>
> I made a brief trial to use static_branch() for that function. At doing that,
> I thought it will be better to implement generic cgroup functions rather
> than having memory cgroup's its own one.
>
> This series consists of 3 patches
> 1 .... implement cgroup_xxxx_disabled() in generic.
> 2 .... use jump_label for cgroup_xxxx_disabled()
> 3 .... remove mem_cgroup_disabled() in memcontrol.c
>
> And I post this series for getting review/comments.
> I'm not sure patches for using jump_label is worth to be merged.
>
> I did a test to run a loop
> 	while(-) {
> 		mmap(1M)
> 		touch all pages
> 		munmap()
> 	}
>
> and measured performance score in ROOT cgroup. Here,
>
> (Before patch)
>     182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
>     192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
>         761,483,416 cache-references                                             [49.98%]
>             159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
>      33,253,084,874 branches                                                     [33.34%]
>         109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
>
>        58.289265709 seconds time elapsed
>
> (After patch)
>   Performance counter stats for './malloc 1':
>
>     183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
>     191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
>         798,635,028 cache-references                                             [49.98%]
>              95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
>      32,755,318,286 branches                                                     [33.34%]
>          77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
>
>        58.332356996 seconds time elapsed
>
> There is no differece in 'time' ;)
> But I got an impression that 'branch' score gets better in several tests.
>

branch and cache misses are a lot smaller as well. I think this is a win.

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[Li Zefan]

KAMEZAWA Hiroyuki wrote:
> 
> Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
> 
> I made a brief trial to use static_branch() for that function. At doing that,
> I thought it will be better to implement generic cgroup functions rather
> than having memory cgroup's its own one.
> 
> This series consists of 3 patches
> 1 .... implement cgroup_xxxx_disabled() in generic.
> 2 .... use jump_label for cgroup_xxxx_disabled()
> 3 .... remove mem_cgroup_disabled() in memcontrol.c
> 
> And I post this series for getting review/comments.
> I'm not sure patches for using jump_label is worth to be merged.
> 
> I did a test to run a loop
> 	while(-) {
> 		mmap(1M)
> 		touch all pages
> 		munmap()
> 	}
> 
> and measured performance score in ROOT cgroup. Here,
> 
> (Before patch)
>    182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
>    192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
>        761,483,416 cache-references                                             [49.98%]
>            159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
>     33,253,084,874 branches                                                     [33.34%]
>        109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
> 
>       58.289265709 seconds time elapsed
> 
> (After patch)
>  Performance counter stats for './malloc 1':
> 
>    183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
>    191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
>        798,635,028 cache-references                                             [49.98%]
>             95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
>     32,755,318,286 branches                                                     [33.34%]
>         77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
> 
>       58.332356996 seconds time elapsed
> 
> There is no differece in 'time' ;) 
> But I got an impression that 'branch' score gets better in several tests.
> 
> Thanks,
> -Kame
> 
> P.S. maybe I can replace 'do_swap_account' with jump_label, too.
> 

The numbers sugguest using jump label is a win.

However I'm not quite convinced that we make it generic. The subsys.disabled flag
was introduced long ago for memcg, but yet it has no other users.

So maybe for now make use of jump label in memcg only? We probably still needs
a bit help from cgroup core, to provide a subsys->disable() callback.

--
Li Zefan
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[Li Zefan]

KAMEZAWA Hiroyuki wrote:
> 
> Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
> 
> I made a brief trial to use static_branch() for that function. At doing that,
> I thought it will be better to implement generic cgroup functions rather
> than having memory cgroup's its own one.
> 
> This series consists of 3 patches
> 1 .... implement cgroup_xxxx_disabled() in generic.
> 2 .... use jump_label for cgroup_xxxx_disabled()
> 3 .... remove mem_cgroup_disabled() in memcontrol.c
> 
> And I post this series for getting review/comments.
> I'm not sure patches for using jump_label is worth to be merged.
> 
> I did a test to run a loop
> 	while(-) {
> 		mmap(1M)
> 		touch all pages
> 		munmap()
> 	}
> 
> and measured performance score in ROOT cgroup. Here,
> 
> (Before patch)
>    182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
>    192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
>        761,483,416 cache-references                                             [49.98%]
>            159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
>     33,253,084,874 branches                                                     [33.34%]
>        109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
> 
>       58.289265709 seconds time elapsed
> 
> (After patch)
>  Performance counter stats for './malloc 1':
> 
>    183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
>    191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
>        798,635,028 cache-references                                             [49.98%]
>             95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
>     32,755,318,286 branches                                                     [33.34%]
>         77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
> 
>       58.332356996 seconds time elapsed
> 
> There is no differece in 'time' ;) 
> But I got an impression that 'branch' score gets better in several tests.
> 
> Thanks,
> -Kame
> 
> P.S. maybe I can replace 'do_swap_account' with jump_label, too.
> 

The numbers sugguest using jump label is a win.

However I'm not quite convinced that we make it generic. The subsys.disabled flag
was introduced long ago for memcg, but yet it has no other users.

So maybe for now make use of jump label in memcg only? We probably still needs
a bit help from cgroup core, to provide a subsys->disable() callback.

--
Li Zefan

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[KAMEZAWA Hiroyuki]

On Thu, 24 Nov 2011 10:22:13 +0800
Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:

> KAMEZAWA Hiroyuki wrote:
> > 
> > Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
> > 
> > I made a brief trial to use static_branch() for that function. At doing that,
> > I thought it will be better to implement generic cgroup functions rather
> > than having memory cgroup's its own one.
> > 
> > This series consists of 3 patches
> > 1 .... implement cgroup_xxxx_disabled() in generic.
> > 2 .... use jump_label for cgroup_xxxx_disabled()
> > 3 .... remove mem_cgroup_disabled() in memcontrol.c
> > 
> > And I post this series for getting review/comments.
> > I'm not sure patches for using jump_label is worth to be merged.
> > 
> > I did a test to run a loop
> > 	while(-) {
> > 		mmap(1M)
> > 		touch all pages
> > 		munmap()
> > 	}
> > 
> > and measured performance score in ROOT cgroup. Here,
> > 
> > (Before patch)
> >    182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
> >    192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
> >        761,483,416 cache-references                                             [49.98%]
> >            159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
> >     33,253,084,874 branches                                                     [33.34%]
> >        109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
> > 
> >       58.289265709 seconds time elapsed
> > 
> > (After patch)
> >  Performance counter stats for './malloc 1':
> > 
> >    183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
> >    191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
> >        798,635,028 cache-references                                             [49.98%]
> >             95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
> >     32,755,318,286 branches                                                     [33.34%]
> >         77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
> > 
> >       58.332356996 seconds time elapsed
> > 
> > There is no differece in 'time' ;) 
> > But I got an impression that 'branch' score gets better in several tests.
> > 
> > Thanks,
> > -Kame
> > 
> > P.S. maybe I can replace 'do_swap_account' with jump_label, too.
> > 
> 
> The numbers sugguest using jump label is a win.
> 
> However I'm not quite convinced that we make it generic. The subsys.disabled flag
> was introduced long ago for memcg, but yet it has no other users.
> 

We have generic cgroup_disable=xxxx boot options. But yes, it seems only memory cgroup
checks it in the hooks. (But I'm not sure it's not necessary to be checked..)

> So maybe for now make use of jump label in memcg only? We probably still needs
> a bit help from cgroup core, to provide a subsys->disable() callback.
> 

Hm, ok. if this is not welcomed, I'll keep this only in memory cgroup and don't
touch cgroup.h. 

Thank you for review.

Regards,
-Kame




--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 0/3] impelemnt cgroup_(subsys)_disabled in generic.

[KAMEZAWA Hiroyuki]

On Thu, 24 Nov 2011 10:22:13 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> KAMEZAWA Hiroyuki wrote:
> > 
> > Now, memory cgroup has 'mem_cgroup_disabled()' in memcontrol.h
> > 
> > I made a brief trial to use static_branch() for that function. At doing that,
> > I thought it will be better to implement generic cgroup functions rather
> > than having memory cgroup's its own one.
> > 
> > This series consists of 3 patches
> > 1 .... implement cgroup_xxxx_disabled() in generic.
> > 2 .... use jump_label for cgroup_xxxx_disabled()
> > 3 .... remove mem_cgroup_disabled() in memcontrol.c
> > 
> > And I post this series for getting review/comments.
> > I'm not sure patches for using jump_label is worth to be merged.
> > 
> > I did a test to run a loop
> > 	while(-) {
> > 		mmap(1M)
> > 		touch all pages
> > 		munmap()
> > 	}
> > 
> > and measured performance score in ROOT cgroup. Here,
> > 
> > (Before patch)
> >    182,932,842,128 cycles                    #    0.000 GHz                     [33.33%]
> >    192,711,643,877 instructions              #    1.05  insns per cycle         [49.99%]
> >        761,483,416 cache-references                                             [49.98%]
> >            159,908 cache-misses              #    0.021 % of all cache refs     [50.00%]
> >     33,253,084,874 branches                                                     [33.34%]
> >        109,796,792 branch-misses             #    0.33% of all branches         [33.34%]
> > 
> >       58.289265709 seconds time elapsed
> > 
> > (After patch)
> >  Performance counter stats for './malloc 1':
> > 
> >    183,068,407,487 cycles                    #    0.000 GHz                     [33.33%]
> >    191,834,248,678 instructions              #    1.05  insns per cycle         [50.00%]
> >        798,635,028 cache-references                                             [49.98%]
> >             95,562 cache-misses              #    0.012 % of all cache refs     [50.00%]
> >     32,755,318,286 branches                                                     [33.34%]
> >         77,774,624 branch-misses             #    0.24% of all branches         [33.34%]
> > 
> >       58.332356996 seconds time elapsed
> > 
> > There is no differece in 'time' ;) 
> > But I got an impression that 'branch' score gets better in several tests.
> > 
> > Thanks,
> > -Kame
> > 
> > P.S. maybe I can replace 'do_swap_account' with jump_label, too.
> > 
> 
> The numbers sugguest using jump label is a win.
> 
> However I'm not quite convinced that we make it generic. The subsys.disabled flag
> was introduced long ago for memcg, but yet it has no other users.
> 

We have generic cgroup_disable=xxxx boot options. But yes, it seems only memory cgroup
checks it in the hooks. (But I'm not sure it's not necessary to be checked..)

> So maybe for now make use of jump label in memcg only? We probably still needs
> a bit help from cgroup core, to provide a subsys->disable() callback.
> 

Hm, ok. if this is not welcomed, I'll keep this only in memory cgroup and don't
touch cgroup.h. 

Thank you for review.

Regards,
-Kame