Skip to contentSkip to conversation

netprio_cgroup: fix an off-by-one bug

PATCH In review Li Zefan 5 participants 15 messages Feb 1, 2012

Subsystems (5)
Networking Network drivers cgroup Traffic control (tc) +1 more subsystems
lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/6 netprio_cgroup: fix an off-by-one bug

  # mount -t cgroup xxx /mnt
  # mkdir /mnt/tmp
  # cat /mnt/tmp/net_prio.ifpriomap
  lo 0
  eth0 0
  virbr0 0
  # echo 'lo 999' > /mnt/tmp/net_prio.ifpriomap
  # cat /mnt/tmp/net_prio.ifpriomap
  lo 999
  eth0 0
  virbr0 4101267344

We got weired output, because we exceeded the boundary of the array. We may even crash the kernel..

Signed-off-by Li Zefan <[email protected]>
---
 net/core/netprio_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/core/netprio_cgroup.c +1 -1
index 3a9fd48..a296cbb 100644
@@ -107,7 +107,7 @@ static void extend_netdev_table(struct net_device *dev, u32 new_len)
 static void update_netdev_tables(void)
 {
 	struct net_device *dev;
-	u32 max_len = atomic_read(&max_prioidx);
+	u32 max_len = atomic_read(&max_prioidx) + 1;
 	struct netprio_map *map;
 
 	rtnl_lock();
Signature · 1 line
1.7.3.1
author
#1 lore raw

PATCH 2/6 netprio_cgroup: don't allocate prio table when a device is registered

So we delay the allocation till the priority is set through cgroup, and this makes skb_update_priority() faster when it's not set.

This also eliminates an off-by-one bug similar with the one fixed in the previous patch.

Signed-off-by Li Zefan <[email protected]>
---
 net/core/netprio_cgroup.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
net/core/netprio_cgroup.c +0 -6
index a296cbb..2edfa6b 100644
@@ -270,7 +270,6 @@ static int netprio_device_event(struct notifier_block *unused,
 {
 	struct net_device *dev = ptr;
 	struct netprio_map *old;
-	u32 max_len = atomic_read(&max_prioidx);
 
 	/*
 	 * Note this is called with rtnl_lock held so we have update side

@@ -278,11 +277,6 @@ static int netprio_device_event(struct notifier_block *unused,
 	 */
 
 	switch (event) {
-
-	case NETDEV_REGISTER:
-		if (max_len)
-			extend_netdev_table(dev, max_len);
-		break;
 	case NETDEV_UNREGISTER:
 		old = rtnl_dereference(dev->priomap);
 		RCU_INIT_POINTER(dev->priomap, NULL);
Signature · 1 line
1.7.3.1
author
#2 lore raw

PATCH 3/6 netprio_cgroup: fix wrong memory access when NETPRIO_CGROUP=m

In reply to Li Zefan

When the netprio_cgroup module is not loaded, net_prio_subsys_id is -1, and so sock_update_prioidx() accesses cgroup_subsys array with negative index subsys[-1].

Make the code resembles cls_cgroup code, which is bug free.

Signed-off-by Li Zefan <[email protected]>
---
 include/net/netprio_cgroup.h |   48 +++++++++++++++++++++++++++++++++++-------
 net/core/sock.c              |    7 +----
 2 files changed, 42 insertions(+), 13 deletions(-)
include/net/netprio_cgroup.h +40 -8
index 7b2d431..d58fdec 100644
@@ -37,19 +37,51 @@ extern int net_prio_subsys_id;
 
 extern void sock_update_netprioidx(struct sock *sk);
 
-static inline struct cgroup_netprio_state
-		*task_netprio_state(struct task_struct *p)
+#if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
+
+static inline u32 task_netprioidx(struct task_struct *p)
 {
-#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
-	return container_of(task_subsys_state(p, net_prio_subsys_id),
-			    struct cgroup_netprio_state, css);
-#else
-	return NULL;
-#endif
+	struct cgroup_netprio_state *state;
+	u32 idx;
+
+	rcu_read_lock();
+	state = container_of(task_subsys_state(p, net_prio_subsys_id),
+			     struct cgroup_netprio_state, css);
+	idx = state->prioidx;
+	rcu_read_unlock();
+	return idx;
+}
+
+#elif IS_MODULE(CONFIG_NETPRIO_CGROUP)
+
+static inline u32 task_netprioidx(struct task_struct *p)
+{
+	struct cgroup_netprio_state *state;
+	int subsys_id;
+	u32 idx = 0;
+
+	rcu_read_lock();
+	subsys_id = rcu_dereference_index_check(net_prio_subsys_id,
+						rcu_read_lock_held());
+	if (subsys_id >= 0) {
+		state = container_of(task_subsys_state(p, subsys_id),
+				     struct cgroup_netprio_state, css);
+		idx = state->prioidx;
+	}
+	rcu_read_unlock();
+	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
net/core/sock.c +2 -5
index 3e81fd2..02f8dfe 100644
@@ -1171,13 +1171,10 @@ EXPORT_SYMBOL(sock_update_classid);
 
 void sock_update_netprioidx(struct sock *sk)
 {
-	struct cgroup_netprio_state *state;
 	if (in_interrupt())
 		return;
-	rcu_read_lock();
-	state = task_netprio_state(current);
-	sk->sk_cgrp_prioidx = state ? state->prioidx : 0;
-	rcu_read_unlock();
+
+	sk->sk_cgrp_prioidx = task_netprioidx(current);
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
Signature · 1 line
1.7.3.1
author
#3 lore raw

PATCH 4/6 netprio_cgroup: use IS_ENABLED() and family

In reply to Li Zefan

- remove unused net_prio_subsys_id and sock->sk_cgrp->prioidx when CGROUPS=y and NETPRIO_CGROUP=n

- don't use "ifndef CONFIG_NETPRIO_CGROUP" to suggest it's a module, use IS_MODULE().

Signed-off-by Li Zefan <[email protected]>
---
 include/net/netprio_cgroup.h |   23 ++++++-----------------
 include/net/sock.h           |    2 +-
 net/core/netprio_cgroup.c    |    7 +++----
 net/core/sock.c              |    7 +++++--
 4 files changed, 15 insertions(+), 24 deletions(-)
include/net/netprio_cgroup.h +6 -17
index d58fdec..8d09944 100644
@@ -24,18 +24,16 @@ struct netprio_map {
 	u32 priomap[];
 };
 
-#ifdef CONFIG_CGROUPS
-
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 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
+static inline void sock_update_netprioidx(struct sock *sk) {}
+#endif
 
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 

@@ -54,6 +52,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 +72,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 */
include/net/sock.h +1 -1
index 91c1c8b..b08a44e 100644
@@ -342,7 +342,7 @@ struct sock {
 	unsigned short		sk_ack_backlog;
 	unsigned short		sk_max_ack_backlog;
 	__u32			sk_priority;
-#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 	__u32			sk_cgrp_prioidx;
 #endif
 	struct pid		*sk_peer_pid;
net/core/netprio_cgroup.c +3 -4
index 2edfa6b..95ab29a 100644
@@ -33,7 +33,7 @@ struct cgroup_subsys net_prio_subsys = {
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
 	.populate	= cgrp_populate,
-#ifdef CONFIG_NETPRIO_CGROUP
+#if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 	.subsys_id	= net_prio_subsys_id,
 #endif
 	.module		= THIS_MODULE

@@ -298,7 +298,7 @@ static int __init init_cgroup_netprio(void)
 	ret = cgroup_load_subsys(&net_prio_subsys);
 	if (ret)
 		goto out;
-#ifndef CONFIG_NETPRIO_CGROUP
+#if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 	smp_wmb();
 	net_prio_subsys_id = net_prio_subsys.subsys_id;
 #endif

@@ -318,11 +318,10 @@ static void __exit exit_cgroup_netprio(void)
 
 	cgroup_unload_subsys(&net_prio_subsys);
 
-#ifndef CONFIG_NETPRIO_CGROUP
+#if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 	net_prio_subsys_id = -1;
 	synchronize_rcu();
 #endif
-
 	rtnl_lock();
 	for_each_netdev(&init_net, dev) {
 		old = rtnl_dereference(dev->priomap);
net/core/sock.c +5 -2
index 02f8dfe..76df203 100644
@@ -272,11 +272,12 @@ EXPORT_SYMBOL(sysctl_optmem_max);
 int net_cls_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_cls_subsys_id);
 #endif
-#if !defined(CONFIG_NETPRIO_CGROUP)
+#endif
+
+#if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 int net_prio_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_prio_subsys_id);
 #endif
-#endif
 
 static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
 {

@@ -1168,7 +1169,9 @@ void sock_update_classid(struct sock *sk)
 		sk->sk_classid = classid;
 }
 EXPORT_SYMBOL(sock_update_classid);
+#endif
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 void sock_update_netprioidx(struct sock *sk)
 {
 	if (in_interrupt())
Signature · 1 line
1.7.3.1
author
#9 lore raw

PATCH 5/6 cls_cgroup: use IS_ENABLED() and family

In reply to Li Zefan

- remove net_cls_subsys_id when CGROUPS=y && !NET_CLS_CGRUOP
- remove sock->classid when !NET_CLS_CGROUP
- don't use "ifndef CONFIG_NET_CLS_CGROUP" to sugguest it's a module, use IS_MODULE().

Signed-off-by Li Zefan <[email protected]>
---
 include/net/cls_cgroup.h |   12 ++++--------
 include/net/sock.h       |    4 +++-
 net/core/sock.c          |    6 ++----
 net/sched/cls_cgroup.c   |    6 +++---
 4 files changed, 12 insertions(+), 16 deletions(-)
include/net/cls_cgroup.h +4 -8
index a4dc5b0..806b9a1 100644
@@ -17,14 +17,14 @@
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
 
-#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 struct cgroup_cls_state
 {
 	struct cgroup_subsys_state css;
 	u32 classid;
 };
 
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 static inline u32 task_cls_classid(struct task_struct *p)
 {
 	int classid;

@@ -39,7 +39,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
 
 	return classid;
 }
-#else
+#elif IS_MODULE(CONFIG_NET_CLS_CGROUP)
 extern int net_cls_subsys_id;
 
 static inline u32 task_cls_classid(struct task_struct *p)

@@ -61,10 +61,6 @@ static inline u32 task_cls_classid(struct task_struct *p)
 	return classid;
 }
 #endif
-#else
-static inline u32 task_cls_classid(struct task_struct *p)
-{
-	return 0;
-}
+
 #endif
 #endif  /* _NET_CLS_CGROUP_H */
include/net/sock.h +3 -1
index b08a44e..7b80d55 100644
@@ -362,7 +362,9 @@ struct sock {
 	void			*sk_security;
 #endif
 	__u32			sk_mark;
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 	u32			sk_classid;
+#endif
 	struct cg_proto		*sk_cgrp;
 	void			(*sk_state_change)(struct sock *sk);
 	void			(*sk_data_ready)(struct sock *sk, int bytes);

@@ -1382,7 +1384,7 @@ extern void *sock_kmalloc(struct sock *sk, int size,
 extern void sock_kfree_s(struct sock *sk, void *mem, int size);
 extern void sk_send_sigurg(struct sock *sk);
 
-#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 extern void sock_update_classid(struct sock *sk);
 #else
 static inline void sock_update_classid(struct sock *sk)
net/core/sock.c +2 -4
index 76df203..213c856 100644
@@ -267,12 +267,10 @@ __u32 sysctl_rmem_default __read_mostly = SK_RMEM_MAX;
 int sysctl_optmem_max __read_mostly = sizeof(unsigned long)*(2*UIO_MAXIOV+512);
 EXPORT_SYMBOL(sysctl_optmem_max);
 
-#if defined(CONFIG_CGROUPS)
-#if !defined(CONFIG_NET_CLS_CGROUP)
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
 int net_cls_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_cls_subsys_id);
 #endif
-#endif
 
 #if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 int net_prio_subsys_id = -1;

@@ -1157,7 +1155,7 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
 	module_put(owner);
 }
 
-#ifdef CONFIG_CGROUPS
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
net/sched/cls_cgroup.c +3 -3
index f84fdc3..f57245c 100644
@@ -32,7 +32,7 @@ struct cgroup_subsys net_cls_subsys = {
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
 	.populate	= cgrp_populate,
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 	.subsys_id	= net_cls_subsys_id,
 #endif
 	.module		= THIS_MODULE,

@@ -294,7 +294,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;

@@ -312,7 +312,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
Signature · 1 line
1.7.3.1
author
#10 lore raw

PATCH 6/6 cls_cgroup: remove redundant rcu_read_lock/unlock

In reply to Li Zefan

We've already used rcu_read_lock/unlock inside task_classid(), so don't use the lock/unlock pair twice in this hot path.

Signed-off-by Li Zefan <[email protected]>
---
 net/core/sock.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
net/core/sock.c +0 -2
index 213c856..c0bab23 100644
@@ -1160,9 +1160,7 @@ void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
 
-	rcu_read_lock();  /* doing current task, which cannot vanish. */
 	classid = task_cls_classid(current);
-	rcu_read_unlock();
 	if (classid && classid != sk->sk_classid)
 		sk->sk_classid = classid;
 }
Signature · 1 line
1.7.3.1
maintainer
#4 lore raw

Re: [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family

In reply to Li Zefan

Do not mix genuine bug fixes and cleanups.

Otherwise I cannot apply your bug fixes to the 'net' tree.

Seperate things out, submit only pure bug fixes first, then later once those changes propagate you can submit the cleanups.

author
#5 lore raw

Re: [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family

Quoted 8 lines · David Miller

Do not mix genuine bug fixes and cleanups.

Otherwise I cannot apply your bug fixes to the 'net' tree.

Seperate things out, submit only pure bug fixes first, then later once those changes propagate you can submit the cleanups.

I did seperate them out, so the first three patches are fixes, and others are cleanups. I can resend cleanup patches once fixes hit mainline.

#11 lore raw

Re: [PATCH 6/6] cls_cgroup: remove redundant rcu_read_lock/unlock

In reply to Li Zefan
Quoted 13 lines · Li Zefan

We've already used rcu_read_lock/unlock inside task_classid(), so don't use the lock/unlock pair twice in this hot path.

Signed-off-by Li Zefan <[email protected]>
---
 net/core/sock.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
net/core/sock.c +0 -0
index 213c856..c0bab23 100644

net/core/sock.c void sock_update_classid(struct sock *sk) · line 1165

Quoted 8 lines
net/core/sock.c +0 -2
@@ -1160,9 +1160,7 @@ void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
 
-	rcu_read_lock();  /* doing current task, which cannot vanish. */
 	classid = task_cls_classid(current);
-	rcu_read_unlock();
 	if (classid && classid != sk->sk_classid)
 		sk->sk_classid = classid;

Yes, this seems fine.

Then, I wonder why we do the "if (classid && classid != sk->sk_classid)"

before the :

	sk->sk_classid = classid;

This seems unnecessary checks.

maintainer
#6 lore raw

Re: [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family

In reply to Li Zefan

From: Li Zefan <[email protected]>
Date: Wed, 01 Feb 2012 15:06:53 +0800

Quoted 6 lines
Quoted 5 lines · David Miller

Do not mix genuine bug fixes and cleanups.

Otherwise I cannot apply your bug fixes to the 'net' tree.

Quoted 7 lines
Quoted 3 lines

Seperate things out, submit only pure bug fixes first, then later once those changes propagate you can submit the cleanups.

I did seperate them out, so the first three patches are fixes, and others are cleanups. I can resend cleanup patches once fixes hit mainline.

Seperate them in different groups, don't submit them as one collection. And definitely don't submit them all at the same damn time! You have to wait with the dependent cleanups until the fixes go in, and subsequently show up in my net-next tree.

I'm not applying this stuff until you submit it properly, in two stages.

maintainer
#12 lore raw

Re: [PATCH 6/6] cls_cgroup: remove redundant rcu_read_lock/unlock

In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Wed, 01 Feb 2012 08:07:19 +0100

Quoted 8 lines

Then, I wonder why we do the "if (classid && classid != sk->sk_classid)"

before the :

	sk->sk_classid = classid;

This seems unnecessary checks.

Avoiding dirtying the sk->sk_classid cache line unnecessarily?

I actually have no idea actually how often this routine can get invoked in real world scenerios.

author
#13 lore raw

Re: [PATCH 6/6] cls_cgroup: remove redundant rcu_read_lock/unlock

In reply to Eric Dumazet
Cc Herbert Xu <[email protected]>
Quoted 16 lines

net/core/sock.c void sock_update_classid(struct sock *sk) · line 1165

Quoted 13 lines
net/core/sock.c +0 -2
index 213c856..c0bab23 100644
@@ -1160,9 +1160,7 @@ void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
 
-	rcu_read_lock();  /* doing current task, which cannot vanish. */
 	classid = task_cls_classid(current);
-	rcu_read_unlock();
 	if (classid && classid != sk->sk_classid)
 		sk->sk_classid = classid;

Yes, this seems fine.

Quoted 8 lines

Then, I wonder why we do the "if (classid && classid != sk->sk_classid)"

before the :

	sk->sk_classid = classid;

This seems unnecessary checks.

I was wondering about this too. He who added this may provide us with an answer.

author
#7 lore raw

Re: [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family

In reply to David Miller
Quoted 12 lines
Quoted 11 lines
Quoted 7 lines

Do not mix genuine bug fixes and cleanups.

Otherwise I cannot apply your bug fixes to the 'net' tree.

Seperate things out, submit only pure bug fixes first, then later once those changes propagate you can submit the cleanups.

I did seperate them out, so the first three patches are fixes, and others are cleanups. I can resend cleanup patches once fixes hit mainline.

Quoted 9 lines

Seperate them in different groups, don't submit them as one collection. And definitely don't submit them all at the same damn time! You have to wait with the dependent cleanups until the fixes go in, and subsequently show up in my net-next tree.

I'm not applying this stuff until you submit it properly, in two stages.

I'll do this when I get some comments/acks.

#14 lore raw

Re: [PATCH 6/6] cls_cgroup: remove redundant rcu_read_lock/unlock

In reply to Li Zefan
Quoted 22 lines · Li Zefan
Cc Herbert Xu <[email protected]>
Quoted 20 lines

net/core/sock.c void sock_update_classid(struct sock *sk) · line 1165

Quoted 13 lines
net/core/sock.c +0 -2
index 213c856..c0bab23 100644
@@ -1160,9 +1160,7 @@ void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
 
-	rcu_read_lock();  /* doing current task, which cannot vanish. */
 	classid = task_cls_classid(current);
-	rcu_read_unlock();
 	if (classid && classid != sk->sk_classid)
 		sk->sk_classid = classid;

Yes, this seems fine.

Then, I wonder why we do the "if (classid && classid != sk->sk_classid)"

before the :

Quoted 7 lines
Quoted 4 lines
	sk->sk_classid = classid;

This seems unnecessary checks.

I was wondering about this too. He who added this may provide us with an answer.

Well writing a cache-line unnecessarily is bad.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#8 lore raw

Re: [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family

In reply to Li Zefan
Quoted 18 lines · Li Zefan
Quoted 18 lines
Quoted 11 lines
Quoted 7 lines

Do not mix genuine bug fixes and cleanups.

Otherwise I cannot apply your bug fixes to the 'net' tree.

Seperate things out, submit only pure bug fixes first, then later once those changes propagate you can submit the cleanups.

I did seperate them out, so the first three patches are fixes, and others are cleanups. I can resend cleanup patches once fixes hit mainline.

Seperate them in different groups, don't submit them as one collection. And definitely don't submit them all at the same damn time! You have to wait with the dependent cleanups until the fixes go in, and subsequently show up in my net-next tree.

Quoted 6 lines
Quoted 3 lines

I'm not applying this stuff until you submit it properly, in two stages.

I'll do this when I get some comments/acks.

The changes look fine to me.
I'll ack them once you submit them properly.
Thanks
Neil