[PATCH v2 0/5] cgroup cls & netprio 'cleanups'

STALE5144d

4 messages, 1 author, 2012-07-11 · open the first message on its own page

[PATCH v2 0/5] cgroup cls & netprio 'cleanups'

From: Daniel Wagner <hidden>
Date: 2012-07-11 09:35:43

From: Daniel Wagner <redacted>

Hi,

While looking a bit at the cls and netprio controller I found a
few things which I think are 'not optimal'.

The first patch makes sock_update_netprioidx() a bit smatter, so that
only on changed value the sk->sk_cgroup_prioidx is written and not
all the time.

The next two patches are changing the built. In case someone 
builds a kernel with cgroup support but disabled cls or netprio
controller, there was still code added.

And the last two patches change the cls and netprio source files in
that way, that the use IS_BUILTIN and IS_MODULE as it used in the
header files.

The patches are against net-next.

cheers,
daniel

Daniel Wagner (5):
  cgroup: Only update sk_cgrp_prioidx on change
  cgroup: Do not add sock_update_classid() when CONFIG_NET_CLS_CGROUP=0
  cgroup: Do not add sock_update_netprioidx() when
    CONFIG_NETPRIO_CGROUP=0
  cgroup: Make builtin vs. module build ifdefs consistent
  cgroup: Make builtin vs. module build ifdefs consistent

 include/net/cls_cgroup.h     | 33 +++++++++++++++++++++++----------
 include/net/netprio_cgroup.h | 31 ++++++++++++++-----------------
 include/net/sock.h           |  8 --------
 net/core/netprio_cgroup.c    |  6 +++---
 net/core/sock.c              | 14 +++++++++++---
 net/sched/cls_cgroup.c       |  6 +++---
 6 files changed, 54 insertions(+), 44 deletions(-)

-- 
1.7.11.1.165.g299666c

[PATCH v2 1/5] cgroup: Only update sk_cgrp_prioidx on change

From: Daniel Wagner <hidden>
Date: 2012-07-11 09:35:43

From: Daniel Wagner <redacted>

Do not make the cache line dirty when nothing has changed.
sock_update_classid() does exactly the same thing.

Signed-off-by: Daniel Wagner <redacted>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Cc: Kamezawa Hiroyuki <redacted>
Cc: Glauber Costa <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <redacted>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/core/sock.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 929bdcc..bbab10d 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -1182,10 +1182,14 @@ EXPORT_SYMBOL(sock_update_classid);
 
 void sock_update_netprioidx(struct sock *sk)
 {
+	u32 prioidx;
+
 	if (in_interrupt())
 		return;
 
-	sk->sk_cgrp_prioidx = task_netprioidx(current);
+	prioidx = task_netprioidx(current);
+	if (prioidx != sk->sk_cgrp_prioidx)
+		sk->sk_cgrp_prioidx = prioidx;
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif

-- 
1.7.11.1.165.g299666c

[PATCH v2 3/5] cgroup: Do not add sock_update_netprioidx() when CONFIG_NETPRIO_CGROUP=0

From: Daniel Wagner <hidden>
Date: 2012-07-11 09:35:45

From: Daniel Wagner <redacted>

With the configuration CONFIG_CGROUPS=y and CONFIG_NETPRIO_CGROUP=0
sock_update_netprioidx() was still added and introduced unnecessary load.

Signed-off-by: Daniel Wagner <redacted>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Glauber Costa <redacted>
Cc: Kamezawa Hiroyuki <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <redacted>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 include/net/netprio_cgroup.h | 31 ++++++++++++++-----------------
 net/core/sock.c              |  4 +++-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index d58fdec..d14dfbe 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h

@@ -17,6 +17,7 @@
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 
 struct netprio_map {
 	struct rcu_head rcu;

@@ -24,19 +25,24 @@ struct netprio_map {
 	u32 priomap[];
 };
 
-#ifdef CONFIG_CGROUPS
-
 struct cgroup_netprio_state {
 	struct cgroup_subsys_state css;
 	u32 prioidx;
 };
 
-#ifndef CONFIG_NETPRIO_CGROUP
-extern int net_prio_subsys_id;
-#endif
-
 extern void sock_update_netprioidx(struct sock *sk);
 
+#else
+
+#define sock_update_netprioidx(sk)
+
+static inline u32 task_netprioidx(struct task_struct *p)
+{
+	return 0;
+}
+
+#endif
+
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 
 static inline u32 task_netprioidx(struct task_struct *p)

@@ -54,6 +60,8 @@ static inline u32 task_netprioidx(struct task_struct *p)
 
 #elif IS_MODULE(CONFIG_NETPRIO_CGROUP)
 
+extern int net_prio_subsys_id;
+
 static inline u32 task_netprioidx(struct task_struct *p)
 {
 	struct cgroup_netprio_state *state;

@@ -72,17 +80,6 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	return idx;
 }
 
-#else
-
-static inline u32 task_netprioidx(struct task_struct *p)
-{
-	return 0;
-}
-
 #endif /* CONFIG_NETPRIO_CGROUP */
 
-#else
-#define sock_update_netprioidx(sk)
-#endif
-
 #endif  /* _NET_CLS_CGROUP_H */

diff --git a/net/core/sock.c b/net/core/sock.c
index 52c4a2a..94e0100 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -276,7 +276,7 @@ EXPORT_SYMBOL(sysctl_optmem_max);
 int net_cls_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_cls_subsys_id);
 #endif
-#if !defined(CONFIG_NETPRIO_CGROUP)
+#if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 int net_prio_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_prio_subsys_id);
 #endif

@@ -1182,6 +1182,7 @@ void sock_update_classid(struct sock *sk)
 EXPORT_SYMBOL(sock_update_classid);
 #endif
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 void sock_update_netprioidx(struct sock *sk)
 {
 	u32 prioidx;

@@ -1195,6 +1196,7 @@ void sock_update_netprioidx(struct sock *sk)
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
+#endif
 
 /**
  *	sk_alloc - All socket objects are allocated here

-- 
1.7.11.1.165.g299666c

[PATCH v2 4/5] cgroup: Make builtin vs. module build ifdefs consistent

From: Daniel Wagner <hidden>
Date: 2012-07-11 09:35:46

From: Daniel Wagner <redacted>

The header file is using IS_MODULE and IS_BUILTIN macros. So let's
use them in the source file too.

Signed-off-by: Daniel Wagner <redacted>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/sched/cls_cgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 7743ea8..4bfb8f7 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c

@@ -77,7 +77,7 @@ struct cgroup_subsys net_cls_subsys = {
 	.name		= "net_cls",
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 	.subsys_id	= net_cls_subsys_id,
 #endif
 	.base_cftypes	= ss_files,

@@ -283,7 +283,7 @@ static int __init init_cgroup_cls(void)
 	if (ret)
 		goto out;
 
-#ifndef CONFIG_NET_CLS_CGROUP
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
 	/* We can't use rcu_assign_pointer because this is an int. */
 	smp_wmb();
 	net_cls_subsys_id = net_cls_subsys.subsys_id;

@@ -301,7 +301,7 @@ static void __exit exit_cgroup_cls(void)
 {
 	unregister_tcf_proto_ops(&cls_cgroup_ops);
 
-#ifndef CONFIG_NET_CLS_CGROUP
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
 	net_cls_subsys_id = -1;
 	synchronize_rcu();
 #endif

-- 
1.7.11.1.165.g299666c

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help