[-mm patch] one e1000 driver should be enough for everyone

[-mm patch] one e1000 driver should be enough for everyone

[Adrian Bunk]

On Wed, Jul 25, 2007 at 04:03:04AM -0700, Andrew Morton wrote:
>...
> Changes since 2.6.22-rc6-mm1:
>...
>  git-e1000new.patch
>...
>  git trees
>...

Both e1000 drivers compiled into the kernel resulted in the following 
compile error:

<--  snip  -->

...
  LD      drivers/net/built-in.o
drivers/net/e1000/built-in.o: In function `e1000_read_mac_addr':
(.text+0xb9f2): multiple definition of `e1000_read_mac_addr'
drivers/net/e1000new/built-in.o:(.text+0x821a): first defined here
drivers/net/e1000/built-in.o: In function `e1000_phy_setup_autoneg':
(.text+0x8799): multiple definition of `e1000_phy_setup_autoneg'
drivers/net/e1000new/built-in.o:(.text+0xa9bd): first defined here
...
make[3]: *** [drivers/net/built-in.o] Error 1

<--  snip  -->

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

BTW:
Unless I'm misunderstanding anything, the new driver should support a 
superset of what the old driver supported.
Therefore, it would be good if the final merge into Linus' tree will
do an
  rm -r drivers/net/e1000
  mv drivers/net/e1000new drivers/net/e1000

--- linux-2.6.23-rc1-mm1/drivers/net/Kconfig.old	2007-07-25 15:06:13.000000000 +0200
+++ linux-2.6.23-rc1-mm1/drivers/net/Kconfig	2007-07-25 15:09:59.000000000 +0200
@@ -2036,7 +2036,7 @@
 
 config E1000
 	tristate "Intel(R) PRO/1000 Gigabit Ethernet support"
-	depends on PCI
+	depends on PCI && E1000NEW=n
 	---help---
 	  This driver supports Intel(R) PRO/1000 gigabit ethernet family of
 	  adapters.  For more information on how to identify your adapter, go 

Re: [-mm patch] one e1000 driver should be enough for everyone

[Jeff Garzik]

Adrian Bunk wrote:
> BTW:
> Unless I'm misunderstanding anything, the new driver should support a 
> superset of what the old driver supported.
> Therefore, it would be good if the final merge into Linus' tree will
> do an
>   rm -r drivers/net/e1000
>   mv drivers/net/e1000new drivers/net/e1000

Based on the most recent discussion, e1000new (or whatever it will be 
called) should support only the newer PCI-Express chips, while e1000 
will retain support for the older chips.

Over the long term this will allow e1000new to grow without affecting 
support for the older, stable chips.

So, e1000 is not going away.

	Jeff



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [-mm patch] one e1000 driver should be enough for everyone

[Adrian Bunk]

On Wed, Jul 25, 2007 at 09:48:55AM -0400, Jeff Garzik wrote:
> Adrian Bunk wrote:
>> BTW:
>> Unless I'm misunderstanding anything, the new driver should support a 
>> superset of what the old driver supported.
>> Therefore, it would be good if the final merge into Linus' tree will
>> do an
>>   rm -r drivers/net/e1000
>>   mv drivers/net/e1000new drivers/net/e1000
>
> Based on the most recent discussion, e1000new (or whatever it will be 
> called) should support only the newer PCI-Express chips, while e1000 will 
> retain support for the older chips.

I found the discussion, and Christoph's e1000e sounds like the best name 
("new" doesn't say whether it's a new driver for old hardware or a 
driver for new hardware).

> Over the long term this will allow e1000new to grow without affecting 
> support for the older, stable chips.
>
> So, e1000 is not going away.

No problem for me, but this obviously implies that global code in the 
new driver has to be renamed.

And please ensure that they will always support distinct PCI IDs, or 
there will be the following common pattern if both drivers support
a card:
- user tries driver A
- driver A doesn't work (although it should have worked)
- user tries driver B
- driver B works
- a later kernel removes support for this card from driver B
- user tries driver A
- driver A still doesn't work
- user writes bug report

Users should report bugs early instead of bouncing between different 
drivers.

> 	Jeff

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [-mm patch] one e1000 driver should be enough for everyone

[Jeff Garzik]

Adrian Bunk wrote:
> I found the discussion, and Christoph's e1000e sounds like the best name 
> ("new" doesn't say whether it's a new driver for old hardware or a 
> driver for new hardware).

Yeah, I think "e1000new" is a lame name.

e1000e is good, or even e1001e if we wanted even more symmetry :)


> No problem for me, but this obviously implies that global code in the 
> new driver has to be renamed.

Yes.  A global namespace is a global namespace.


> And please ensure that they will always support distinct PCI IDs, or 
> there will be the following common pattern if both drivers support
> a card:

IIRC I think Auke said there is some minor PCI ID overlap that must be 
addressed in the transition.  Disappointing and it raises transition 
issues, but that's the way the split falls out naturally AFAICS.

	Jeff



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [-mm patch] one e1000 driver should be enough for everyone

[Kok, Auke]

Jeff Garzik wrote:
> Adrian Bunk wrote:
>> I found the discussion, and Christoph's e1000e sounds like the best name 
>> ("new" doesn't say whether it's a new driver for old hardware or a 
>> driver for new hardware).
> 
> Yeah, I think "e1000new" is a lame name.

Moreover, Andrew should probably just drop this driver from -mm for now.

> e1000e is good, or even e1001e if we wanted even more symmetry :)

I'm working on "e1000e" right now...

>> No problem for me, but this obviously implies that global code in the 
>> new driver has to be renamed.
> 
> Yes.  A global namespace is a global namespace.

yes, these are some of the kinks I still need to address. Allthough minor, it's 
going to take me some time to get it to the first step before I want to submit 
it (patience :))

>> And please ensure that they will always support distinct PCI IDs, or 
>> there will be the following common pattern if both drivers support
>> a card:
> 
> IIRC I think Auke said there is some minor PCI ID overlap that must be 
> addressed in the transition.  Disappointing and it raises transition 
> issues, but that's the way the split falls out naturally AFAICS.

I'll submit it with only ich9 id's at first, but it will be able to drive (sysfs 
bind) to some other devices too. This allows me to keep an eye out on the future 
structure that I want to give it without removing too much code that I might 
then later have to add back.

Auke

Re: [-mm patch] one e1000 driver should be enough for everyone

[Jeff Garzik]

Kok, Auke wrote:
> I'm working on "e1000e" right now...

Cool :)


> I'll submit it with only ich9 id's at first, but it will be able to 
> drive (sysfs bind) to some other devices too. This allows me to keep an 
> eye out on the future structure that I want to give it without removing 
> too much code that I might then later have to add back.

Sounds good to me...

	Jeff

Re: [-mm patch] one e1000 driver should be enough for everyone

[Andrew Morton]

On Wed, 25 Jul 2007 08:21:10 -0700
"Kok, Auke" <auke-jan.h.kok@intel.com> wrote:

> Moreover, Andrew should probably just drop this driver from -mm for now.

gone..

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error

[Adrian Bunk]

On Wed, Jul 25, 2007 at 04:03:04AM -0700, Andrew Morton wrote:
>...
> Changes since 2.6.22-rc6-mm1:
>...
> +immunize-rcu_dereference-against-crazy-compiler-writers.patch
>...
>  Misc new patches
>...

This patch causes the following compile error:

<--  snip  -->

...
  CC      net/ipv4/fib_trie.o
/home/bunk/linux/kernel-2.6/linux-2.6.23-rc1-mm1/net/ipv4/fib_trie.c: In function ‘trie_rebalance’:
/home/bunk/linux/kernel-2.6/linux-2.6.23-rc1-mm1/net/ipv4/fib_trie.c:969: error: lvalue required as unary ‘&’ operand
/home/bunk/linux/kernel-2.6/linux-2.6.23-rc1-mm1/net/ipv4/fib_trie.c:971: error: lvalue required as unary ‘&’ operand
/home/bunk/linux/kernel-2.6/linux-2.6.23-rc1-mm1/net/ipv4/fib_trie.c:977: error: lvalue required as unary ‘&’ operand
/home/bunk/linux/kernel-2.6/linux-2.6.23-rc1-mm1/net/ipv4/fib_trie.c:980: error: lvalue required as unary ‘&’ operand
...
make[3]: *** [net/ipv4/fib_trie.o] Error 1

<--  snip  -->


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

[RFT] fib_trie: cleanup

[Stephen Hemminger]

Try this out:
     * replace macro's with inlines
     * get rid of places doing multiple evaluations of NODE_PARENT


--- a/net/ipv4/fib_trie.c	2007-07-26 09:22:47.000000000 +0100
+++ b/net/ipv4/fib_trie.c	2007-07-26 09:26:19.000000000 +0100
@@ -93,15 +93,8 @@ typedef unsigned int t_key;
 #define T_TNODE 0
 #define T_LEAF  1
 #define NODE_TYPE_MASK	0x1UL
-#define NODE_PARENT(node) \
-	((struct tnode *)rcu_dereference(((node)->parent & ~NODE_TYPE_MASK)))
-
 #define NODE_TYPE(node) ((node)->parent & NODE_TYPE_MASK)
 
-#define NODE_SET_PARENT(node, ptr)		\
-	rcu_assign_pointer((node)->parent,	\
-			   ((unsigned long)(ptr)) | NODE_TYPE(node))
-
 #define IS_TNODE(n) (!(n->parent & T_LEAF))
 #define IS_LEAF(n) (n->parent & T_LEAF)
 
@@ -174,6 +167,16 @@ static void tnode_free(struct tnode *tn)
 static struct kmem_cache *fn_alias_kmem __read_mostly;
 static struct trie *trie_local = NULL, *trie_main = NULL;
 
+static inline struct tnode *node_parent(struct node *node)
+{
+	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
+}
+
+static inline void node_set_parent(struct node *node, struct tnode *ptr)
+{
+	rcu_assign_pointer(node->parent,
+			   (unsigned long)ptr | NODE_TYPE(node));
+}
 
 /* rcu_read_lock needs to be hold by caller from readside */
 
@@ -446,7 +449,7 @@ static void tnode_put_child_reorg(struct
 		tn->full_children++;
 
 	if (n)
-		NODE_SET_PARENT(n, tn);
+		node_set_parent(n, tn);
 
 	rcu_assign_pointer(tn->child[i], n);
 }
@@ -481,7 +484,7 @@ static struct node *resize(struct trie *
 				continue;
 
 			/* compress one level */
-			NODE_SET_PARENT(n, NULL);
+			node_set_parent(n, NULL);
 			tnode_free(tn);
 			return n;
 		}
@@ -636,7 +639,7 @@ static struct node *resize(struct trie *
 
 			/* compress one level */
 
-			NODE_SET_PARENT(n, NULL);
+			node_set_parent(n, NULL);
 			tnode_free(tn);
 			return n;
 		}
@@ -961,24 +964,21 @@ fib_find_node(struct trie *t, u32 key)
 static struct node *trie_rebalance(struct trie *t, struct tnode *tn)
 {
 	int wasfull;
-	t_key cindex, key;
-	struct tnode *tp = NULL;
-
-	key = tn->key;
+	t_key cindex, key = tn->key;
+	struct tnode *tp;
 
-	while (tn != NULL && NODE_PARENT(tn) != NULL) {
-
-		tp = NODE_PARENT(tn);
+	while (tn != NULL && (tp = node_parent((struct node *)tn)) != NULL) {
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
 		wasfull = tnode_full(tp, tnode_get_child(tp, cindex));
 		tn = (struct tnode *) resize (t, (struct tnode *)tn);
 		tnode_put_child_reorg((struct tnode *)tp, cindex,(struct node*)tn, wasfull);
 
-		if (!NODE_PARENT(tn))
+		tp = node_parent((struct node *) tn);
+		if (!tp)
 			break;
-
-		tn = NODE_PARENT(tn);
+		tn = tp;
 	}
+
 	/* Handle last (top) tnode */
 	if (IS_TNODE(tn))
 		tn = (struct tnode*) resize(t, (struct tnode *)tn);
@@ -1031,7 +1031,7 @@ fib_insert_node(struct trie *t, int *err
 			pos = tn->pos + tn->bits;
 			n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
 
-			BUG_ON(n && NODE_PARENT(n) != tn);
+			BUG_ON(n && node_parent(n) != tn);
 		} else
 			break;
 	}
@@ -1083,7 +1083,7 @@ fib_insert_node(struct trie *t, int *err
 	if (t->trie && n == NULL) {
 		/* Case 2: n is NULL, and will just insert a new leaf */
 
-		NODE_SET_PARENT(l, tp);
+		node_set_parent((struct node *)l, tp);
 
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
 		put_child(t, (struct tnode *)tp, cindex, (struct node *)l);
@@ -1114,7 +1114,7 @@ fib_insert_node(struct trie *t, int *err
 			goto err;
 		}
 
-		NODE_SET_PARENT(tn, tp);
+		node_set_parent((struct node *)tn, tp);
 
 		missbit = tkey_extract_bits(key, newpos, 1);
 		put_child(t, tn, missbit, (struct node *)l);
@@ -1495,12 +1495,13 @@ backtrace:
 		if (chopped_off <= pn->bits) {
 			cindex &= ~(1 << (chopped_off-1));
 		} else {
-			if (NODE_PARENT(pn) == NULL)
+			struct tnode *parent = node_parent((struct node *) pn);
+			if (!parent)
 				goto failed;
 
 			/* Get Child's index */
-			cindex = tkey_extract_bits(pn->key, NODE_PARENT(pn)->pos, NODE_PARENT(pn)->bits);
-			pn = NODE_PARENT(pn);
+			cindex = tkey_extract_bits(pn->key, parent->pos, parent->bits);
+			pn = parent;
 			chopped_off = 0;
 
 #ifdef CONFIG_IP_FIB_TRIE_STATS
@@ -1536,7 +1537,7 @@ static int trie_leaf_remove(struct trie 
 		check_tnode(tn);
 		n = tnode_get_child(tn ,tkey_extract_bits(key, tn->pos, tn->bits));
 
-		BUG_ON(n && NODE_PARENT(n) != tn);
+		BUG_ON(n && node_parent(n) != tn);
 	}
 	l = (struct leaf *) n;
 
@@ -1551,7 +1552,7 @@ static int trie_leaf_remove(struct trie 
 	t->revision++;
 	t->size--;
 
-	tp = NODE_PARENT(n);
+	tp = node_parent(n);
 	tnode_free((struct tnode *) n);
 
 	if (tp) {
@@ -1703,7 +1704,7 @@ static struct leaf *nextleaf(struct trie
 
 		p = (struct tnode*) trie;  /* Start */
 	} else
-		p = (struct tnode *) NODE_PARENT(c);
+		p = node_parent(c);
 
 	while (p) {
 		int pos, last;
@@ -1740,7 +1741,7 @@ static struct leaf *nextleaf(struct trie
 up:
 		/* No more children go up one step  */
 		c = (struct node *) p;
-		p = (struct tnode *) NODE_PARENT(p);
+		p = node_parent(c);
 	}
 	return NULL; /* Ready. Root of trie */
 }
@@ -2043,7 +2044,7 @@ rescan:
 	}
 
 	/* Current node exhausted, pop back up */
-	p = NODE_PARENT(tn);
+	p = node_parent((struct node *)tn);
 	if (p) {
 		cindex = tkey_extract_bits(tn->key, p->pos, p->bits)+1;
 		tn = p;
@@ -2317,7 +2318,7 @@ static int fib_trie_seq_show(struct seq_
 	if (v == SEQ_START_TOKEN)
 		return 0;
 
-	if (!NODE_PARENT(n)) {
+	if (!node_parent(n)) {
 		if (iter->trie == trie_local)
 			seq_puts(seq, "<local>:\n");
 		else

Re: [RFT] fib_trie: cleanup

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 26 Jul 2007 09:46:48 +0100

> Try this out:
>      * replace macro's with inlines
>      * get rid of places doing multiple evaluations of NODE_PARENT

No objections from me.

Robert?

Re: [RFT] fib_trie: cleanup

[Robert Olsson]

David Miller writes:
 > From: Stephen Hemminger <shemminger@linux-foundation.org>
 > Date: Thu, 26 Jul 2007 09:46:48 +0100
 > 
 > > Try this out:
 > >      * replace macro's with inlines
 > >      * get rid of places doing multiple evaluations of NODE_PARENT
 > 
 > No objections from me.
 > 
 > Robert?

 Fine it looks cleaner and compiles... thanks Stephen.

 BTW 
 I think might be possible improve functional/performance parts too. It's the 
 list handling I'm thinking of.

 I've observed that in most cases there is only one leaf_info in the leaf list.
 In 96% of current Internet prefixes in the router I just looked into (below). 
 So optimizing for this case could be an idea. Some variant were the leaf holds
 the leaf_info data direct could be worth testing.

 Cheers.
						--ro


 Main:
        Aver depth:     2.57
        Max depth:      6
        Leaves:         215131
        Internal nodes: 52394
          1: 27662  2: 9982  3: 8510  4: 3625  5: 1684  6: 626  7: 240  8: 64  16: 1
        Pointers: 427924
Null ptrs: 160400
Total size: 7102  kB


ip route list | wc -l 
 224649

215131/224649 = 96%

Re: [RFT] fib_trie: cleanup

[Andrew Morton]

On Thu, 26 Jul 2007 09:46:48 +0100 Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> Try this out:
>      * replace macro's with inlines
>      * get rid of places doing multiple evaluations of NODE_PARENT

And it fixes the rcu abuse which resulted in
http://lkml.org/lkml/2007/7/25/319.  Which I assume is what inspired the
effort.


your typecasting style is inconsistent:

> +	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));

space

> +			   (unsigned long)ptr | NODE_TYPE(node));

no space

> +	while (tn != NULL && (tp = node_parent((struct node *)tn)) != NULL) {

no space

> +		tp = node_parent((struct node *) tn);

space

> +		node_set_parent((struct node *)l, tp);

no space

> +		node_set_parent((struct node *)tn, tp);

no space

> +			struct tnode *parent = node_parent((struct node *) pn);

space

etc.


"no space" seems to be more usual, by about a 50.1:49.9 ratio.  It is
a lot more common in net/:

box:/usr/src/25> grep '[(]unsigned long[)] ' net/*/*.c  | wc -l
91
box:/usr/src/25> grep '[(]unsigned long[)][^ ]' net/*/*.c  | wc -l 
242

Re: [RFT] fib_trie: cleanup

[Stephen Hemminger]

On Thu, 26 Jul 2007 02:04:31 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 26 Jul 2007 09:46:48 +0100 Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> 
> > Try this out:
> >      * replace macro's with inlines
> >      * get rid of places doing multiple evaluations of NODE_PARENT
> 
> And it fixes the rcu abuse which resulted in
> http://lkml.org/lkml/2007/7/25/319.  Which I assume is what inspired the
> effort.
> 
> 
> your typecasting style is inconsistent:
> 
> > +	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
> 
> space

I have a couple of followon cleanup patches, one of them is whitespace demunging

[RFC] fib_trie: whitespace cleanup

[Stephen Hemminger]

Whitespace cleanup run code through lindent then cleanup results.
Applys after other two patches.

--- a/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
+++ b/net/ipv4/fib_trie.c	2007-07-26 11:47:52.000000000 +0100
@@ -156,7 +156,8 @@ struct trie {
 };
 
 static void put_child(struct trie *t, struct tnode *tn, int i, struct node *n);
-static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull);
+static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
+				  int wasfull);
 static struct node *resize(struct trie *t, struct tnode *tn);
 static struct tnode *inflate(struct trie *t, struct tnode *tn);
 static struct tnode *halve(struct trie *t, struct tnode *tn);
@@ -167,13 +168,12 @@ static struct trie *trie_local = NULL, *
 
 static inline struct tnode *node_parent(struct node *node)
 {
-	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
+	return rcu_dereference((struct tnode *)(node->parent & ~NODE_TYPE_MASK));
 }
 
 static inline void node_set_parent(struct node *node, struct tnode *ptr)
 {
-	rcu_assign_pointer(node->parent,
-			   (unsigned long)ptr | NODE_TYPE(node));
+	rcu_assign_pointer(node->parent, (unsigned long)ptr | NODE_TYPE(node));
 }
 
 /* rcu_read_lock needs to be hold by caller from readside */
@@ -192,13 +192,13 @@ static inline int tnode_child_length(con
 
 static inline t_key mask_pfx(t_key k, unsigned short l)
 {
-	return (l == 0) ? 0 : k >> (KEYLENGTH-l) << (KEYLENGTH-l);
+	return (l == 0) ? 0 : k >> (KEYLENGTH - l) << (KEYLENGTH - l);
 }
 
 static inline t_key tkey_extract_bits(t_key a, int offset, int bits)
 {
 	if (offset < KEYLENGTH)
-		return ((t_key)(a << offset)) >> (KEYLENGTH - bits);
+		return ((t_key) (a << offset)) >> (KEYLENGTH - bits);
 	else
 		return 0;
 }
@@ -223,7 +223,7 @@ static inline int tkey_mismatch(t_key a,
 
 	if (!diff)
 		return 0;
-	while ((diff << i) >> (KEYLENGTH-1) == 0)
+	while ((diff << i) >> (KEYLENGTH - 1) == 0)
 		i++;
 	return i;
 }
@@ -285,7 +285,6 @@ static inline int tkey_mismatch(t_key a,
   The bits from (n->pos) to (n->pos + n->bits - 1) - "C" - are the index into
   n's child array, and will of course be different for each child.
 
-
   The rest of the bits, from (n->pos + n->bits) onward, are completely unknown
   at this point.
 
@@ -293,7 +292,7 @@ static inline int tkey_mismatch(t_key a,
 
 static inline void check_tnode(const struct tnode *tn)
 {
-	WARN_ON(tn && tn->pos+tn->bits > 32);
+	WARN_ON(tn && tn->pos + tn->bits > 32);
 }
 
 static int halve_threshold = 25;
@@ -301,7 +300,6 @@ static int inflate_threshold = 50;
 static int halve_threshold_root = 8;
 static int inflate_threshold_root = 15;
 
-
 static void __alias_free_mem(struct rcu_head *head)
 {
 	struct fib_alias *fa = container_of(head, struct fib_alias, rcu);
@@ -335,7 +333,7 @@ static struct tnode *tnode_alloc(unsigne
 	if (size <= PAGE_SIZE)
 		return kcalloc(size, 1, GFP_KERNEL);
 
-	pages = alloc_pages(GFP_KERNEL|__GFP_ZERO, get_order(size));
+	pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(size));
 	if (!pages)
 		return NULL;
 
@@ -346,7 +344,7 @@ static void __tnode_free_rcu(struct rcu_
 {
 	struct tnode *tn = container_of(head, struct tnode, rcu);
 	unsigned int size = sizeof(struct tnode) +
-		(1 << tn->bits) * sizeof(struct node *);
+	    (1 << tn->bits) * sizeof(struct node *);
 
 	if (size <= PAGE_SIZE)
 		kfree(tn);
@@ -357,7 +355,7 @@ static void __tnode_free_rcu(struct rcu_
 static inline void tnode_free(struct tnode *tn)
 {
 	if (IS_LEAF(tn)) {
-		struct leaf *l = (struct leaf *) tn;
+		struct leaf *l = (struct leaf *)tn;
 		call_rcu_bh(&l->rcu, __leaf_free_rcu);
 	} else
 		call_rcu(&tn->rcu, __tnode_free_rcu);
@@ -365,7 +363,7 @@ static inline void tnode_free(struct tno
 
 static struct leaf *leaf_new(void)
 {
-	struct leaf *l = kmalloc(sizeof(struct leaf),  GFP_KERNEL);
+	struct leaf *l = kmalloc(sizeof(struct leaf), GFP_KERNEL);
 	if (l) {
 		l->parent = T_LEAF;
 		INIT_HLIST_HEAD(&l->list);
@@ -375,7 +373,7 @@ static struct leaf *leaf_new(void)
 
 static struct leaf_info *leaf_info_new(int plen)
 {
-	struct leaf_info *li = kmalloc(sizeof(struct leaf_info),  GFP_KERNEL);
+	struct leaf_info *li = kmalloc(sizeof(struct leaf_info), GFP_KERNEL);
 	if (li) {
 		li->plen = plen;
 		INIT_LIST_HEAD(&li->falh);
@@ -383,9 +381,9 @@ static struct leaf_info *leaf_info_new(i
 	return li;
 }
 
-static struct tnode* tnode_new(t_key key, int pos, int bits)
+static struct tnode *tnode_new(t_key key, int pos, int bits)
 {
-	int nchildren = 1<<bits;
+	int nchildren = 1 << bits;
 	int sz = sizeof(struct tnode) + nchildren * sizeof(struct node *);
 	struct tnode *tn = tnode_alloc(sz);
 
@@ -396,11 +394,11 @@ static struct tnode* tnode_new(t_key key
 		tn->bits = bits;
 		tn->key = key;
 		tn->full_children = 0;
-		tn->empty_children = 1<<bits;
+		tn->empty_children = 1 << bits;
 	}
 
-	pr_debug("AT %p s=%u %u\n", tn, (unsigned int) sizeof(struct tnode),
-		 (unsigned int) (sizeof(struct node) * 1<<bits));
+	pr_debug("AT %p s=%u %u\n", tn, (unsigned int)sizeof(struct tnode),
+		 (unsigned int)(sizeof(struct node) * 1 << bits));
 	return tn;
 }
 
@@ -414,10 +412,11 @@ static inline int tnode_full(const struc
 	if (n == NULL || IS_LEAF(n))
 		return 0;
 
-	return ((struct tnode *) n)->pos == tn->pos + tn->bits;
+	return ((struct tnode *)n)->pos == tn->pos + tn->bits;
 }
 
-static inline void put_child(struct trie *t, struct tnode *tn, int i, struct node *n)
+static inline void put_child(struct trie *t, struct tnode *tn, int i,
+			     struct node *n)
 {
 	tnode_put_child_reorg(tn, i, n, -1);
 }
@@ -427,13 +426,13 @@ static inline void put_child(struct trie
   * Update the value of full_children and empty_children.
   */
 
-static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull)
+static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
+				  int wasfull)
 {
 	struct node *chi = tn->child[i];
 	int isfull;
 
-	BUG_ON(i >= 1<<tn->bits);
-
+	BUG_ON(i >= 1 << tn->bits);
 
 	/* update emptyChildren */
 	if (n == NULL && chi != NULL)
@@ -566,9 +565,10 @@ static struct node *resize(struct trie *
 
 	err = 0;
 	max_resize = 10;
-	while ((tn->full_children > 0 &&  max_resize-- &&
-	       50 * (tn->full_children + tnode_child_length(tn) - tn->empty_children) >=
-				inflate_threshold_use * tnode_child_length(tn))) {
+	while ((tn->full_children > 0 && max_resize-- &&
+		50 * (tn->full_children + tnode_child_length(tn) -
+		      tn->empty_children) >=
+		inflate_threshold_use * tnode_child_length(tn))) {
 
 		old_tn = tn;
 		tn = inflate(t, tn);
@@ -583,10 +583,12 @@ static struct node *resize(struct trie *
 
 	if (max_resize < 0) {
 		if (!tn->parent)
-			printk(KERN_WARNING "Fix inflate_threshold_root. Now=%d size=%d bits\n",
+			printk(KERN_WARNING
+			       "Fix inflate_threshold_root. Now=%d size=%d bits\n",
 			       inflate_threshold_root, tn->bits);
 		else
-			printk(KERN_WARNING "Fix inflate_threshold. Now=%d size=%d bits\n",
+			printk(KERN_WARNING
+			       "Fix inflate_threshold. Now=%d size=%d bits\n",
 			       inflate_threshold, tn->bits);
 	}
 
@@ -597,7 +599,6 @@ static struct node *resize(struct trie *
 	 * node is above threshold.
 	 */
 
-
 	/* Keep root node larger  */
 
 	if (!tn->parent)
@@ -607,7 +608,7 @@ static struct node *resize(struct trie *
 
 	err = 0;
 	max_resize = 10;
-	while (tn->bits > 1 &&  max_resize-- &&
+	while (tn->bits > 1 && max_resize-- &&
 	       100 * (tnode_child_length(tn) - tn->empty_children) <
 	       halve_threshold_use * tnode_child_length(tn)) {
 
@@ -624,10 +625,12 @@ static struct node *resize(struct trie *
 
 	if (max_resize < 0) {
 		if (!tn->parent)
-			printk(KERN_WARNING "Fix halve_threshold_root. Now=%d size=%d bits\n",
+			printk(KERN_WARNING
+			       "Fix halve_threshold_root. Now=%d size=%d bits\n",
 			       halve_threshold_root, tn->bits);
 		else
-			printk(KERN_WARNING "Fix halve_threshold. Now=%d size=%d bits\n",
+			printk(KERN_WARNING
+			       "Fix halve_threshold. Now=%d size=%d bits\n",
 			       halve_threshold, tn->bits);
 	}
 
@@ -647,7 +650,7 @@ static struct node *resize(struct trie *
 			return n;
 		}
 
-	return (struct node *) tn;
+	return (struct node *)tn;
 }
 
 static struct tnode *inflate(struct trie *t, struct tnode *tn)
@@ -672,7 +675,8 @@ static struct tnode *inflate(struct trie
 	 */
 
 	for (i = 0; i < olen; i++) {
-		struct tnode *inode = (struct tnode *) tnode_get_child(oldtnode, i);
+		struct tnode *inode =
+		    (struct tnode *)tnode_get_child(oldtnode, i);
 
 		if (inode &&
 		    IS_TNODE(inode) &&
@@ -681,12 +685,12 @@ static struct tnode *inflate(struct trie
 			struct tnode *left, *right;
 			t_key m = ~0U << (KEYLENGTH - 1) >> inode->pos;
 
-			left = tnode_new(inode->key&(~m), inode->pos + 1,
+			left = tnode_new(inode->key & (~m), inode->pos + 1,
 					 inode->bits - 1);
 			if (!left)
 				goto nomem;
 
-			right = tnode_new(inode->key|m, inode->pos + 1,
+			right = tnode_new(inode->key | m, inode->pos + 1,
 					  inode->bits - 1);
 
 			if (!right) {
@@ -694,8 +698,8 @@ static struct tnode *inflate(struct trie
 				goto nomem;
 			}
 
-			put_child(t, tn, 2*i, (struct node *) left);
-			put_child(t, tn, 2*i+1, (struct node *) right);
+			put_child(t, tn, 2 * i, (struct node *)left);
+			put_child(t, tn, 2 * i + 1, (struct node *)right);
 		}
 	}
 
@@ -710,22 +714,22 @@ static struct tnode *inflate(struct trie
 
 		/* A leaf or an internal node with skipped bits */
 
-		if (IS_LEAF(node) || ((struct tnode *) node)->pos >
-		   tn->pos + tn->bits - 1) {
-			if (tkey_extract_bits(node->key, oldtnode->pos + oldtnode->bits,
-					     1) == 0)
-				put_child(t, tn, 2*i, node);
+		if (IS_LEAF(node) || ((struct tnode *)node)->pos >
+		    tn->pos + tn->bits - 1) {
+			if (!tkey_extract_bits(node->key,
+					      oldtnode->pos + oldtnode->bits, 1))
+				put_child(t, tn, 2 * i, node);
 			else
-				put_child(t, tn, 2*i+1, node);
+				put_child(t, tn, 2 * i + 1, node);
 			continue;
 		}
 
 		/* An internal node with two children */
-		inode = (struct tnode *) node;
+		inode = (struct tnode *)node;
 
 		if (inode->bits == 1) {
-			put_child(t, tn, 2*i, inode->child[0]);
-			put_child(t, tn, 2*i+1, inode->child[1]);
+			put_child(t, tn, 2 * i, inode->child[0]);
+			put_child(t, tn, 2 * i + 1, inode->child[1]);
 
 			tnode_free(inode);
 			continue;
@@ -754,13 +758,13 @@ static struct tnode *inflate(struct trie
 		 *   bit to zero.
 		 */
 
-		left = (struct tnode *) tnode_get_child(tn, 2*i);
-		put_child(t, tn, 2*i, NULL);
+		left = (struct tnode *)tnode_get_child(tn, 2 * i);
+		put_child(t, tn, 2 * i, NULL);
 
 		BUG_ON(!left);
 
-		right = (struct tnode *) tnode_get_child(tn, 2*i+1);
-		put_child(t, tn, 2*i+1, NULL);
+		right = (struct tnode *)tnode_get_child(tn, 2 * i + 1);
+		put_child(t, tn, 2 * i + 1, NULL);
 
 		BUG_ON(!right);
 
@@ -769,8 +773,8 @@ static struct tnode *inflate(struct trie
 			put_child(t, left, j, inode->child[j]);
 			put_child(t, right, j, inode->child[j + size]);
 		}
-		put_child(t, tn, 2*i, resize(t, left));
-		put_child(t, tn, 2*i+1, resize(t, right));
+		put_child(t, tn, 2 * i, resize(t, left));
+		put_child(t, tn, 2 * i + 1, resize(t, right));
 
 		tnode_free(inode);
 	}
@@ -814,7 +818,7 @@ static struct tnode *halve(struct trie *
 
 	for (i = 0; i < olen; i += 2) {
 		left = tnode_get_child(oldtnode, i);
-		right = tnode_get_child(oldtnode, i+1);
+		right = tnode_get_child(oldtnode, i + 1);
 
 		/* Two nonempty children */
 		if (left && right) {
@@ -825,7 +829,7 @@ static struct tnode *halve(struct trie *
 			if (!newn)
 				goto nomem;
 
-			put_child(t, tn, i/2, (struct node *)newn);
+			put_child(t, tn, i / 2, (struct node *)newn);
 		}
 
 	}
@@ -834,27 +838,27 @@ static struct tnode *halve(struct trie *
 		struct tnode *newBinNode;
 
 		left = tnode_get_child(oldtnode, i);
-		right = tnode_get_child(oldtnode, i+1);
+		right = tnode_get_child(oldtnode, i + 1);
 
 		/* At least one of the children is empty */
 		if (left == NULL) {
-			if (right == NULL)    /* Both are empty */
+			if (right == NULL)	/* Both are empty */
 				continue;
-			put_child(t, tn, i/2, right);
+			put_child(t, tn, i / 2, right);
 			continue;
 		}
 
 		if (right == NULL) {
-			put_child(t, tn, i/2, left);
+			put_child(t, tn, i / 2, left);
 			continue;
 		}
 
 		/* Two nonempty children */
-		newBinNode = (struct tnode *) tnode_get_child(tn, i/2);
-		put_child(t, tn, i/2, NULL);
+		newBinNode = (struct tnode *)tnode_get_child(tn, i / 2);
+		put_child(t, tn, i / 2, NULL);
 		put_child(t, newBinNode, 0, left);
 		put_child(t, newBinNode, 1, right);
-		put_child(t, tn, i/2, resize(t, newBinNode));
+		put_child(t, tn, i / 2, resize(t, newBinNode));
 	}
 	tnode_free(oldtnode);
 	return tn;
@@ -896,13 +900,13 @@ static struct leaf_info *find_leaf_info(
 	struct leaf_info *li;
 
 	hlist_for_each_entry_rcu(li, node, head, hlist)
-		if (li->plen == plen)
-			return li;
+	    if (li->plen == plen)
+		return li;
 
 	return NULL;
 }
 
-static inline struct list_head * get_fa_head(struct leaf *l, int plen)
+static inline struct list_head *get_fa_head(struct leaf *l, int plen)
 {
 	struct leaf_info *li = find_leaf_info(l, plen);
 
@@ -935,8 +939,7 @@ static void insert_leaf_info(struct hlis
 
 /* rcu_read_lock needs to be hold by caller from readside */
 
-static struct leaf *
-fib_find_node(struct trie *t, u32 key)
+static struct leaf *fib_find_node(struct trie *t, u32 key)
 {
 	int pos;
 	struct tnode *tn;
@@ -945,14 +948,16 @@ fib_find_node(struct trie *t, u32 key)
 	pos = 0;
 	n = rcu_dereference(t->trie);
 
-	while (n != NULL &&  NODE_TYPE(n) == T_TNODE) {
-		tn = (struct tnode *) n;
+	while (n != NULL && NODE_TYPE(n) == T_TNODE) {
+		tn = (struct tnode *)n;
 
 		check_tnode(tn);
 
-		if (tkey_sub_equals(tn->key, pos, tn->pos-pos, key)) {
+		if (tkey_sub_equals(tn->key, pos, tn->pos - pos, key)) {
 			pos = tn->pos + tn->bits;
-			n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
+			n = tnode_get_child(tn,
+					    tkey_extract_bits(key, tn->pos,
+							      tn->bits));
 		} else
 			break;
 	}
@@ -973,10 +978,11 @@ static struct node *trie_rebalance(struc
 	while (tn != NULL && (tp = node_parent((struct node *)tn)) != NULL) {
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
 		wasfull = tnode_full(tp, tnode_get_child(tp, cindex));
-		tn = (struct tnode *) resize (t, (struct tnode *)tn);
-		tnode_put_child_reorg((struct tnode *)tp, cindex,(struct node*)tn, wasfull);
+		tn = (struct tnode *)resize(t, (struct tnode *)tn);
+		tnode_put_child_reorg((struct tnode *)tp, cindex,
+				      (struct node *)tn, wasfull);
 
-		tp = node_parent((struct node *) tn);
+		tp = node_parent((struct node *)tn);
 		if (!tp)
 			break;
 		tn = tp;
@@ -984,15 +990,15 @@ static struct node *trie_rebalance(struc
 
 	/* Handle last (top) tnode */
 	if (IS_TNODE(tn))
-		tn = (struct tnode*) resize(t, (struct tnode *)tn);
+		tn = (struct tnode *)resize(t, (struct tnode *)tn);
 
-	return (struct node*) tn;
+	return (struct node *)tn;
 }
 
 /* only used from updater-side */
 
-static  struct list_head *
-fib_insert_node(struct trie *t, int *err, u32 key, int plen)
+static struct list_head *fib_insert_node(struct trie *t, int *err, u32 key,
+					 int plen)
 {
 	int pos, newpos;
 	struct tnode *tp = NULL, *tn = NULL;
@@ -1024,15 +1030,17 @@ fib_insert_node(struct trie *t, int *err
 	 * If it doesn't, we need to replace it with a T_TNODE.
 	 */
 
-	while (n != NULL &&  NODE_TYPE(n) == T_TNODE) {
-		tn = (struct tnode *) n;
+	while (n != NULL && NODE_TYPE(n) == T_TNODE) {
+		tn = (struct tnode *)n;
 
 		check_tnode(tn);
 
-		if (tkey_sub_equals(tn->key, pos, tn->pos-pos, key)) {
+		if (tkey_sub_equals(tn->key, pos, tn->pos - pos, key)) {
 			tp = tn;
 			pos = tn->pos + tn->bits;
-			n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
+			n = tnode_get_child(tn,
+					    tkey_extract_bits(key, tn->pos,
+							      tn->bits));
 
 			BUG_ON(n && node_parent(n) != tn);
 		} else
@@ -1050,7 +1058,7 @@ fib_insert_node(struct trie *t, int *err
 	/* Case 1: n is a leaf. Compare prefixes */
 
 	if (n != NULL && IS_LEAF(n) && tkey_equals(key, n->key)) {
-		struct leaf *l = (struct leaf *) n;
+		struct leaf *l = (struct leaf *)n;
 
 		li = leaf_info_new(plen);
 
@@ -1075,7 +1083,7 @@ fib_insert_node(struct trie *t, int *err
 	li = leaf_info_new(plen);
 
 	if (!li) {
-		tnode_free((struct tnode *) l);
+		tnode_free((struct tnode *)l);
 		*err = -ENOMEM;
 		goto err;
 	}
@@ -1098,7 +1106,7 @@ fib_insert_node(struct trie *t, int *err
 		 */
 
 		if (tp)
-			pos = tp->pos+tp->bits;
+			pos = tp->pos + tp->bits;
 		else
 			pos = 0;
 
@@ -1107,12 +1115,12 @@ fib_insert_node(struct trie *t, int *err
 			tn = tnode_new(n->key, newpos, 1);
 		} else {
 			newpos = 0;
-			tn = tnode_new(key, newpos, 1); /* First tnode */
+			tn = tnode_new(key, newpos, 1);	/* First tnode */
 		}
 
 		if (!tn) {
 			free_leaf_info(li);
-			tnode_free((struct tnode *) l);
+			tnode_free((struct tnode *)l);
 			*err = -ENOMEM;
 			goto err;
 		}
@@ -1121,20 +1129,22 @@ fib_insert_node(struct trie *t, int *err
 
 		missbit = tkey_extract_bits(key, newpos, 1);
 		put_child(t, tn, missbit, (struct node *)l);
-		put_child(t, tn, 1-missbit, n);
+		put_child(t, tn, 1 - missbit, n);
 
 		if (tp) {
 			cindex = tkey_extract_bits(key, tp->pos, tp->bits);
-			put_child(t, (struct tnode *)tp, cindex, (struct node *)tn);
+			put_child(t, (struct tnode *)tp, cindex,
+				  (struct node *)tn);
 		} else {
-			rcu_assign_pointer(t->trie, (struct node *)tn); /* First tnode */
+			rcu_assign_pointer(t->trie, (struct node *)tn);	/* First tnode */
 			tp = tn;
 		}
 	}
 
 	if (tp && tp->pos + tp->bits > 32)
-		printk(KERN_WARNING "fib_trie tp=%p pos=%d, bits=%d, key=%0x plen=%d\n",
-		       tp, tp->pos, tp->bits, key, plen);
+		printk(KERN_WARNING
+		       "fib_trie tp=%p pos=%d, bits=%d, key=%0x plen=%d\n", tp,
+		       tp->pos, tp->bits, key, plen);
 
 	/* Rebalance the trie */
 
@@ -1150,7 +1160,7 @@ err:
  */
 static int fn_trie_insert(struct fib_table *tb, struct fib_config *cfg)
 {
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 	struct fib_alias *fa, *new_fa;
 	struct list_head *fa_head = NULL;
 	struct fib_info *fi;
@@ -1230,7 +1240,7 @@ static int fn_trie_insert(struct fib_tab
 			if (state & FA_S_ACCESSED)
 				rt_cache_flush(-1);
 			rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen,
-				tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
+				  tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
 
 			goto succeeded;
 		}
@@ -1278,8 +1288,7 @@ static int fn_trie_insert(struct fib_tab
 			goto out_free_new_fa;
 	}
 
-	list_add_tail_rcu(&new_fa->fa_list,
-			  (fa ? &fa->fa_list : fa_head));
+	list_add_tail_rcu(&new_fa->fa_list, (fa ? &fa->fa_list : fa_head));
 
 	rt_cache_flush(-1);
 	rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen, tb->tb_id,
@@ -1295,7 +1304,6 @@ err:
 	return err;
 }
 
-
 /* should be called with rcu_read_lock */
 static inline int check_leaf(struct trie *t, struct leaf *l,
 			     t_key key, int *plen, const struct flowi *flp,
@@ -1313,7 +1321,9 @@ static inline int check_leaf(struct trie
 		if (l->key != (key & ntohl(mask)))
 			continue;
 
-		if ((err = fib_semantic_match(&li->falh, flp, res, htonl(l->key), mask, i)) <= 0) {
+		err = fib_semantic_match(&li->falh, flp, res, htonl(l->key),
+					 mask, i);
+		if (err <= 0) {
 			*plen = i;
 #ifdef CONFIG_IP_FIB_TRIE_STATS
 			t->stats.semantic_match_passed++;
@@ -1328,9 +1338,10 @@ static inline int check_leaf(struct trie
 }
 
 static int
-fn_trie_lookup(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
+fn_trie_lookup(struct fib_table *tb, const struct flowi *flp,
+	       struct fib_result *res)
 {
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 	int plen, ret = 0;
 	struct node *n;
 	struct tnode *pn;
@@ -1355,11 +1366,12 @@ fn_trie_lookup(struct fib_table *tb, con
 
 	/* Just a leaf? */
 	if (IS_LEAF(n)) {
-		if ((ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res)) <= 0)
+		ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res);
+		if (ret <= 0)
 			goto found;
 		goto failed;
 	}
-	pn = (struct tnode *) n;
+	pn = (struct tnode *)n;
 	chopped_off = 0;
 
 	while (pn) {
@@ -1367,7 +1379,7 @@ fn_trie_lookup(struct fib_table *tb, con
 		bits = pn->bits;
 
 		if (!chopped_off)
-			cindex = tkey_extract_bits(mask_pfx(key, current_prefix_length),
+			cindex = tkey_extract_bits(mask_pfx (key, current_prefix_length),
 						   pos, bits);
 
 		n = tnode_get_child(pn, cindex);
@@ -1380,12 +1392,12 @@ fn_trie_lookup(struct fib_table *tb, con
 		}
 
 		if (IS_LEAF(n)) {
-			if ((ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res)) <= 0)
+			ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res);
+			if (ret <= 0)
 				goto found;
 			else
 				goto backtrace;
 		}
-
 #define HL_OPTIMIZE
 #ifdef HL_OPTIMIZE
 		cn = (struct tnode *)n;
@@ -1418,10 +1430,10 @@ fn_trie_lookup(struct fib_table *tb, con
 
 		/* NOTA BENE: CHECKING ONLY SKIPPED BITS FOR THE NEW NODE HERE */
 
-		if (current_prefix_length < pos+bits) {
+		if (current_prefix_length < pos + bits) {
 			if (tkey_extract_bits(cn->key, current_prefix_length,
-						cn->pos - current_prefix_length) != 0 ||
-			    !(cn->child[0]))
+					      cn->pos - current_prefix_length) != 0
+			    || !(cn->child[0]))
 				goto backtrace;
 		}
 
@@ -1456,19 +1468,19 @@ fn_trie_lookup(struct fib_table *tb, con
 
 		node_prefix = mask_pfx(cn->key, cn->pos);
 		key_prefix = mask_pfx(key, cn->pos);
-		pref_mismatch = key_prefix^node_prefix;
+		pref_mismatch = key_prefix ^ node_prefix;
 		mp = 0;
 
 		/* In short: If skipped bits in this node do not match the search
 		 * key, enter the "prefix matching" state.directly.
 		 */
 		if (pref_mismatch) {
-			while (!(pref_mismatch & (1<<(KEYLENGTH-1)))) {
+			while (!(pref_mismatch & (1 << (KEYLENGTH - 1)))) {
 				mp++;
-				pref_mismatch = pref_mismatch <<1;
+				pref_mismatch = pref_mismatch << 1;
 			}
-			key_prefix = tkey_extract_bits(cn->key, mp, cn->pos-mp);
 
+			key_prefix = tkey_extract_bits(cn->key, mp, cn->pos - mp);
 			if (key_prefix != 0)
 				goto backtrace;
 
@@ -1476,7 +1488,7 @@ fn_trie_lookup(struct fib_table *tb, con
 				current_prefix_length = mp;
 		}
 #endif
-		pn = (struct tnode *)n; /* Descend */
+		pn = (struct tnode *)n;	/* Descend */
 		chopped_off = 0;
 		continue;
 
@@ -1484,12 +1496,14 @@ backtrace:
 		chopped_off++;
 
 		/* As zero don't change the child key (cindex) */
-		while ((chopped_off <= pn->bits) && !(cindex & (1<<(chopped_off-1))))
+		while (chopped_off <= pn->bits
+		       && !(cindex & (1 << (chopped_off - 1))))
 			chopped_off++;
 
 		/* Decrease current_... with bits chopped off */
 		if (current_prefix_length > pn->pos + pn->bits - chopped_off)
-			current_prefix_length = pn->pos + pn->bits - chopped_off;
+			current_prefix_length =
+			    pn->pos + pn->bits - chopped_off;
 
 		/*
 		 * Either we do the actual chop off according or if we have
@@ -1497,14 +1511,15 @@ backtrace:
 		 */
 
 		if (chopped_off <= pn->bits) {
-			cindex &= ~(1 << (chopped_off-1));
+			cindex &= ~(1 << (chopped_off - 1));
 		} else {
-			struct tnode *parent = node_parent((struct node *) pn);
+			struct tnode *parent = node_parent((struct node *)pn);
 			if (!parent)
 				goto failed;
 
 			/* Get Child's index */
-			cindex = tkey_extract_bits(pn->key, parent->pos, parent->bits);
+			cindex = tkey_extract_bits(pn->key,
+						   parent->pos, parent->bits);
 			pn = parent;
 			chopped_off = 0;
 
@@ -1537,13 +1552,13 @@ static int trie_leaf_remove(struct trie 
 	 */
 
 	while (n != NULL && IS_TNODE(n)) {
-		struct tnode *tn = (struct tnode *) n;
+		struct tnode *tn = (struct tnode *)n;
 		check_tnode(tn);
-		n = tnode_get_child(tn ,tkey_extract_bits(key, tn->pos, tn->bits));
+		n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
 
 		BUG_ON(n && node_parent(n) != tn);
 	}
-	l = (struct leaf *) n;
+	l = (struct leaf *)n;
 
 	if (!n || !tkey_equals(l->key, key))
 		return 0;
@@ -1557,7 +1572,7 @@ static int trie_leaf_remove(struct trie 
 	t->size--;
 
 	tp = node_parent(n);
-	tnode_free((struct tnode *) n);
+	tnode_free((struct tnode *)n);
 
 	if (tp) {
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
@@ -1574,7 +1589,7 @@ static int trie_leaf_remove(struct trie 
  */
 static int fn_trie_delete(struct fib_table *tb, struct fib_config *cfg)
 {
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 	u32 key, mask;
 	int plen = cfg->fc_dst_len;
 	u8 tos = cfg->fc_tos;
@@ -1694,7 +1709,7 @@ static int trie_flush_leaf(struct trie *
 
 static struct leaf *nextleaf(struct trie *t, struct leaf *thisleaf)
 {
-	struct node *c = (struct node *) thisleaf;
+	struct node *c = (struct node *)thisleaf;
 	struct tnode *p;
 	int idx;
 	struct node *trie = rcu_dereference(t->trie);
@@ -1703,10 +1718,10 @@ static struct leaf *nextleaf(struct trie
 		if (trie == NULL)
 			return NULL;
 
-		if (IS_LEAF(trie))          /* trie w. just a leaf */
-			return (struct leaf *) trie;
+		if (IS_LEAF(trie))	/* trie w. just a leaf */
+			return (struct leaf *)trie;
 
-		p = (struct tnode*) trie;  /* Start */
+		p = (struct tnode *)trie;	/* Start */
 	} else
 		p = node_parent(c);
 
@@ -1720,7 +1735,7 @@ static struct leaf *nextleaf(struct trie
 			pos = 0;
 
 		last = 1 << p->bits;
-		for (idx = pos; idx < last ; idx++) {
+		for (idx = pos; idx < last; idx++) {
 			c = rcu_dereference(p->child[idx]);
 
 			if (!c)
@@ -1728,26 +1743,28 @@ static struct leaf *nextleaf(struct trie
 
 			/* Decend if tnode */
 			while (IS_TNODE(c)) {
-				p = (struct tnode *) c;
+				p = (struct tnode *)c;
 				idx = 0;
 
 				/* Rightmost non-NULL branch */
 				if (p && IS_TNODE(p))
 					while (!(c = rcu_dereference(p->child[idx]))
-					       && idx < (1<<p->bits)) idx++;
+					       && idx < (1 << p->bits))
+						idx++;
 
 				/* Done with this tnode? */
 				if (idx >= (1 << p->bits) || !c)
 					goto up;
 			}
-			return (struct leaf *) c;
+			return (struct leaf *)c;
 		}
 up:
 		/* No more children go up one step  */
-		c = (struct node *) p;
+		c = (struct node *)p;
 		p = node_parent(c);
 	}
-	return NULL; /* Ready. Root of trie */
+
+	return NULL;		/* Ready. Root of trie */
 }
 
 /*
@@ -1755,7 +1772,7 @@ up:
  */
 static int fn_trie_flush(struct fib_table *tb)
 {
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 	struct leaf *ll = NULL, *l = NULL;
 	int found = 0, h;
 
@@ -1779,9 +1796,10 @@ static int fn_trie_flush(struct fib_tabl
 static int trie_last_dflt = -1;
 
 static void
-fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
+fn_trie_select_default(struct fib_table *tb, const struct flowi *flp,
+		       struct fib_result *res)
 {
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 	int order, last_idx;
 	struct fib_info *fi = NULL;
 	struct fib_info *last_resort;
@@ -1809,8 +1827,7 @@ fn_trie_select_default(struct fib_table 
 	list_for_each_entry_rcu(fa, fa_head, fa_list) {
 		struct fib_info *next_fi = fa->fa_info;
 
-		if (fa->fa_scope != res->scope ||
-		    fa->fa_type != RTN_UNICAST)
+		if (fa->fa_scope != res->scope || fa->fa_type != RTN_UNICAST)
 			continue;
 
 		if (next_fi->fib_priority > res->fi->fib_priority)
@@ -1856,12 +1873,13 @@ fn_trie_select_default(struct fib_table 
 			atomic_inc(&last_resort->fib_clntref);
 	}
 	trie_last_dflt = last_idx;
- out:;
+out:
 	rcu_read_unlock();
 }
 
-static int fn_trie_dump_fa(t_key key, int plen, struct list_head *fah, struct fib_table *tb,
-			   struct sk_buff *skb, struct netlink_callback *cb)
+static int fn_trie_dump_fa(t_key key, int plen, struct list_head *fah,
+			   struct fib_table *tb, struct sk_buff *skb,
+			   struct netlink_callback *cb)
 {
 	int i, s_i;
 	struct fib_alias *fa;
@@ -1886,10 +1904,7 @@ static int fn_trie_dump_fa(t_key key, in
 				  tb->tb_id,
 				  fa->fa_type,
 				  fa->fa_scope,
-				  xkey,
-				  plen,
-				  fa->fa_tos,
-				  fa->fa_info, 0) < 0) {
+				  xkey, plen, fa->fa_tos, fa->fa_info, 0) < 0) {
 			cb->args[4] = i;
 			return -1;
 		}
@@ -1899,8 +1914,8 @@ static int fn_trie_dump_fa(t_key key, in
 	return skb->len;
 }
 
-static int fn_trie_dump_plen(struct trie *t, int plen, struct fib_table *tb, struct sk_buff *skb,
-			     struct netlink_callback *cb)
+static int fn_trie_dump_plen(struct trie *t, int plen, struct fib_table *tb,
+			     struct sk_buff *skb, struct netlink_callback *cb)
 {
 	int h, s_h;
 	struct list_head *fa_head;
@@ -1913,7 +1928,7 @@ static int fn_trie_dump_plen(struct trie
 			continue;
 		if (h > s_h)
 			memset(&cb->args[4], 0,
-			       sizeof(cb->args) - 4*sizeof(cb->args[0]));
+			       sizeof(cb->args) - 4 * sizeof(cb->args[0]));
 
 		fa_head = get_fa_head(l, plen);
 
@@ -1923,7 +1938,7 @@ static int fn_trie_dump_plen(struct trie
 		if (list_empty(fa_head))
 			continue;
 
-		if (fn_trie_dump_fa(l->key, plen, fa_head, tb, skb, cb)<0) {
+		if (fn_trie_dump_fa(l->key, plen, fa_head, tb, skb, cb) < 0) {
 			cb->args[3] = h;
 			return -1;
 		}
@@ -1932,10 +1947,11 @@ static int fn_trie_dump_plen(struct trie
 	return skb->len;
 }
 
-static int fn_trie_dump(struct fib_table *tb, struct sk_buff *skb, struct netlink_callback *cb)
+static int fn_trie_dump(struct fib_table *tb, struct sk_buff *skb,
+			struct netlink_callback *cb)
 {
 	int m, s_m;
-	struct trie *t = (struct trie *) tb->tb_data;
+	struct trie *t = (struct trie *)tb->tb_data;
 
 	s_m = cb->args[2];
 
@@ -1945,9 +1961,9 @@ static int fn_trie_dump(struct fib_table
 			continue;
 		if (m > s_m)
 			memset(&cb->args[3], 0,
-				sizeof(cb->args) - 3*sizeof(cb->args[0]));
+			       sizeof(cb->args) - 3 * sizeof(cb->args[0]));
 
-		if (fn_trie_dump_plen(t, 32-m, tb, skb, cb)<0) {
+		if (fn_trie_dump_plen(t, 32 - m, tb, skb, cb) < 0) {
 			cb->args[2] = m;
 			goto out;
 		}
@@ -1963,9 +1979,9 @@ out:
 /* Fix more generic FIB names for init later */
 
 #ifdef CONFIG_IP_MULTIPLE_TABLES
-struct fib_table * fib_hash_init(u32 id)
+struct fib_table *fib_hash_init(u32 id)
 #else
-struct fib_table * __init fib_hash_init(u32 id)
+struct fib_table *__init fib_hash_init(u32 id)
 #endif
 {
 	struct fib_table *tb;
@@ -1974,8 +1990,7 @@ struct fib_table * __init fib_hash_init(
 	if (fn_alias_kmem == NULL)
 		fn_alias_kmem = kmem_cache_create("ip_fib_alias",
 						  sizeof(struct fib_alias),
-						  0, SLAB_HWCACHE_ALIGN,
-						  NULL);
+						  0, SLAB_HWCACHE_ALIGN, NULL);
 
 	tb = kmalloc(sizeof(struct fib_table) + sizeof(struct trie),
 		     GFP_KERNEL);
@@ -1991,7 +2006,7 @@ struct fib_table * __init fib_hash_init(
 	tb->tb_dump = fn_trie_dump;
 	memset(tb->tb_data, 0, sizeof(struct trie));
 
-	t = (struct trie *) tb->tb_data;
+	t = (struct trie *)tb->tb_data;
 
 	trie_init(t);
 
@@ -2001,7 +2016,8 @@ struct fib_table * __init fib_hash_init(
 		trie_main = t;
 
 	if (id == RT_TABLE_LOCAL)
-		printk(KERN_INFO "IPv4 FIB: Using LC-trie version %s\n", VERSION);
+		printk(KERN_INFO "IPv4 FIB: Using LC-trie version %s\n",
+		       VERSION);
 
 	return tb;
 }
@@ -2028,7 +2044,7 @@ static struct node *fib_trie_get_next(st
 	pr_debug("get_next iter={node=%p index=%d depth=%d}\n",
 		 iter->tnode, iter->index, iter->depth);
 rescan:
-	while (cindex < (1<<tn->bits)) {
+	while (cindex < (1 << tn->bits)) {
 		struct node *n = tnode_get_child(tn, cindex);
 
 		if (n) {
@@ -2037,7 +2053,7 @@ rescan:
 				iter->index = cindex + 1;
 			} else {
 				/* push down one level */
-				iter->tnode = (struct tnode *) n;
+				iter->tnode = (struct tnode *)n;
 				iter->index = 0;
 				++iter->depth;
 			}
@@ -2050,7 +2066,7 @@ rescan:
 	/* Current node exhausted, pop back up */
 	p = node_parent((struct node *)tn);
 	if (p) {
-		cindex = tkey_extract_bits(tn->key, p->pos, p->bits)+1;
+		cindex = tkey_extract_bits(tn->key, p->pos, p->bits) + 1;
 		tn = p;
 		--iter->depth;
 		goto rescan;
@@ -2063,7 +2079,7 @@ rescan:
 static struct node *fib_trie_get_first(struct fib_trie_iter *iter,
 				       struct trie *t)
 {
-	struct node *n ;
+	struct node *n;
 
 	if (!t)
 		return NULL;
@@ -2075,13 +2091,13 @@ static struct node *fib_trie_get_first(s
 
 	if (n) {
 		if (IS_TNODE(n)) {
-			iter->tnode = (struct tnode *) n;
+			iter->tnode = (struct tnode *)n;
 			iter->trie = t;
 			iter->index = 0;
 			iter->depth = 1;
 		} else {
 			iter->tnode = NULL;
-			iter->trie  = t;
+			iter->trie = t;
 			iter->index = 0;
 			iter->depth = 0;
 		}
@@ -2098,22 +2114,21 @@ static void trie_collect_stats(struct tr
 	memset(s, 0, sizeof(*s));
 
 	rcu_read_lock();
-	for (n = fib_trie_get_first(&iter, t); n;
-	     n = fib_trie_get_next(&iter)) {
+	for (n = fib_trie_get_first(&iter, t); n; n = fib_trie_get_next(&iter)) {
 		if (IS_LEAF(n)) {
 			s->leaves++;
 			s->totdepth += iter.depth;
 			if (iter.depth > s->maxdepth)
 				s->maxdepth = iter.depth;
 		} else {
-			const struct tnode *tn = (const struct tnode *) n;
+			const struct tnode *tn = (const struct tnode *)n;
 			int i;
 
 			s->tnodes++;
 			if (tn->bits < MAX_STAT_DEPTH)
 				s->nodesizes[tn->bits]++;
 
-			for (i = 0; i < (1<<tn->bits); i++)
+			for (i = 0; i < (1 << tn->bits); i++)
 				if (!tn->child[i])
 					s->nullpointers++;
 		}
@@ -2129,11 +2144,12 @@ static void trie_show_stats(struct seq_f
 	unsigned i, max, pointers, bytes, avdepth;
 
 	if (stat->leaves)
-		avdepth = stat->totdepth*100 / stat->leaves;
+		avdepth = stat->totdepth * 100 / stat->leaves;
 	else
 		avdepth = 0;
 
-	seq_printf(seq, "\tAver depth:     %d.%02d\n", avdepth / 100, avdepth % 100 );
+	seq_printf(seq, "\tAver depth:     %d.%02d\n", avdepth / 100,
+		   avdepth % 100);
 	seq_printf(seq, "\tMax depth:      %u\n", stat->maxdepth);
 
 	seq_printf(seq, "\tLeaves:         %u\n", stat->leaves);
@@ -2143,14 +2159,14 @@ static void trie_show_stats(struct seq_f
 	bytes += sizeof(struct tnode) * stat->tnodes;
 
 	max = MAX_STAT_DEPTH;
-	while (max > 0 && stat->nodesizes[max-1] == 0)
+	while (max > 0 && stat->nodesizes[max - 1] == 0)
 		max--;
 
 	pointers = 0;
 	for (i = 1; i <= max; i++)
 		if (stat->nodesizes[i] != 0) {
-			seq_printf(seq, "  %d: %d",  i, stat->nodesizes[i]);
-			pointers += (1<<i) * stat->nodesizes[i];
+			seq_printf(seq, "  %d: %d", i, stat->nodesizes[i]);
+			pointers += (1 << i) * stat->nodesizes[i];
 		}
 	seq_putc(seq, '\n');
 	seq_printf(seq, "\tPointers: %d\n", pointers);
@@ -2161,12 +2177,15 @@ static void trie_show_stats(struct seq_f
 
 #ifdef CONFIG_IP_FIB_TRIE_STATS
 	seq_printf(seq, "Counters:\n---------\n");
-	seq_printf(seq,"gets = %d\n", t->stats.gets);
-	seq_printf(seq,"backtracks = %d\n", t->stats.backtrack);
-	seq_printf(seq,"semantic match passed = %d\n", t->stats.semantic_match_passed);
-	seq_printf(seq,"semantic match miss = %d\n", t->stats.semantic_match_miss);
-	seq_printf(seq,"null node hit= %d\n", t->stats.null_node_hit);
-	seq_printf(seq,"skipped node resize = %d\n", t->stats.resize_node_skipped);
+	seq_printf(seq, "gets = %d\n", t->stats.gets);
+	seq_printf(seq, "backtracks = %d\n", t->stats.backtrack);
+	seq_printf(seq, "semantic match passed = %d\n",
+		   t->stats.semantic_match_passed);
+	seq_printf(seq, "semantic match miss = %d\n",
+		   t->stats.semantic_match_miss);
+	seq_printf(seq, "null node hit= %d\n", t->stats.null_node_hit);
+	seq_printf(seq, "skipped node resize = %d\n",
+		   t->stats.resize_node_skipped);
 #ifdef CLEAR_STATS
 	memset(&(t->stats), 0, sizeof(t->stats));
 #endif
@@ -2181,7 +2200,8 @@ static int fib_triestat_seq_show(struct 
 	if (!stat)
 		return -ENOMEM;
 
-	seq_printf(seq, "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
+	seq_printf(seq,
+		   "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
 		   sizeof(struct leaf), sizeof(struct tnode));
 
 	if (trie_local) {
@@ -2213,8 +2233,7 @@ static const struct file_operations fib_
 	.release = single_release,
 };
 
-static struct node *fib_trie_get_idx(struct fib_trie_iter *iter,
-				      loff_t pos)
+static struct node *fib_trie_get_idx(struct fib_trie_iter *iter, loff_t pos)
 {
 	loff_t idx = 0;
 	struct node *n;
@@ -2233,7 +2252,7 @@ static struct node *fib_trie_get_idx(str
 	return NULL;
 }
 
-static void *fib_trie_seq_start(struct seq_file *seq, loff_t *pos)
+static void *fib_trie_seq_start(struct seq_file *seq, loff_t * pos)
 {
 	rcu_read_lock();
 	if (*pos == 0)
@@ -2241,7 +2260,7 @@ static void *fib_trie_seq_start(struct s
 	return fib_trie_get_idx(seq->private, *pos - 1);
 }
 
-static void *fib_trie_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+static void *fib_trie_seq_next(struct seq_file *seq, void *v, loff_t * pos)
 {
 	struct fib_trie_iter *iter = seq->private;
 	void *l = v;
@@ -2269,7 +2288,8 @@ static void fib_trie_seq_stop(struct seq
 
 static void seq_indent(struct seq_file *seq, int n)
 {
-	while (n-- > 0) seq_puts(seq, "   ");
+	while (n-- > 0)
+		seq_puts(seq, "   ");
 }
 
 static inline const char *rtn_scope(enum rt_scope_t s)
@@ -2277,11 +2297,16 @@ static inline const char *rtn_scope(enum
 	static char buf[32];
 
 	switch (s) {
-	case RT_SCOPE_UNIVERSE: return "universe";
-	case RT_SCOPE_SITE:	return "site";
-	case RT_SCOPE_LINK:	return "link";
-	case RT_SCOPE_HOST:	return "host";
-	case RT_SCOPE_NOWHERE:	return "nowhere";
+	case RT_SCOPE_UNIVERSE:
+		return "universe";
+	case RT_SCOPE_SITE:
+		return "site";
+	case RT_SCOPE_LINK:
+		return "link";
+	case RT_SCOPE_HOST:
+		return "host";
+	case RT_SCOPE_NOWHERE:
+		return "nowhere";
 	default:
 		snprintf(buf, sizeof(buf), "scope=%d", s);
 		return buf;
@@ -2330,16 +2355,16 @@ static int fib_trie_seq_show(struct seq_
 	}
 
 	if (IS_TNODE(n)) {
-		struct tnode *tn = (struct tnode *) n;
+		struct tnode *tn = (struct tnode *)n;
 		__be32 prf = htonl(mask_pfx(tn->key, tn->pos));
 
-		seq_indent(seq, iter->depth-1);
+		seq_indent(seq, iter->depth - 1);
 		seq_printf(seq, "  +-- %d.%d.%d.%d/%d %d %d %d\n",
 			   NIPQUAD(prf), tn->pos, tn->bits, tn->full_children,
 			   tn->empty_children);
 
 	} else {
-		struct leaf *l = (struct leaf *) n;
+		struct leaf *l = (struct leaf *)n;
 		int i;
 		__be32 val = htonl(l->key);
 
@@ -2350,7 +2375,7 @@ static int fib_trie_seq_show(struct seq_
 			if (li) {
 				struct fib_alias *fa;
 				list_for_each_entry_rcu(fa, &li->falh, fa_list) {
-					seq_indent(seq, iter->depth+1);
+					seq_indent(seq, iter->depth + 1);
 					seq_printf(seq, "  /%d %s %s", i,
 						   rtn_scope(fa->fa_scope),
 						   rtn_type(fa->fa_type));
@@ -2386,7 +2411,7 @@ static int fib_trie_seq_open(struct inod
 	if (rc)
 		goto out_kfree;
 
-	seq	     = file->private_data;
+	seq = file->private_data;
 	seq->private = s;
 	memset(s, 0, sizeof(*s));
 out:
@@ -2407,7 +2432,8 @@ static const struct file_operations fib_
 static unsigned fib_flag_trans(int type, __be32 mask, const struct fib_info *fi)
 {
 	static unsigned type2flags[RTN_MAX + 1] = {
-		[7] = RTF_REJECT, [8] = RTF_REJECT,
+		[7] = RTF_REJECT,
+		[8] = RTF_REJECT,
 	};
 	unsigned flags = type2flags[type];
 
@@ -2444,7 +2470,7 @@ static int fib_route_seq_show(struct seq
 	if (IS_TNODE(l))
 		return 0;
 
-	for (i=32; i>=0; i--) {
+	for (i = 32; i >= 0; i--) {
 		struct leaf_info *li = find_leaf_info(l, i);
 		struct fib_alias *fa;
 		__be32 mask, prefix;
@@ -2471,7 +2497,7 @@ static int fib_route_seq_show(struct seq
 					 fi->fib_nh->nh_gw, flags, 0, 0,
 					 fi->fib_priority,
 					 mask,
-					 (fi->fib_advmss ? fi->fib_advmss + 40 : 0),
+					 fi->fib_advmss ? fi->fib_advmss + 40 : 0,
 					 fi->fib_window,
 					 fi->fib_rtt >> 3);
 			else
@@ -2507,7 +2533,7 @@ static int fib_route_seq_open(struct ino
 	if (rc)
 		goto out_kfree;
 
-	seq	     = file->private_data;
+	seq = file->private_data;
 	seq->private = s;
 	memset(s, 0, sizeof(*s));
 out:

Re: [RFC] fib_trie: whitespace cleanup

[Paul E. McKenney]

On Thu, Jul 26, 2007 at 11:49:42AM +0100, Stephen Hemminger wrote:
> Whitespace cleanup run code through lindent then cleanup results.
> Applys after other two patches.
> 
> --- a/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
> +++ b/net/ipv4/fib_trie.c	2007-07-26 11:47:52.000000000 +0100
> @@ -156,7 +156,8 @@ struct trie {
>  };
> 
>  static void put_child(struct trie *t, struct tnode *tn, int i, struct node *n);
> -static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull);
> +static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
> +				  int wasfull);
>  static struct node *resize(struct trie *t, struct tnode *tn);
>  static struct tnode *inflate(struct trie *t, struct tnode *tn);
>  static struct tnode *halve(struct trie *t, struct tnode *tn);
> @@ -167,13 +168,12 @@ static struct trie *trie_local = NULL, *
> 
>  static inline struct tnode *node_parent(struct node *node)
>  {
> -	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
> +	return rcu_dereference((struct tnode *)(node->parent & ~NODE_TYPE_MASK));

The potential issue is applying rcu_dereference() to an rvalue
as opposed to an lvalue.  So how about the following?

	return rcu_dereference(node->parent) & ~NODE_TYPE_MASK;

>  }
> 
>  static inline void node_set_parent(struct node *node, struct tnode *ptr)
>  {
> -	rcu_assign_pointer(node->parent,
> -			   (unsigned long)ptr | NODE_TYPE(node));
> +	rcu_assign_pointer(node->parent, (unsigned long)ptr | NODE_TYPE(node));
>  }
> 
>  /* rcu_read_lock needs to be hold by caller from readside */
> @@ -192,13 +192,13 @@ static inline int tnode_child_length(con
> 
>  static inline t_key mask_pfx(t_key k, unsigned short l)
>  {
> -	return (l == 0) ? 0 : k >> (KEYLENGTH-l) << (KEYLENGTH-l);
> +	return (l == 0) ? 0 : k >> (KEYLENGTH - l) << (KEYLENGTH - l);
>  }
> 
>  static inline t_key tkey_extract_bits(t_key a, int offset, int bits)
>  {
>  	if (offset < KEYLENGTH)
> -		return ((t_key)(a << offset)) >> (KEYLENGTH - bits);
> +		return ((t_key) (a << offset)) >> (KEYLENGTH - bits);
>  	else
>  		return 0;
>  }
> @@ -223,7 +223,7 @@ static inline int tkey_mismatch(t_key a,
> 
>  	if (!diff)
>  		return 0;
> -	while ((diff << i) >> (KEYLENGTH-1) == 0)
> +	while ((diff << i) >> (KEYLENGTH - 1) == 0)
>  		i++;
>  	return i;
>  }
> @@ -285,7 +285,6 @@ static inline int tkey_mismatch(t_key a,
>    The bits from (n->pos) to (n->pos + n->bits - 1) - "C" - are the index into
>    n's child array, and will of course be different for each child.
> 
> -
>    The rest of the bits, from (n->pos + n->bits) onward, are completely unknown
>    at this point.
> 
> @@ -293,7 +292,7 @@ static inline int tkey_mismatch(t_key a,
> 
>  static inline void check_tnode(const struct tnode *tn)
>  {
> -	WARN_ON(tn && tn->pos+tn->bits > 32);
> +	WARN_ON(tn && tn->pos + tn->bits > 32);
>  }
> 
>  static int halve_threshold = 25;
> @@ -301,7 +300,6 @@ static int inflate_threshold = 50;
>  static int halve_threshold_root = 8;
>  static int inflate_threshold_root = 15;
> 
> -
>  static void __alias_free_mem(struct rcu_head *head)
>  {
>  	struct fib_alias *fa = container_of(head, struct fib_alias, rcu);
> @@ -335,7 +333,7 @@ static struct tnode *tnode_alloc(unsigne
>  	if (size <= PAGE_SIZE)
>  		return kcalloc(size, 1, GFP_KERNEL);
> 
> -	pages = alloc_pages(GFP_KERNEL|__GFP_ZERO, get_order(size));
> +	pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(size));
>  	if (!pages)
>  		return NULL;
> 
> @@ -346,7 +344,7 @@ static void __tnode_free_rcu(struct rcu_
>  {
>  	struct tnode *tn = container_of(head, struct tnode, rcu);
>  	unsigned int size = sizeof(struct tnode) +
> -		(1 << tn->bits) * sizeof(struct node *);
> +	    (1 << tn->bits) * sizeof(struct node *);
> 
>  	if (size <= PAGE_SIZE)
>  		kfree(tn);
> @@ -357,7 +355,7 @@ static void __tnode_free_rcu(struct rcu_
>  static inline void tnode_free(struct tnode *tn)
>  {
>  	if (IS_LEAF(tn)) {
> -		struct leaf *l = (struct leaf *) tn;
> +		struct leaf *l = (struct leaf *)tn;
>  		call_rcu_bh(&l->rcu, __leaf_free_rcu);
>  	} else
>  		call_rcu(&tn->rcu, __tnode_free_rcu);
> @@ -365,7 +363,7 @@ static inline void tnode_free(struct tno
> 
>  static struct leaf *leaf_new(void)
>  {
> -	struct leaf *l = kmalloc(sizeof(struct leaf),  GFP_KERNEL);
> +	struct leaf *l = kmalloc(sizeof(struct leaf), GFP_KERNEL);
>  	if (l) {
>  		l->parent = T_LEAF;
>  		INIT_HLIST_HEAD(&l->list);
> @@ -375,7 +373,7 @@ static struct leaf *leaf_new(void)
> 
>  static struct leaf_info *leaf_info_new(int plen)
>  {
> -	struct leaf_info *li = kmalloc(sizeof(struct leaf_info),  GFP_KERNEL);
> +	struct leaf_info *li = kmalloc(sizeof(struct leaf_info), GFP_KERNEL);
>  	if (li) {
>  		li->plen = plen;
>  		INIT_LIST_HEAD(&li->falh);
> @@ -383,9 +381,9 @@ static struct leaf_info *leaf_info_new(i
>  	return li;
>  }
> 
> -static struct tnode* tnode_new(t_key key, int pos, int bits)
> +static struct tnode *tnode_new(t_key key, int pos, int bits)
>  {
> -	int nchildren = 1<<bits;
> +	int nchildren = 1 << bits;
>  	int sz = sizeof(struct tnode) + nchildren * sizeof(struct node *);
>  	struct tnode *tn = tnode_alloc(sz);
> 
> @@ -396,11 +394,11 @@ static struct tnode* tnode_new(t_key key
>  		tn->bits = bits;
>  		tn->key = key;
>  		tn->full_children = 0;
> -		tn->empty_children = 1<<bits;
> +		tn->empty_children = 1 << bits;
>  	}
> 
> -	pr_debug("AT %p s=%u %u\n", tn, (unsigned int) sizeof(struct tnode),
> -		 (unsigned int) (sizeof(struct node) * 1<<bits));
> +	pr_debug("AT %p s=%u %u\n", tn, (unsigned int)sizeof(struct tnode),
> +		 (unsigned int)(sizeof(struct node) * 1 << bits));
>  	return tn;
>  }
> 
> @@ -414,10 +412,11 @@ static inline int tnode_full(const struc
>  	if (n == NULL || IS_LEAF(n))
>  		return 0;
> 
> -	return ((struct tnode *) n)->pos == tn->pos + tn->bits;
> +	return ((struct tnode *)n)->pos == tn->pos + tn->bits;
>  }
> 
> -static inline void put_child(struct trie *t, struct tnode *tn, int i, struct node *n)
> +static inline void put_child(struct trie *t, struct tnode *tn, int i,
> +			     struct node *n)
>  {
>  	tnode_put_child_reorg(tn, i, n, -1);
>  }
> @@ -427,13 +426,13 @@ static inline void put_child(struct trie
>    * Update the value of full_children and empty_children.
>    */
> 
> -static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull)
> +static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
> +				  int wasfull)
>  {
>  	struct node *chi = tn->child[i];
>  	int isfull;
> 
> -	BUG_ON(i >= 1<<tn->bits);
> -
> +	BUG_ON(i >= 1 << tn->bits);
> 
>  	/* update emptyChildren */
>  	if (n == NULL && chi != NULL)
> @@ -566,9 +565,10 @@ static struct node *resize(struct trie *
> 
>  	err = 0;
>  	max_resize = 10;
> -	while ((tn->full_children > 0 &&  max_resize-- &&
> -	       50 * (tn->full_children + tnode_child_length(tn) - tn->empty_children) >=
> -				inflate_threshold_use * tnode_child_length(tn))) {
> +	while ((tn->full_children > 0 && max_resize-- &&
> +		50 * (tn->full_children + tnode_child_length(tn) -
> +		      tn->empty_children) >=
> +		inflate_threshold_use * tnode_child_length(tn))) {
> 
>  		old_tn = tn;
>  		tn = inflate(t, tn);
> @@ -583,10 +583,12 @@ static struct node *resize(struct trie *
> 
>  	if (max_resize < 0) {
>  		if (!tn->parent)
> -			printk(KERN_WARNING "Fix inflate_threshold_root. Now=%d size=%d bits\n",
> +			printk(KERN_WARNING
> +			       "Fix inflate_threshold_root. Now=%d size=%d bits\n",
>  			       inflate_threshold_root, tn->bits);
>  		else
> -			printk(KERN_WARNING "Fix inflate_threshold. Now=%d size=%d bits\n",
> +			printk(KERN_WARNING
> +			       "Fix inflate_threshold. Now=%d size=%d bits\n",
>  			       inflate_threshold, tn->bits);
>  	}
> 
> @@ -597,7 +599,6 @@ static struct node *resize(struct trie *
>  	 * node is above threshold.
>  	 */
> 
> -
>  	/* Keep root node larger  */
> 
>  	if (!tn->parent)
> @@ -607,7 +608,7 @@ static struct node *resize(struct trie *
> 
>  	err = 0;
>  	max_resize = 10;
> -	while (tn->bits > 1 &&  max_resize-- &&
> +	while (tn->bits > 1 && max_resize-- &&
>  	       100 * (tnode_child_length(tn) - tn->empty_children) <
>  	       halve_threshold_use * tnode_child_length(tn)) {
> 
> @@ -624,10 +625,12 @@ static struct node *resize(struct trie *
> 
>  	if (max_resize < 0) {
>  		if (!tn->parent)
> -			printk(KERN_WARNING "Fix halve_threshold_root. Now=%d size=%d bits\n",
> +			printk(KERN_WARNING
> +			       "Fix halve_threshold_root. Now=%d size=%d bits\n",
>  			       halve_threshold_root, tn->bits);
>  		else
> -			printk(KERN_WARNING "Fix halve_threshold. Now=%d size=%d bits\n",
> +			printk(KERN_WARNING
> +			       "Fix halve_threshold. Now=%d size=%d bits\n",
>  			       halve_threshold, tn->bits);
>  	}
> 
> @@ -647,7 +650,7 @@ static struct node *resize(struct trie *
>  			return n;
>  		}
> 
> -	return (struct node *) tn;
> +	return (struct node *)tn;
>  }
> 
>  static struct tnode *inflate(struct trie *t, struct tnode *tn)
> @@ -672,7 +675,8 @@ static struct tnode *inflate(struct trie
>  	 */
> 
>  	for (i = 0; i < olen; i++) {
> -		struct tnode *inode = (struct tnode *) tnode_get_child(oldtnode, i);
> +		struct tnode *inode =
> +		    (struct tnode *)tnode_get_child(oldtnode, i);
> 
>  		if (inode &&
>  		    IS_TNODE(inode) &&
> @@ -681,12 +685,12 @@ static struct tnode *inflate(struct trie
>  			struct tnode *left, *right;
>  			t_key m = ~0U << (KEYLENGTH - 1) >> inode->pos;
> 
> -			left = tnode_new(inode->key&(~m), inode->pos + 1,
> +			left = tnode_new(inode->key & (~m), inode->pos + 1,
>  					 inode->bits - 1);
>  			if (!left)
>  				goto nomem;
> 
> -			right = tnode_new(inode->key|m, inode->pos + 1,
> +			right = tnode_new(inode->key | m, inode->pos + 1,
>  					  inode->bits - 1);
> 
>  			if (!right) {
> @@ -694,8 +698,8 @@ static struct tnode *inflate(struct trie
>  				goto nomem;
>  			}
> 
> -			put_child(t, tn, 2*i, (struct node *) left);
> -			put_child(t, tn, 2*i+1, (struct node *) right);
> +			put_child(t, tn, 2 * i, (struct node *)left);
> +			put_child(t, tn, 2 * i + 1, (struct node *)right);
>  		}
>  	}
> 
> @@ -710,22 +714,22 @@ static struct tnode *inflate(struct trie
> 
>  		/* A leaf or an internal node with skipped bits */
> 
> -		if (IS_LEAF(node) || ((struct tnode *) node)->pos >
> -		   tn->pos + tn->bits - 1) {
> -			if (tkey_extract_bits(node->key, oldtnode->pos + oldtnode->bits,
> -					     1) == 0)
> -				put_child(t, tn, 2*i, node);
> +		if (IS_LEAF(node) || ((struct tnode *)node)->pos >
> +		    tn->pos + tn->bits - 1) {
> +			if (!tkey_extract_bits(node->key,
> +					      oldtnode->pos + oldtnode->bits, 1))
> +				put_child(t, tn, 2 * i, node);
>  			else
> -				put_child(t, tn, 2*i+1, node);
> +				put_child(t, tn, 2 * i + 1, node);
>  			continue;
>  		}
> 
>  		/* An internal node with two children */
> -		inode = (struct tnode *) node;
> +		inode = (struct tnode *)node;
> 
>  		if (inode->bits == 1) {
> -			put_child(t, tn, 2*i, inode->child[0]);
> -			put_child(t, tn, 2*i+1, inode->child[1]);
> +			put_child(t, tn, 2 * i, inode->child[0]);
> +			put_child(t, tn, 2 * i + 1, inode->child[1]);
> 
>  			tnode_free(inode);
>  			continue;
> @@ -754,13 +758,13 @@ static struct tnode *inflate(struct trie
>  		 *   bit to zero.
>  		 */
> 
> -		left = (struct tnode *) tnode_get_child(tn, 2*i);
> -		put_child(t, tn, 2*i, NULL);
> +		left = (struct tnode *)tnode_get_child(tn, 2 * i);
> +		put_child(t, tn, 2 * i, NULL);
> 
>  		BUG_ON(!left);
> 
> -		right = (struct tnode *) tnode_get_child(tn, 2*i+1);
> -		put_child(t, tn, 2*i+1, NULL);
> +		right = (struct tnode *)tnode_get_child(tn, 2 * i + 1);
> +		put_child(t, tn, 2 * i + 1, NULL);
> 
>  		BUG_ON(!right);
> 
> @@ -769,8 +773,8 @@ static struct tnode *inflate(struct trie
>  			put_child(t, left, j, inode->child[j]);
>  			put_child(t, right, j, inode->child[j + size]);
>  		}
> -		put_child(t, tn, 2*i, resize(t, left));
> -		put_child(t, tn, 2*i+1, resize(t, right));
> +		put_child(t, tn, 2 * i, resize(t, left));
> +		put_child(t, tn, 2 * i + 1, resize(t, right));
> 
>  		tnode_free(inode);
>  	}
> @@ -814,7 +818,7 @@ static struct tnode *halve(struct trie *
> 
>  	for (i = 0; i < olen; i += 2) {
>  		left = tnode_get_child(oldtnode, i);
> -		right = tnode_get_child(oldtnode, i+1);
> +		right = tnode_get_child(oldtnode, i + 1);
> 
>  		/* Two nonempty children */
>  		if (left && right) {
> @@ -825,7 +829,7 @@ static struct tnode *halve(struct trie *
>  			if (!newn)
>  				goto nomem;
> 
> -			put_child(t, tn, i/2, (struct node *)newn);
> +			put_child(t, tn, i / 2, (struct node *)newn);
>  		}
> 
>  	}
> @@ -834,27 +838,27 @@ static struct tnode *halve(struct trie *
>  		struct tnode *newBinNode;
> 
>  		left = tnode_get_child(oldtnode, i);
> -		right = tnode_get_child(oldtnode, i+1);
> +		right = tnode_get_child(oldtnode, i + 1);
> 
>  		/* At least one of the children is empty */
>  		if (left == NULL) {
> -			if (right == NULL)    /* Both are empty */
> +			if (right == NULL)	/* Both are empty */
>  				continue;
> -			put_child(t, tn, i/2, right);
> +			put_child(t, tn, i / 2, right);
>  			continue;
>  		}
> 
>  		if (right == NULL) {
> -			put_child(t, tn, i/2, left);
> +			put_child(t, tn, i / 2, left);
>  			continue;
>  		}
> 
>  		/* Two nonempty children */
> -		newBinNode = (struct tnode *) tnode_get_child(tn, i/2);
> -		put_child(t, tn, i/2, NULL);
> +		newBinNode = (struct tnode *)tnode_get_child(tn, i / 2);
> +		put_child(t, tn, i / 2, NULL);
>  		put_child(t, newBinNode, 0, left);
>  		put_child(t, newBinNode, 1, right);
> -		put_child(t, tn, i/2, resize(t, newBinNode));
> +		put_child(t, tn, i / 2, resize(t, newBinNode));
>  	}
>  	tnode_free(oldtnode);
>  	return tn;
> @@ -896,13 +900,13 @@ static struct leaf_info *find_leaf_info(
>  	struct leaf_info *li;
> 
>  	hlist_for_each_entry_rcu(li, node, head, hlist)
> -		if (li->plen == plen)
> -			return li;
> +	    if (li->plen == plen)
> +		return li;
> 
>  	return NULL;
>  }
> 
> -static inline struct list_head * get_fa_head(struct leaf *l, int plen)
> +static inline struct list_head *get_fa_head(struct leaf *l, int plen)
>  {
>  	struct leaf_info *li = find_leaf_info(l, plen);
> 
> @@ -935,8 +939,7 @@ static void insert_leaf_info(struct hlis
> 
>  /* rcu_read_lock needs to be hold by caller from readside */
> 
> -static struct leaf *
> -fib_find_node(struct trie *t, u32 key)
> +static struct leaf *fib_find_node(struct trie *t, u32 key)
>  {
>  	int pos;
>  	struct tnode *tn;
> @@ -945,14 +948,16 @@ fib_find_node(struct trie *t, u32 key)
>  	pos = 0;
>  	n = rcu_dereference(t->trie);
> 
> -	while (n != NULL &&  NODE_TYPE(n) == T_TNODE) {
> -		tn = (struct tnode *) n;
> +	while (n != NULL && NODE_TYPE(n) == T_TNODE) {
> +		tn = (struct tnode *)n;
> 
>  		check_tnode(tn);
> 
> -		if (tkey_sub_equals(tn->key, pos, tn->pos-pos, key)) {
> +		if (tkey_sub_equals(tn->key, pos, tn->pos - pos, key)) {
>  			pos = tn->pos + tn->bits;
> -			n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
> +			n = tnode_get_child(tn,
> +					    tkey_extract_bits(key, tn->pos,
> +							      tn->bits));
>  		} else
>  			break;
>  	}
> @@ -973,10 +978,11 @@ static struct node *trie_rebalance(struc
>  	while (tn != NULL && (tp = node_parent((struct node *)tn)) != NULL) {
>  		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
>  		wasfull = tnode_full(tp, tnode_get_child(tp, cindex));
> -		tn = (struct tnode *) resize (t, (struct tnode *)tn);
> -		tnode_put_child_reorg((struct tnode *)tp, cindex,(struct node*)tn, wasfull);
> +		tn = (struct tnode *)resize(t, (struct tnode *)tn);
> +		tnode_put_child_reorg((struct tnode *)tp, cindex,
> +				      (struct node *)tn, wasfull);
> 
> -		tp = node_parent((struct node *) tn);
> +		tp = node_parent((struct node *)tn);
>  		if (!tp)
>  			break;
>  		tn = tp;
> @@ -984,15 +990,15 @@ static struct node *trie_rebalance(struc
> 
>  	/* Handle last (top) tnode */
>  	if (IS_TNODE(tn))
> -		tn = (struct tnode*) resize(t, (struct tnode *)tn);
> +		tn = (struct tnode *)resize(t, (struct tnode *)tn);
> 
> -	return (struct node*) tn;
> +	return (struct node *)tn;
>  }
> 
>  /* only used from updater-side */
> 
> -static  struct list_head *
> -fib_insert_node(struct trie *t, int *err, u32 key, int plen)
> +static struct list_head *fib_insert_node(struct trie *t, int *err, u32 key,
> +					 int plen)
>  {
>  	int pos, newpos;
>  	struct tnode *tp = NULL, *tn = NULL;
> @@ -1024,15 +1030,17 @@ fib_insert_node(struct trie *t, int *err
>  	 * If it doesn't, we need to replace it with a T_TNODE.
>  	 */
> 
> -	while (n != NULL &&  NODE_TYPE(n) == T_TNODE) {
> -		tn = (struct tnode *) n;
> +	while (n != NULL && NODE_TYPE(n) == T_TNODE) {
> +		tn = (struct tnode *)n;
> 
>  		check_tnode(tn);
> 
> -		if (tkey_sub_equals(tn->key, pos, tn->pos-pos, key)) {
> +		if (tkey_sub_equals(tn->key, pos, tn->pos - pos, key)) {
>  			tp = tn;
>  			pos = tn->pos + tn->bits;
> -			n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
> +			n = tnode_get_child(tn,
> +					    tkey_extract_bits(key, tn->pos,
> +							      tn->bits));
> 
>  			BUG_ON(n && node_parent(n) != tn);
>  		} else
> @@ -1050,7 +1058,7 @@ fib_insert_node(struct trie *t, int *err
>  	/* Case 1: n is a leaf. Compare prefixes */
> 
>  	if (n != NULL && IS_LEAF(n) && tkey_equals(key, n->key)) {
> -		struct leaf *l = (struct leaf *) n;
> +		struct leaf *l = (struct leaf *)n;
> 
>  		li = leaf_info_new(plen);
> 
> @@ -1075,7 +1083,7 @@ fib_insert_node(struct trie *t, int *err
>  	li = leaf_info_new(plen);
> 
>  	if (!li) {
> -		tnode_free((struct tnode *) l);
> +		tnode_free((struct tnode *)l);
>  		*err = -ENOMEM;
>  		goto err;
>  	}
> @@ -1098,7 +1106,7 @@ fib_insert_node(struct trie *t, int *err
>  		 */
> 
>  		if (tp)
> -			pos = tp->pos+tp->bits;
> +			pos = tp->pos + tp->bits;
>  		else
>  			pos = 0;
> 
> @@ -1107,12 +1115,12 @@ fib_insert_node(struct trie *t, int *err
>  			tn = tnode_new(n->key, newpos, 1);
>  		} else {
>  			newpos = 0;
> -			tn = tnode_new(key, newpos, 1); /* First tnode */
> +			tn = tnode_new(key, newpos, 1);	/* First tnode */
>  		}
> 
>  		if (!tn) {
>  			free_leaf_info(li);
> -			tnode_free((struct tnode *) l);
> +			tnode_free((struct tnode *)l);
>  			*err = -ENOMEM;
>  			goto err;
>  		}
> @@ -1121,20 +1129,22 @@ fib_insert_node(struct trie *t, int *err
> 
>  		missbit = tkey_extract_bits(key, newpos, 1);
>  		put_child(t, tn, missbit, (struct node *)l);
> -		put_child(t, tn, 1-missbit, n);
> +		put_child(t, tn, 1 - missbit, n);
> 
>  		if (tp) {
>  			cindex = tkey_extract_bits(key, tp->pos, tp->bits);
> -			put_child(t, (struct tnode *)tp, cindex, (struct node *)tn);
> +			put_child(t, (struct tnode *)tp, cindex,
> +				  (struct node *)tn);
>  		} else {
> -			rcu_assign_pointer(t->trie, (struct node *)tn); /* First tnode */
> +			rcu_assign_pointer(t->trie, (struct node *)tn);	/* First tnode */
>  			tp = tn;
>  		}
>  	}
> 
>  	if (tp && tp->pos + tp->bits > 32)
> -		printk(KERN_WARNING "fib_trie tp=%p pos=%d, bits=%d, key=%0x plen=%d\n",
> -		       tp, tp->pos, tp->bits, key, plen);
> +		printk(KERN_WARNING
> +		       "fib_trie tp=%p pos=%d, bits=%d, key=%0x plen=%d\n", tp,
> +		       tp->pos, tp->bits, key, plen);
> 
>  	/* Rebalance the trie */
> 
> @@ -1150,7 +1160,7 @@ err:
>   */
>  static int fn_trie_insert(struct fib_table *tb, struct fib_config *cfg)
>  {
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
>  	struct fib_alias *fa, *new_fa;
>  	struct list_head *fa_head = NULL;
>  	struct fib_info *fi;
> @@ -1230,7 +1240,7 @@ static int fn_trie_insert(struct fib_tab
>  			if (state & FA_S_ACCESSED)
>  				rt_cache_flush(-1);
>  			rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen,
> -				tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
> +				  tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
> 
>  			goto succeeded;
>  		}
> @@ -1278,8 +1288,7 @@ static int fn_trie_insert(struct fib_tab
>  			goto out_free_new_fa;
>  	}
> 
> -	list_add_tail_rcu(&new_fa->fa_list,
> -			  (fa ? &fa->fa_list : fa_head));
> +	list_add_tail_rcu(&new_fa->fa_list, (fa ? &fa->fa_list : fa_head));
> 
>  	rt_cache_flush(-1);
>  	rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen, tb->tb_id,
> @@ -1295,7 +1304,6 @@ err:
>  	return err;
>  }
> 
> -
>  /* should be called with rcu_read_lock */
>  static inline int check_leaf(struct trie *t, struct leaf *l,
>  			     t_key key, int *plen, const struct flowi *flp,
> @@ -1313,7 +1321,9 @@ static inline int check_leaf(struct trie
>  		if (l->key != (key & ntohl(mask)))
>  			continue;
> 
> -		if ((err = fib_semantic_match(&li->falh, flp, res, htonl(l->key), mask, i)) <= 0) {
> +		err = fib_semantic_match(&li->falh, flp, res, htonl(l->key),
> +					 mask, i);
> +		if (err <= 0) {
>  			*plen = i;
>  #ifdef CONFIG_IP_FIB_TRIE_STATS
>  			t->stats.semantic_match_passed++;
> @@ -1328,9 +1338,10 @@ static inline int check_leaf(struct trie
>  }
> 
>  static int
> -fn_trie_lookup(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
> +fn_trie_lookup(struct fib_table *tb, const struct flowi *flp,
> +	       struct fib_result *res)
>  {
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
>  	int plen, ret = 0;
>  	struct node *n;
>  	struct tnode *pn;
> @@ -1355,11 +1366,12 @@ fn_trie_lookup(struct fib_table *tb, con
> 
>  	/* Just a leaf? */
>  	if (IS_LEAF(n)) {
> -		if ((ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res)) <= 0)
> +		ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res);
> +		if (ret <= 0)
>  			goto found;
>  		goto failed;
>  	}
> -	pn = (struct tnode *) n;
> +	pn = (struct tnode *)n;
>  	chopped_off = 0;
> 
>  	while (pn) {
> @@ -1367,7 +1379,7 @@ fn_trie_lookup(struct fib_table *tb, con
>  		bits = pn->bits;
> 
>  		if (!chopped_off)
> -			cindex = tkey_extract_bits(mask_pfx(key, current_prefix_length),
> +			cindex = tkey_extract_bits(mask_pfx (key, current_prefix_length),
>  						   pos, bits);
> 
>  		n = tnode_get_child(pn, cindex);
> @@ -1380,12 +1392,12 @@ fn_trie_lookup(struct fib_table *tb, con
>  		}
> 
>  		if (IS_LEAF(n)) {
> -			if ((ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res)) <= 0)
> +			ret = check_leaf(t, (struct leaf *)n, key, &plen, flp, res);
> +			if (ret <= 0)
>  				goto found;
>  			else
>  				goto backtrace;
>  		}
> -
>  #define HL_OPTIMIZE
>  #ifdef HL_OPTIMIZE
>  		cn = (struct tnode *)n;
> @@ -1418,10 +1430,10 @@ fn_trie_lookup(struct fib_table *tb, con
> 
>  		/* NOTA BENE: CHECKING ONLY SKIPPED BITS FOR THE NEW NODE HERE */
> 
> -		if (current_prefix_length < pos+bits) {
> +		if (current_prefix_length < pos + bits) {
>  			if (tkey_extract_bits(cn->key, current_prefix_length,
> -						cn->pos - current_prefix_length) != 0 ||
> -			    !(cn->child[0]))
> +					      cn->pos - current_prefix_length) != 0
> +			    || !(cn->child[0]))
>  				goto backtrace;
>  		}
> 
> @@ -1456,19 +1468,19 @@ fn_trie_lookup(struct fib_table *tb, con
> 
>  		node_prefix = mask_pfx(cn->key, cn->pos);
>  		key_prefix = mask_pfx(key, cn->pos);
> -		pref_mismatch = key_prefix^node_prefix;
> +		pref_mismatch = key_prefix ^ node_prefix;
>  		mp = 0;
> 
>  		/* In short: If skipped bits in this node do not match the search
>  		 * key, enter the "prefix matching" state.directly.
>  		 */
>  		if (pref_mismatch) {
> -			while (!(pref_mismatch & (1<<(KEYLENGTH-1)))) {
> +			while (!(pref_mismatch & (1 << (KEYLENGTH - 1)))) {
>  				mp++;
> -				pref_mismatch = pref_mismatch <<1;
> +				pref_mismatch = pref_mismatch << 1;
>  			}
> -			key_prefix = tkey_extract_bits(cn->key, mp, cn->pos-mp);
> 
> +			key_prefix = tkey_extract_bits(cn->key, mp, cn->pos - mp);
>  			if (key_prefix != 0)
>  				goto backtrace;
> 
> @@ -1476,7 +1488,7 @@ fn_trie_lookup(struct fib_table *tb, con
>  				current_prefix_length = mp;
>  		}
>  #endif
> -		pn = (struct tnode *)n; /* Descend */
> +		pn = (struct tnode *)n;	/* Descend */
>  		chopped_off = 0;
>  		continue;
> 
> @@ -1484,12 +1496,14 @@ backtrace:
>  		chopped_off++;
> 
>  		/* As zero don't change the child key (cindex) */
> -		while ((chopped_off <= pn->bits) && !(cindex & (1<<(chopped_off-1))))
> +		while (chopped_off <= pn->bits
> +		       && !(cindex & (1 << (chopped_off - 1))))
>  			chopped_off++;
> 
>  		/* Decrease current_... with bits chopped off */
>  		if (current_prefix_length > pn->pos + pn->bits - chopped_off)
> -			current_prefix_length = pn->pos + pn->bits - chopped_off;
> +			current_prefix_length =
> +			    pn->pos + pn->bits - chopped_off;
> 
>  		/*
>  		 * Either we do the actual chop off according or if we have
> @@ -1497,14 +1511,15 @@ backtrace:
>  		 */
> 
>  		if (chopped_off <= pn->bits) {
> -			cindex &= ~(1 << (chopped_off-1));
> +			cindex &= ~(1 << (chopped_off - 1));
>  		} else {
> -			struct tnode *parent = node_parent((struct node *) pn);
> +			struct tnode *parent = node_parent((struct node *)pn);
>  			if (!parent)
>  				goto failed;
> 
>  			/* Get Child's index */
> -			cindex = tkey_extract_bits(pn->key, parent->pos, parent->bits);
> +			cindex = tkey_extract_bits(pn->key,
> +						   parent->pos, parent->bits);
>  			pn = parent;
>  			chopped_off = 0;
> 
> @@ -1537,13 +1552,13 @@ static int trie_leaf_remove(struct trie 
>  	 */
> 
>  	while (n != NULL && IS_TNODE(n)) {
> -		struct tnode *tn = (struct tnode *) n;
> +		struct tnode *tn = (struct tnode *)n;
>  		check_tnode(tn);
> -		n = tnode_get_child(tn ,tkey_extract_bits(key, tn->pos, tn->bits));
> +		n = tnode_get_child(tn, tkey_extract_bits(key, tn->pos, tn->bits));
> 
>  		BUG_ON(n && node_parent(n) != tn);
>  	}
> -	l = (struct leaf *) n;
> +	l = (struct leaf *)n;
> 
>  	if (!n || !tkey_equals(l->key, key))
>  		return 0;
> @@ -1557,7 +1572,7 @@ static int trie_leaf_remove(struct trie 
>  	t->size--;
> 
>  	tp = node_parent(n);
> -	tnode_free((struct tnode *) n);
> +	tnode_free((struct tnode *)n);
> 
>  	if (tp) {
>  		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
> @@ -1574,7 +1589,7 @@ static int trie_leaf_remove(struct trie 
>   */
>  static int fn_trie_delete(struct fib_table *tb, struct fib_config *cfg)
>  {
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
>  	u32 key, mask;
>  	int plen = cfg->fc_dst_len;
>  	u8 tos = cfg->fc_tos;
> @@ -1694,7 +1709,7 @@ static int trie_flush_leaf(struct trie *
> 
>  static struct leaf *nextleaf(struct trie *t, struct leaf *thisleaf)
>  {
> -	struct node *c = (struct node *) thisleaf;
> +	struct node *c = (struct node *)thisleaf;
>  	struct tnode *p;
>  	int idx;
>  	struct node *trie = rcu_dereference(t->trie);
> @@ -1703,10 +1718,10 @@ static struct leaf *nextleaf(struct trie
>  		if (trie == NULL)
>  			return NULL;
> 
> -		if (IS_LEAF(trie))          /* trie w. just a leaf */
> -			return (struct leaf *) trie;
> +		if (IS_LEAF(trie))	/* trie w. just a leaf */
> +			return (struct leaf *)trie;
> 
> -		p = (struct tnode*) trie;  /* Start */
> +		p = (struct tnode *)trie;	/* Start */
>  	} else
>  		p = node_parent(c);
> 
> @@ -1720,7 +1735,7 @@ static struct leaf *nextleaf(struct trie
>  			pos = 0;
> 
>  		last = 1 << p->bits;
> -		for (idx = pos; idx < last ; idx++) {
> +		for (idx = pos; idx < last; idx++) {
>  			c = rcu_dereference(p->child[idx]);
> 
>  			if (!c)
> @@ -1728,26 +1743,28 @@ static struct leaf *nextleaf(struct trie
> 
>  			/* Decend if tnode */
>  			while (IS_TNODE(c)) {
> -				p = (struct tnode *) c;
> +				p = (struct tnode *)c;
>  				idx = 0;
> 
>  				/* Rightmost non-NULL branch */
>  				if (p && IS_TNODE(p))
>  					while (!(c = rcu_dereference(p->child[idx]))
> -					       && idx < (1<<p->bits)) idx++;
> +					       && idx < (1 << p->bits))
> +						idx++;
> 
>  				/* Done with this tnode? */
>  				if (idx >= (1 << p->bits) || !c)
>  					goto up;
>  			}
> -			return (struct leaf *) c;
> +			return (struct leaf *)c;
>  		}
>  up:
>  		/* No more children go up one step  */
> -		c = (struct node *) p;
> +		c = (struct node *)p;
>  		p = node_parent(c);
>  	}
> -	return NULL; /* Ready. Root of trie */
> +
> +	return NULL;		/* Ready. Root of trie */
>  }
> 
>  /*
> @@ -1755,7 +1772,7 @@ up:
>   */
>  static int fn_trie_flush(struct fib_table *tb)
>  {
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
>  	struct leaf *ll = NULL, *l = NULL;
>  	int found = 0, h;
> 
> @@ -1779,9 +1796,10 @@ static int fn_trie_flush(struct fib_tabl
>  static int trie_last_dflt = -1;
> 
>  static void
> -fn_trie_select_default(struct fib_table *tb, const struct flowi *flp, struct fib_result *res)
> +fn_trie_select_default(struct fib_table *tb, const struct flowi *flp,
> +		       struct fib_result *res)
>  {
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
>  	int order, last_idx;
>  	struct fib_info *fi = NULL;
>  	struct fib_info *last_resort;
> @@ -1809,8 +1827,7 @@ fn_trie_select_default(struct fib_table 
>  	list_for_each_entry_rcu(fa, fa_head, fa_list) {
>  		struct fib_info *next_fi = fa->fa_info;
> 
> -		if (fa->fa_scope != res->scope ||
> -		    fa->fa_type != RTN_UNICAST)
> +		if (fa->fa_scope != res->scope || fa->fa_type != RTN_UNICAST)
>  			continue;
> 
>  		if (next_fi->fib_priority > res->fi->fib_priority)
> @@ -1856,12 +1873,13 @@ fn_trie_select_default(struct fib_table 
>  			atomic_inc(&last_resort->fib_clntref);
>  	}
>  	trie_last_dflt = last_idx;
> - out:;
> +out:
>  	rcu_read_unlock();
>  }
> 
> -static int fn_trie_dump_fa(t_key key, int plen, struct list_head *fah, struct fib_table *tb,
> -			   struct sk_buff *skb, struct netlink_callback *cb)
> +static int fn_trie_dump_fa(t_key key, int plen, struct list_head *fah,
> +			   struct fib_table *tb, struct sk_buff *skb,
> +			   struct netlink_callback *cb)
>  {
>  	int i, s_i;
>  	struct fib_alias *fa;
> @@ -1886,10 +1904,7 @@ static int fn_trie_dump_fa(t_key key, in
>  				  tb->tb_id,
>  				  fa->fa_type,
>  				  fa->fa_scope,
> -				  xkey,
> -				  plen,
> -				  fa->fa_tos,
> -				  fa->fa_info, 0) < 0) {
> +				  xkey, plen, fa->fa_tos, fa->fa_info, 0) < 0) {
>  			cb->args[4] = i;
>  			return -1;
>  		}
> @@ -1899,8 +1914,8 @@ static int fn_trie_dump_fa(t_key key, in
>  	return skb->len;
>  }
> 
> -static int fn_trie_dump_plen(struct trie *t, int plen, struct fib_table *tb, struct sk_buff *skb,
> -			     struct netlink_callback *cb)
> +static int fn_trie_dump_plen(struct trie *t, int plen, struct fib_table *tb,
> +			     struct sk_buff *skb, struct netlink_callback *cb)
>  {
>  	int h, s_h;
>  	struct list_head *fa_head;
> @@ -1913,7 +1928,7 @@ static int fn_trie_dump_plen(struct trie
>  			continue;
>  		if (h > s_h)
>  			memset(&cb->args[4], 0,
> -			       sizeof(cb->args) - 4*sizeof(cb->args[0]));
> +			       sizeof(cb->args) - 4 * sizeof(cb->args[0]));
> 
>  		fa_head = get_fa_head(l, plen);
> 
> @@ -1923,7 +1938,7 @@ static int fn_trie_dump_plen(struct trie
>  		if (list_empty(fa_head))
>  			continue;
> 
> -		if (fn_trie_dump_fa(l->key, plen, fa_head, tb, skb, cb)<0) {
> +		if (fn_trie_dump_fa(l->key, plen, fa_head, tb, skb, cb) < 0) {
>  			cb->args[3] = h;
>  			return -1;
>  		}
> @@ -1932,10 +1947,11 @@ static int fn_trie_dump_plen(struct trie
>  	return skb->len;
>  }
> 
> -static int fn_trie_dump(struct fib_table *tb, struct sk_buff *skb, struct netlink_callback *cb)
> +static int fn_trie_dump(struct fib_table *tb, struct sk_buff *skb,
> +			struct netlink_callback *cb)
>  {
>  	int m, s_m;
> -	struct trie *t = (struct trie *) tb->tb_data;
> +	struct trie *t = (struct trie *)tb->tb_data;
> 
>  	s_m = cb->args[2];
> 
> @@ -1945,9 +1961,9 @@ static int fn_trie_dump(struct fib_table
>  			continue;
>  		if (m > s_m)
>  			memset(&cb->args[3], 0,
> -				sizeof(cb->args) - 3*sizeof(cb->args[0]));
> +			       sizeof(cb->args) - 3 * sizeof(cb->args[0]));
> 
> -		if (fn_trie_dump_plen(t, 32-m, tb, skb, cb)<0) {
> +		if (fn_trie_dump_plen(t, 32 - m, tb, skb, cb) < 0) {
>  			cb->args[2] = m;
>  			goto out;
>  		}
> @@ -1963,9 +1979,9 @@ out:
>  /* Fix more generic FIB names for init later */
> 
>  #ifdef CONFIG_IP_MULTIPLE_TABLES
> -struct fib_table * fib_hash_init(u32 id)
> +struct fib_table *fib_hash_init(u32 id)
>  #else
> -struct fib_table * __init fib_hash_init(u32 id)
> +struct fib_table *__init fib_hash_init(u32 id)
>  #endif
>  {
>  	struct fib_table *tb;
> @@ -1974,8 +1990,7 @@ struct fib_table * __init fib_hash_init(
>  	if (fn_alias_kmem == NULL)
>  		fn_alias_kmem = kmem_cache_create("ip_fib_alias",
>  						  sizeof(struct fib_alias),
> -						  0, SLAB_HWCACHE_ALIGN,
> -						  NULL);
> +						  0, SLAB_HWCACHE_ALIGN, NULL);
> 
>  	tb = kmalloc(sizeof(struct fib_table) + sizeof(struct trie),
>  		     GFP_KERNEL);
> @@ -1991,7 +2006,7 @@ struct fib_table * __init fib_hash_init(
>  	tb->tb_dump = fn_trie_dump;
>  	memset(tb->tb_data, 0, sizeof(struct trie));
> 
> -	t = (struct trie *) tb->tb_data;
> +	t = (struct trie *)tb->tb_data;
> 
>  	trie_init(t);
> 
> @@ -2001,7 +2016,8 @@ struct fib_table * __init fib_hash_init(
>  		trie_main = t;
> 
>  	if (id == RT_TABLE_LOCAL)
> -		printk(KERN_INFO "IPv4 FIB: Using LC-trie version %s\n", VERSION);
> +		printk(KERN_INFO "IPv4 FIB: Using LC-trie version %s\n",
> +		       VERSION);
> 
>  	return tb;
>  }
> @@ -2028,7 +2044,7 @@ static struct node *fib_trie_get_next(st
>  	pr_debug("get_next iter={node=%p index=%d depth=%d}\n",
>  		 iter->tnode, iter->index, iter->depth);
>  rescan:
> -	while (cindex < (1<<tn->bits)) {
> +	while (cindex < (1 << tn->bits)) {
>  		struct node *n = tnode_get_child(tn, cindex);
> 
>  		if (n) {
> @@ -2037,7 +2053,7 @@ rescan:
>  				iter->index = cindex + 1;
>  			} else {
>  				/* push down one level */
> -				iter->tnode = (struct tnode *) n;
> +				iter->tnode = (struct tnode *)n;
>  				iter->index = 0;
>  				++iter->depth;
>  			}
> @@ -2050,7 +2066,7 @@ rescan:
>  	/* Current node exhausted, pop back up */
>  	p = node_parent((struct node *)tn);
>  	if (p) {
> -		cindex = tkey_extract_bits(tn->key, p->pos, p->bits)+1;
> +		cindex = tkey_extract_bits(tn->key, p->pos, p->bits) + 1;
>  		tn = p;
>  		--iter->depth;
>  		goto rescan;
> @@ -2063,7 +2079,7 @@ rescan:
>  static struct node *fib_trie_get_first(struct fib_trie_iter *iter,
>  				       struct trie *t)
>  {
> -	struct node *n ;
> +	struct node *n;
> 
>  	if (!t)
>  		return NULL;
> @@ -2075,13 +2091,13 @@ static struct node *fib_trie_get_first(s
> 
>  	if (n) {
>  		if (IS_TNODE(n)) {
> -			iter->tnode = (struct tnode *) n;
> +			iter->tnode = (struct tnode *)n;
>  			iter->trie = t;
>  			iter->index = 0;
>  			iter->depth = 1;
>  		} else {
>  			iter->tnode = NULL;
> -			iter->trie  = t;
> +			iter->trie = t;
>  			iter->index = 0;
>  			iter->depth = 0;
>  		}
> @@ -2098,22 +2114,21 @@ static void trie_collect_stats(struct tr
>  	memset(s, 0, sizeof(*s));
> 
>  	rcu_read_lock();
> -	for (n = fib_trie_get_first(&iter, t); n;
> -	     n = fib_trie_get_next(&iter)) {
> +	for (n = fib_trie_get_first(&iter, t); n; n = fib_trie_get_next(&iter)) {
>  		if (IS_LEAF(n)) {
>  			s->leaves++;
>  			s->totdepth += iter.depth;
>  			if (iter.depth > s->maxdepth)
>  				s->maxdepth = iter.depth;
>  		} else {
> -			const struct tnode *tn = (const struct tnode *) n;
> +			const struct tnode *tn = (const struct tnode *)n;
>  			int i;
> 
>  			s->tnodes++;
>  			if (tn->bits < MAX_STAT_DEPTH)
>  				s->nodesizes[tn->bits]++;
> 
> -			for (i = 0; i < (1<<tn->bits); i++)
> +			for (i = 0; i < (1 << tn->bits); i++)
>  				if (!tn->child[i])
>  					s->nullpointers++;
>  		}
> @@ -2129,11 +2144,12 @@ static void trie_show_stats(struct seq_f
>  	unsigned i, max, pointers, bytes, avdepth;
> 
>  	if (stat->leaves)
> -		avdepth = stat->totdepth*100 / stat->leaves;
> +		avdepth = stat->totdepth * 100 / stat->leaves;
>  	else
>  		avdepth = 0;
> 
> -	seq_printf(seq, "\tAver depth:     %d.%02d\n", avdepth / 100, avdepth % 100 );
> +	seq_printf(seq, "\tAver depth:     %d.%02d\n", avdepth / 100,
> +		   avdepth % 100);
>  	seq_printf(seq, "\tMax depth:      %u\n", stat->maxdepth);
> 
>  	seq_printf(seq, "\tLeaves:         %u\n", stat->leaves);
> @@ -2143,14 +2159,14 @@ static void trie_show_stats(struct seq_f
>  	bytes += sizeof(struct tnode) * stat->tnodes;
> 
>  	max = MAX_STAT_DEPTH;
> -	while (max > 0 && stat->nodesizes[max-1] == 0)
> +	while (max > 0 && stat->nodesizes[max - 1] == 0)
>  		max--;
> 
>  	pointers = 0;
>  	for (i = 1; i <= max; i++)
>  		if (stat->nodesizes[i] != 0) {
> -			seq_printf(seq, "  %d: %d",  i, stat->nodesizes[i]);
> -			pointers += (1<<i) * stat->nodesizes[i];
> +			seq_printf(seq, "  %d: %d", i, stat->nodesizes[i]);
> +			pointers += (1 << i) * stat->nodesizes[i];
>  		}
>  	seq_putc(seq, '\n');
>  	seq_printf(seq, "\tPointers: %d\n", pointers);
> @@ -2161,12 +2177,15 @@ static void trie_show_stats(struct seq_f
> 
>  #ifdef CONFIG_IP_FIB_TRIE_STATS
>  	seq_printf(seq, "Counters:\n---------\n");
> -	seq_printf(seq,"gets = %d\n", t->stats.gets);
> -	seq_printf(seq,"backtracks = %d\n", t->stats.backtrack);
> -	seq_printf(seq,"semantic match passed = %d\n", t->stats.semantic_match_passed);
> -	seq_printf(seq,"semantic match miss = %d\n", t->stats.semantic_match_miss);
> -	seq_printf(seq,"null node hit= %d\n", t->stats.null_node_hit);
> -	seq_printf(seq,"skipped node resize = %d\n", t->stats.resize_node_skipped);
> +	seq_printf(seq, "gets = %d\n", t->stats.gets);
> +	seq_printf(seq, "backtracks = %d\n", t->stats.backtrack);
> +	seq_printf(seq, "semantic match passed = %d\n",
> +		   t->stats.semantic_match_passed);
> +	seq_printf(seq, "semantic match miss = %d\n",
> +		   t->stats.semantic_match_miss);
> +	seq_printf(seq, "null node hit= %d\n", t->stats.null_node_hit);
> +	seq_printf(seq, "skipped node resize = %d\n",
> +		   t->stats.resize_node_skipped);
>  #ifdef CLEAR_STATS
>  	memset(&(t->stats), 0, sizeof(t->stats));
>  #endif
> @@ -2181,7 +2200,8 @@ static int fib_triestat_seq_show(struct 
>  	if (!stat)
>  		return -ENOMEM;
> 
> -	seq_printf(seq, "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
> +	seq_printf(seq,
> +		   "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
>  		   sizeof(struct leaf), sizeof(struct tnode));
> 
>  	if (trie_local) {
> @@ -2213,8 +2233,7 @@ static const struct file_operations fib_
>  	.release = single_release,
>  };
> 
> -static struct node *fib_trie_get_idx(struct fib_trie_iter *iter,
> -				      loff_t pos)
> +static struct node *fib_trie_get_idx(struct fib_trie_iter *iter, loff_t pos)
>  {
>  	loff_t idx = 0;
>  	struct node *n;
> @@ -2233,7 +2252,7 @@ static struct node *fib_trie_get_idx(str
>  	return NULL;
>  }
> 
> -static void *fib_trie_seq_start(struct seq_file *seq, loff_t *pos)
> +static void *fib_trie_seq_start(struct seq_file *seq, loff_t * pos)
>  {
>  	rcu_read_lock();
>  	if (*pos == 0)
> @@ -2241,7 +2260,7 @@ static void *fib_trie_seq_start(struct s
>  	return fib_trie_get_idx(seq->private, *pos - 1);
>  }
> 
> -static void *fib_trie_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> +static void *fib_trie_seq_next(struct seq_file *seq, void *v, loff_t * pos)
>  {
>  	struct fib_trie_iter *iter = seq->private;
>  	void *l = v;
> @@ -2269,7 +2288,8 @@ static void fib_trie_seq_stop(struct seq
> 
>  static void seq_indent(struct seq_file *seq, int n)
>  {
> -	while (n-- > 0) seq_puts(seq, "   ");
> +	while (n-- > 0)
> +		seq_puts(seq, "   ");
>  }
> 
>  static inline const char *rtn_scope(enum rt_scope_t s)
> @@ -2277,11 +2297,16 @@ static inline const char *rtn_scope(enum
>  	static char buf[32];
> 
>  	switch (s) {
> -	case RT_SCOPE_UNIVERSE: return "universe";
> -	case RT_SCOPE_SITE:	return "site";
> -	case RT_SCOPE_LINK:	return "link";
> -	case RT_SCOPE_HOST:	return "host";
> -	case RT_SCOPE_NOWHERE:	return "nowhere";
> +	case RT_SCOPE_UNIVERSE:
> +		return "universe";
> +	case RT_SCOPE_SITE:
> +		return "site";
> +	case RT_SCOPE_LINK:
> +		return "link";
> +	case RT_SCOPE_HOST:
> +		return "host";
> +	case RT_SCOPE_NOWHERE:
> +		return "nowhere";
>  	default:
>  		snprintf(buf, sizeof(buf), "scope=%d", s);
>  		return buf;
> @@ -2330,16 +2355,16 @@ static int fib_trie_seq_show(struct seq_
>  	}
> 
>  	if (IS_TNODE(n)) {
> -		struct tnode *tn = (struct tnode *) n;
> +		struct tnode *tn = (struct tnode *)n;
>  		__be32 prf = htonl(mask_pfx(tn->key, tn->pos));
> 
> -		seq_indent(seq, iter->depth-1);
> +		seq_indent(seq, iter->depth - 1);
>  		seq_printf(seq, "  +-- %d.%d.%d.%d/%d %d %d %d\n",
>  			   NIPQUAD(prf), tn->pos, tn->bits, tn->full_children,
>  			   tn->empty_children);
> 
>  	} else {
> -		struct leaf *l = (struct leaf *) n;
> +		struct leaf *l = (struct leaf *)n;
>  		int i;
>  		__be32 val = htonl(l->key);
> 
> @@ -2350,7 +2375,7 @@ static int fib_trie_seq_show(struct seq_
>  			if (li) {
>  				struct fib_alias *fa;
>  				list_for_each_entry_rcu(fa, &li->falh, fa_list) {
> -					seq_indent(seq, iter->depth+1);
> +					seq_indent(seq, iter->depth + 1);
>  					seq_printf(seq, "  /%d %s %s", i,
>  						   rtn_scope(fa->fa_scope),
>  						   rtn_type(fa->fa_type));
> @@ -2386,7 +2411,7 @@ static int fib_trie_seq_open(struct inod
>  	if (rc)
>  		goto out_kfree;
> 
> -	seq	     = file->private_data;
> +	seq = file->private_data;
>  	seq->private = s;
>  	memset(s, 0, sizeof(*s));
>  out:
> @@ -2407,7 +2432,8 @@ static const struct file_operations fib_
>  static unsigned fib_flag_trans(int type, __be32 mask, const struct fib_info *fi)
>  {
>  	static unsigned type2flags[RTN_MAX + 1] = {
> -		[7] = RTF_REJECT, [8] = RTF_REJECT,
> +		[7] = RTF_REJECT,
> +		[8] = RTF_REJECT,
>  	};
>  	unsigned flags = type2flags[type];
> 
> @@ -2444,7 +2470,7 @@ static int fib_route_seq_show(struct seq
>  	if (IS_TNODE(l))
>  		return 0;
> 
> -	for (i=32; i>=0; i--) {
> +	for (i = 32; i >= 0; i--) {
>  		struct leaf_info *li = find_leaf_info(l, i);
>  		struct fib_alias *fa;
>  		__be32 mask, prefix;
> @@ -2471,7 +2497,7 @@ static int fib_route_seq_show(struct seq
>  					 fi->fib_nh->nh_gw, flags, 0, 0,
>  					 fi->fib_priority,
>  					 mask,
> -					 (fi->fib_advmss ? fi->fib_advmss + 40 : 0),
> +					 fi->fib_advmss ? fi->fib_advmss + 40 : 0,
>  					 fi->fib_window,
>  					 fi->fib_rtt >> 3);
>  			else
> @@ -2507,7 +2533,7 @@ static int fib_route_seq_open(struct ino
>  	if (rc)
>  		goto out_kfree;
> 
> -	seq	     = file->private_data;
> +	seq = file->private_data;
>  	seq->private = s;
>  	memset(s, 0, sizeof(*s));
>  out:

Re: [RFC] fib_trie: whitespace cleanup

[Andrew Morton]

On Thu, 26 Jul 2007 08:44:21 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Thu, Jul 26, 2007 at 11:49:42AM +0100, Stephen Hemminger wrote:
> > Whitespace cleanup run code through lindent then cleanup results.
> > Applys after other two patches.
> > 
> > --- a/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
> > +++ b/net/ipv4/fib_trie.c	2007-07-26 11:47:52.000000000 +0100
> > @@ -156,7 +156,8 @@ struct trie {
> >  };
> > 
> >  static void put_child(struct trie *t, struct tnode *tn, int i, struct node *n);
> > -static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull);
> > +static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
> > +				  int wasfull);
> >  static struct node *resize(struct trie *t, struct tnode *tn);
> >  static struct tnode *inflate(struct trie *t, struct tnode *tn);
> >  static struct tnode *halve(struct trie *t, struct tnode *tn);
> > @@ -167,13 +168,12 @@ static struct trie *trie_local = NULL, *
> > 
> >  static inline struct tnode *node_parent(struct node *node)
> >  {
> > -	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
> > +	return rcu_dereference((struct tnode *)(node->parent & ~NODE_TYPE_MASK));
> 
> The potential issue is applying rcu_dereference() to an rvalue
> as opposed to an lvalue.  So how about the following?
> 

I did this:

static inline struct tnode *node_parent(struct node *node)
{
	struct tnode *ret;

	ret = (struct tnode *)(node->parent & ~NODE_TYPE_MASK);
	return rcu_dereference(ret);
}

Re: [RFC] fib_trie: whitespace cleanup

[Paul E. McKenney]

On Thu, Jul 26, 2007 at 09:56:30PM -0700, Andrew Morton wrote:
> On Thu, 26 Jul 2007 08:44:21 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Thu, Jul 26, 2007 at 11:49:42AM +0100, Stephen Hemminger wrote:
> > > Whitespace cleanup run code through lindent then cleanup results.
> > > Applys after other two patches.
> > > 
> > > --- a/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
> > > +++ b/net/ipv4/fib_trie.c	2007-07-26 11:47:52.000000000 +0100
> > > @@ -156,7 +156,8 @@ struct trie {
> > >  };
> > > 
> > >  static void put_child(struct trie *t, struct tnode *tn, int i, struct node *n);
> > > -static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n, int wasfull);
> > > +static void tnode_put_child_reorg(struct tnode *tn, int i, struct node *n,
> > > +				  int wasfull);
> > >  static struct node *resize(struct trie *t, struct tnode *tn);
> > >  static struct tnode *inflate(struct trie *t, struct tnode *tn);
> > >  static struct tnode *halve(struct trie *t, struct tnode *tn);
> > > @@ -167,13 +168,12 @@ static struct trie *trie_local = NULL, *
> > > 
> > >  static inline struct tnode *node_parent(struct node *node)
> > >  {
> > > -	return rcu_dereference((struct tnode *) (node->parent & ~NODE_TYPE_MASK));
> > > +	return rcu_dereference((struct tnode *)(node->parent & ~NODE_TYPE_MASK));
> > 
> > The potential issue is applying rcu_dereference() to an rvalue
> > as opposed to an lvalue.  So how about the following?
> > 
> 
> I did this:
> 
> static inline struct tnode *node_parent(struct node *node)
> {
> 	struct tnode *ret;
> 
> 	ret = (struct tnode *)(node->parent & ~NODE_TYPE_MASK);
> 	return rcu_dereference(ret);
> }

I would feel more comfortable with the rcu_dereference() covering the
initial fetch from node->parent, but do not have any hard objections
to your approach.

						Thanx, Paul

[RFT] fib_trie: macro cleanup

[Stephen Hemminger]

This patch converts the messy macro for MASK_PFX to inline function
and expands TKEY_GET_MASK in the one place it is used.


--- a/net/ipv4/fib_trie.c	2007-07-26 09:26:19.000000000 +0100
+++ b/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
@@ -85,8 +85,6 @@
 #define MAX_STAT_DEPTH 32
 
 #define KEYLENGTH (8*sizeof(t_key))
-#define MASK_PFX(k, l) (((l)==0)?0:(k >> (KEYLENGTH-l)) << (KEYLENGTH-l))
-#define TKEY_GET_MASK(offset, bits) (((bits)==0)?0:((t_key)(-1) << (KEYLENGTH - bits) >> offset))
 
 typedef unsigned int t_key;
 
@@ -192,6 +190,11 @@ static inline int tnode_child_length(con
 	return 1 << tn->bits;
 }
 
+static inline t_key mask_pfx(t_key k, unsigned short l)
+{
+	return (l == 0) ? 0 : k >> (KEYLENGTH-l) << (KEYLENGTH-l);
+}
+
 static inline t_key tkey_extract_bits(t_key a, int offset, int bits)
 {
 	if (offset < KEYLENGTH)
@@ -676,7 +679,7 @@ static struct tnode *inflate(struct trie
 		    inode->pos == oldtnode->pos + oldtnode->bits &&
 		    inode->bits > 1) {
 			struct tnode *left, *right;
-			t_key m = TKEY_GET_MASK(inode->pos, 1);
+			t_key m = ~0U << (KEYLENGTH - 1) >> inode->pos;
 
 			left = tnode_new(inode->key&(~m), inode->pos + 1,
 					 inode->bits - 1);
@@ -1364,7 +1367,8 @@ fn_trie_lookup(struct fib_table *tb, con
 		bits = pn->bits;
 
 		if (!chopped_off)
-			cindex = tkey_extract_bits(MASK_PFX(key, current_prefix_length), pos, bits);
+			cindex = tkey_extract_bits(mask_pfx(key, current_prefix_length),
+						   pos, bits);
 
 		n = tnode_get_child(pn, cindex);
 
@@ -1450,8 +1454,8 @@ fn_trie_lookup(struct fib_table *tb, con
 		 * to find a matching prefix.
 		 */
 
-		node_prefix = MASK_PFX(cn->key, cn->pos);
-		key_prefix = MASK_PFX(key, cn->pos);
+		node_prefix = mask_pfx(cn->key, cn->pos);
+		key_prefix = mask_pfx(key, cn->pos);
 		pref_mismatch = key_prefix^node_prefix;
 		mp = 0;
 
@@ -2327,7 +2331,7 @@ static int fib_trie_seq_show(struct seq_
 
 	if (IS_TNODE(n)) {
 		struct tnode *tn = (struct tnode *) n;
-		__be32 prf = htonl(MASK_PFX(tn->key, tn->pos));
+		__be32 prf = htonl(mask_pfx(tn->key, tn->pos));
 
 		seq_indent(seq, iter->depth-1);
 		seq_printf(seq, "  +-- %d.%d.%d.%d/%d %d %d %d\n",

Re: [RFT] fib_trie: macro cleanup

[Andrew Morton]

On Thu, 26 Jul 2007 11:43:34 +0100 Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> This patch converts the messy macro for MASK_PFX to inline function
> and expands TKEY_GET_MASK in the one place it is used.
> 
> 
> --- a/net/ipv4/fib_trie.c	2007-07-26 09:26:19.000000000 +0100
> +++ b/net/ipv4/fib_trie.c	2007-07-26 10:17:21.000000000 +0100
> @@ -85,8 +85,6 @@
>  #define MAX_STAT_DEPTH 32
>  
>  #define KEYLENGTH (8*sizeof(t_key))
> -#define MASK_PFX(k, l) (((l)==0)?0:(k >> (KEYLENGTH-l)) << (KEYLENGTH-l))
> -#define TKEY_GET_MASK(offset, bits) (((bits)==0)?0:((t_key)(-1) << (KEYLENGTH - bits) >> offset))
>  
>  typedef unsigned int t_key;
>  
> @@ -192,6 +190,11 @@ static inline int tnode_child_length(con
>  	return 1 << tn->bits;
>  }
>  
> +static inline t_key mask_pfx(t_key k, unsigned short l)
> +{
> +	return (l == 0) ? 0 : k >> (KEYLENGTH-l) << (KEYLENGTH-l);
> +}

that's a funy way of doing the masking, isn't it?  I suppose gcc will turn
it into the single and-immediate.

>  static inline t_key tkey_extract_bits(t_key a, int offset, int bits)
>  {
>  	if (offset < KEYLENGTH)
> @@ -676,7 +679,7 @@ static struct tnode *inflate(struct trie
>  		    inode->pos == oldtnode->pos + oldtnode->bits &&
>  		    inode->bits > 1) {
>  			struct tnode *left, *right;
> -			t_key m = TKEY_GET_MASK(inode->pos, 1);
> +			t_key m = ~0U << (KEYLENGTH - 1) >> inode->pos;

hm, so we "know" that t_key is an unsigned int.  It makes the typedef a bit
pointless.

<wonders what an inode is doing in there>

<oh>

NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Gabriel C]

Hi,

I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).

...

net/core/netpoll.c: In function 'netpoll_poll':
net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
net/core/netpoll.c: In function 'netpoll_setup':
net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
make[2]: *** [net/core/netpoll.o] Error 1
make[1]: *** [net/core] Error 2
make: *** [net] Error 2
make: *** Waiting for unfinished jobs....

...


I think is because KGDBOE selects just NETPOLL.


Regards,

Gabriel

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Andrew Morton]

On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:

> Hi,
> 
> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
> 
> ...
> 
> net/core/netpoll.c: In function 'netpoll_poll':
> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
> net/core/netpoll.c: In function 'netpoll_setup':
> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
> make[2]: *** [net/core/netpoll.o] Error 1
> make[1]: *** [net/core] Error 2
> make: *** [net] Error 2
> make: *** Waiting for unfinished jobs....
> 
> ...
> 
> 
> I think is because KGDBOE selects just NETPOLL.
> 

Looks like it.

Select went and selected NETPOLL and NETPOLL_TRAP but things like
CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
remains evil.

Something like this..

--- a/lib/Kconfig.kgdb~kgdb-kconfig-fix
+++ a/lib/Kconfig.kgdb
@@ -175,8 +175,7 @@ endchoice
 config KGDBOE
 	tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
 	depends on m && KGDB
-	select NETPOLL
-	select NETPOLL_TRAP
+	depends on NETPOLL_TRAP && NET_POLL_CONTROLLER
 	help
 	  Uses the NETPOLL API to communicate with the host GDB via UDP.
 	  In order for this to work, the ethernet interface specified must
_

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Gabriel C]

Andrew Morton wrote:
> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> 
>> Hi,
>>
>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>
>> ...
>>
>> net/core/netpoll.c: In function 'netpoll_poll':
>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>> net/core/netpoll.c: In function 'netpoll_setup':
>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>> make[2]: *** [net/core/netpoll.o] Error 1
>> make[1]: *** [net/core] Error 2
>> make: *** [net] Error 2
>> make: *** Waiting for unfinished jobs....
>>
>> ...
>>
>>
>> I think is because KGDBOE selects just NETPOLL.
>>
> 
> Looks like it.
> 
> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> remains evil.
> 
> Something like this..
> 
> --- a/lib/Kconfig.kgdb~kgdb-kconfig-fix
> +++ a/lib/Kconfig.kgdb
> @@ -175,8 +175,7 @@ endchoice
>  config KGDBOE
>  	tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
>  	depends on m && KGDB
> -	select NETPOLL
> -	select NETPOLL_TRAP
> +	depends on NETPOLL_TRAP && NET_POLL_CONTROLLER
>  	help
>  	  Uses the NETPOLL API to communicate with the host GDB via UDP.
>  	  In order for this to work, the ethernet interface specified must
> _
> 
> 


That doesn't fix it. With that patch an 'make oldconfig' all NETPOLL stuff gone and we end up with :

...

drivers/built-in.o: In function `option_setup':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:160: undefined reference to `netpoll_parse_options'
drivers/built-in.o: In function `configure_kgdboe':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:183: undefined reference to `netpoll_setup'
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:189: undefined reference to `netpoll_cleanup'
drivers/built-in.o: In function `eth_post_exception_handler':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:119: undefined reference to `netpoll_set_trap'
drivers/built-in.o: In function `eth_pre_exception_handler':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:111: undefined reference to `netpoll_set_trap'
drivers/built-in.o: In function `eth_flush_buf':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:138: undefined reference to `netpoll_send_udp'
drivers/built-in.o: In function `eth_get_char':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:127: undefined reference to `netpoll_poll'
drivers/built-in.o: In function `cleanup_kgdboe':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:217: undefined reference to `netpoll_cleanup'
make: *** [.tmp_vmlinux1] Error 1

...


If I get that right  select is needed here because  all NETPOLL{_*} depends on if NETDEVICES && if NET_ETHERNET.

Also doing 
	
	...
	select NETPOLL_TRAP 
	select NETPOLL
	select NET_POLL_CONTROLLER
	...

makes the driver happy and everything compiles fine.

I think there may be a logical issue ( again if I got it right ).
We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.

So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 

( really sory if I said something stupid these Kconfig depends are not really easy to figure for me )


Gabriel 

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On 28-07-2007 20:42, Gabriel C wrote:
> Andrew Morton wrote:
>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>
>>> Hi,
>>>
>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>
>>> ...
>>>
>>> net/core/netpoll.c: In function 'netpoll_poll':
>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>> net/core/netpoll.c: In function 'netpoll_setup':
>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>> make[2]: *** [net/core/netpoll.o] Error 1
>>> make[1]: *** [net/core] Error 2
>>> make: *** [net] Error 2
>>> make: *** Waiting for unfinished jobs....
>>>
>>> ...
>>>
>>>
>>> I think is because KGDBOE selects just NETPOLL.
>>>
>> Looks like it.
>>
>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
>> remains evil.
...
> I think there may be a logical issue ( again if I got it right ).
> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
> 
> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 

IMHO, the only logical issue here is netpoll.c mustn't use 
CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
add this dependency itself.

Cheers,
Jarek P. 

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Gabriel C]

Jarek Poplawski wrote:
> On 28-07-2007 20:42, Gabriel C wrote:
>> Andrew Morton wrote:
>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>>
>>>> ...
>>>>
>>>> net/core/netpoll.c: In function 'netpoll_poll':
>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>>> net/core/netpoll.c: In function 'netpoll_setup':
>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>>> make[2]: *** [net/core/netpoll.o] Error 1
>>>> make[1]: *** [net/core] Error 2
>>>> make: *** [net] Error 2
>>>> make: *** Waiting for unfinished jobs....
>>>>
>>>> ...
>>>>
>>>>
>>>> I think is because KGDBOE selects just NETPOLL.
>>>>
>>> Looks like it.
>>>
>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
>>> remains evil.
> ...
>> I think there may be a logical issue ( again if I got it right ).
>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
>>
>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
> 
> IMHO, the only logical issue here is netpoll.c mustn't use 
> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
> add this dependency itself.
> 

Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.

Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?

> Cheers,
> Jarek P. 
> 


Gabriel

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jason Wessel]

Gabriel C wrote:
> Jarek Poplawski wrote:
>   
>> On 28-07-2007 20:42, Gabriel C wrote:
>>     
>>> Andrew Morton wrote:
>>>       
>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>>>
>>>>         
>>>>> Hi,
>>>>>
>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>>>
>>>>> ...
>>>>>
>>>>> net/core/netpoll.c: In function 'netpoll_poll':
>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>>>> net/core/netpoll.c: In function 'netpoll_setup':
>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>>>> make[2]: *** [net/core/netpoll.o] Error 1
>>>>> make[1]: *** [net/core] Error 2
>>>>> make: *** [net] Error 2
>>>>> make: *** Waiting for unfinished jobs....
>>>>>
>>>>> ...
>>>>>
>>>>>
>>>>> I think is because KGDBOE selects just NETPOLL.
>>>>>
>>>>>           
>>>> Looks like it.
>>>>
>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
>>>> remains evil.
>>>>         
>> ...
>>     
>>> I think there may be a logical issue ( again if I got it right ).
>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
>>>
>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
>>>       
>> IMHO, the only logical issue here is netpoll.c mustn't use 
>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
>> add this dependency itself.
>>
>>     
>
> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
>
> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
>   

kgdboe is completely useless without a network card that has a polling 
driver.  It seems to me that the simple and easy fix is to set it to 
depend on NETDEVICES but allow it to use select on NETPOLL.

Would that seem reasonable?

Jason.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Tue, Jul 31, 2007 at 06:44:52AM -0500, Jason Wessel wrote:
...
> kgdboe is completely useless without a network card that has a polling 
> driver.  It seems to me that the simple and easy fix is to set it to 
> depend on NETDEVICES but allow it to use select on NETPOLL.

Maybe I miss your point but polling drivers don't need NETPOLL
to work (unless you need netconsole). But I don't know if there
is any easy method to check such driver's dependency with select.

Jarek P.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> Jarek Poplawski wrote:
> > On 28-07-2007 20:42, Gabriel C wrote:
> >> Andrew Morton wrote:
> >>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
> >>>>
> >>>> ...
> >>>>
> >>>> net/core/netpoll.c: In function 'netpoll_poll':
> >>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> >>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
> >>>> net/core/netpoll.c: In function 'netpoll_setup':
> >>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
> >>>> make[2]: *** [net/core/netpoll.o] Error 1
> >>>> make[1]: *** [net/core] Error 2
> >>>> make: *** [net] Error 2
> >>>> make: *** Waiting for unfinished jobs....
> >>>>
> >>>> ...
> >>>>
> >>>>
> >>>> I think is because KGDBOE selects just NETPOLL.
> >>>>
> >>> Looks like it.
> >>>
> >>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> >>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> >>> remains evil.
> > ...
> >> I think there may be a logical issue ( again if I got it right ).
> >> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
> >>
> >> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
> > 
> > IMHO, the only logical issue here is netpoll.c mustn't use 
> > CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
> > add this dependency itself.
> > 
> 
> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.

"does if XXX" means may "use if XXX".

> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?

Why kgdboe should care what netpoll needs? So, I hope, you are adding
this select under config NETPOLL. On the other hand, if NETPOLL should
depend on NET_POLL_CONTROLLER there is probably no reason to have them
both.

The "does it work" question isn't logical issue, so it's irrelevant
here...

Jarek P.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Gabriel C]

Jarek Poplawski wrote:
> On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
>> Jarek Poplawski wrote:
>>> On 28-07-2007 20:42, Gabriel C wrote:
>>>> Andrew Morton wrote:
>>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>>>>
>>>>>> ...
>>>>>>
>>>>>> net/core/netpoll.c: In function 'netpoll_poll':
>>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> net/core/netpoll.c: In function 'netpoll_setup':
>>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> make[2]: *** [net/core/netpoll.o] Error 1
>>>>>> make[1]: *** [net/core] Error 2
>>>>>> make: *** [net] Error 2
>>>>>> make: *** Waiting for unfinished jobs....
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>
>>>>>> I think is because KGDBOE selects just NETPOLL.
>>>>>>
>>>>> Looks like it.
>>>>>
>>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
>>>>> remains evil.
>>> ...
>>>> I think there may be a logical issue ( again if I got it right ).
>>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
>>>>
>>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
>>> IMHO, the only logical issue here is netpoll.c mustn't use 
>>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
>>> add this dependency itself.
>>>
>> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
> 
> "does if XXX" means may "use if XXX".

>From what I know means only use "if xxx" on !xxx everything inside the "if xxx" is n and "depends on <something inside the if xxx> 
does not work.

...

menuconfig FOO
	bool "FOO"
	depends on BAR
	default y 
	-- help --
	something
if FOO

config BAZ
	depends on WHATEVR && !NOT_THIS

menuconfig SOMETHING_ELSE
	....
if SOMETHING_ELSE

config BLUBB
	depends on PCI && WHATNOT

endif # SOMETHING_ELSE

config NETPOLL
        def_bool NETCONSOLE
        
config NETPOLL_TRAP
        bool "Netpoll traffic trapping"
        default n
        depends on NETPOLL
          
config NET_POLL_CONTROLLER
        def_bool NETPOLL

endif # FOO

Now if you set FOO=n all is gone and your driver have to select whatever it needs from there.

> 
>> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
>> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
> 
> Why kgdboe should care what netpoll needs? So, I hope, you are adding
> this select under config NETPOLL. On the other hand, if NETPOLL should
> depend on NET_POLL_CONTROLLER there is probably no reason to have them
> both.

NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .

Net peoples ping ?:)

> 
> The "does it work" question isn't logical issue, so it's irrelevant
> here...

Right irrelevant for the compile error but relevant for the fix in my opinion.

> 
> Jarek P.
> 

Gabriel

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Tue, Jul 31, 2007 at 05:05:00PM +0200, Gabriel C wrote:
> Jarek Poplawski wrote:
> > On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> >> Jarek Poplawski wrote:
> >>> On 28-07-2007 20:42, Gabriel C wrote:
> >>>> Andrew Morton wrote:
> >>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> >>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
> >>>>>>
> >>>>>> ...
> >>>>>>
> >>>>>> net/core/netpoll.c: In function 'netpoll_poll':
> >>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> >>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
> >>>>>> net/core/netpoll.c: In function 'netpoll_setup':
> >>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
> >>>>>> make[2]: *** [net/core/netpoll.o] Error 1
> >>>>>> make[1]: *** [net/core] Error 2
> >>>>>> make: *** [net] Error 2
> >>>>>> make: *** Waiting for unfinished jobs....
> >>>>>>
> >>>>>> ...
> >>>>>>
> >>>>>>
> >>>>>> I think is because KGDBOE selects just NETPOLL.
> >>>>>>
> >>>>> Looks like it.
> >>>>>
> >>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> >>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> >>>>> remains evil.
> >>> ...
> >>>> I think there may be a logical issue ( again if I got it right ).
> >>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
> >>>>
> >>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
> >>> IMHO, the only logical issue here is netpoll.c mustn't use 
> >>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
> >>> add this dependency itself.
> >>>
> >> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
> > 
> > "does if XXX" means may "use if XXX".
> 
> From what I know means only use "if xxx" on !xxx everything inside the "if xxx" is n and "depends on <something inside the if xxx> 
> does not work.
> 
> ...
> 
> menuconfig FOO
> 	bool "FOO"
> 	depends on BAR
> 	default y 
> 	-- help --
> 	something
> if FOO
> 
> config BAZ
> 	depends on WHATEVR && !NOT_THIS
> 
> menuconfig SOMETHING_ELSE
> 	....
> if SOMETHING_ELSE
> 
> config BLUBB
> 	depends on PCI && WHATNOT
> 
> endif # SOMETHING_ELSE
> 
> config NETPOLL
>         def_bool NETCONSOLE
>         
> config NETPOLL_TRAP
>         bool "Netpoll traffic trapping"
>         default n
>         depends on NETPOLL
>           
> config NET_POLL_CONTROLLER
>         def_bool NETPOLL
> 
> endif # FOO
> 
> Now if you set FOO=n all is gone and your driver have to select whatever it needs from there.

Probably not exactly so...

>From drivers/net/Kconfig:

> # All the following symbols are dependent on NETDEVICES - do not repeat
> # that for each of the symbols.
> if NETDEVICES

So, according to this netpoll could presume NETDEVICES and
NET_POLL_CONTROLLER are always on.

But, as you've found, it's possible to select NETPOLL and !NETDEVICES,
so this comment is at least not precise.

On the other side something like this:

...
endif # NETDEVICES

config NETPOLL
        depends on NETDEVICES
        def_bool NETCONSOLE

config NETPOLL_TRAP
        bool "Netpoll traffic trapping"
        default n
        depends on NETPOLL

config NET_POLL_CONTROLLER
        def_bool NETPOLL
        depends on NETPOLL


seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
still doesn't check for NETDEVICES dependency.

> 
> > 
> >> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> >> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
> > 
> > Why kgdboe should care what netpoll needs? So, I hope, you are adding
> > this select under config NETPOLL. On the other hand, if NETPOLL should
> > depend on NET_POLL_CONTROLLER there is probably no reason to have them
> > both.
> 
> NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .
> 
> Net peoples ping ?:)

OK, I wasn't right here: there is no visible reason for both in the
kernel code, but I can imagine there could be some external users of
NET_POLL_CONTROLLER without NETPOLL.

> 
> > 
> > The "does it work" question isn't logical issue, so it's irrelevant
> > here...
> 
> Right irrelevant for the compile error but relevant for the fix in my opinion.

This was kind of joking, but since some people prefer things to work,
and it's hard to do this right (logical) way, some strange (unlogical)
measures have to be done like repeating dependencies here.

Regards,
Jarek P.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Matt Mackall]

On Wed, Aug 01, 2007 at 11:59:21AM +0200, Jarek Poplawski wrote:
> On Tue, Jul 31, 2007 at 05:05:00PM +0200, Gabriel C wrote:
> > Jarek Poplawski wrote:
> > > On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> > >> Jarek Poplawski wrote:
> > >>> On 28-07-2007 20:42, Gabriel C wrote:
> > >>>> Andrew Morton wrote:
> > >>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> > >>>>>
> > >>>>>> Hi,
> > >>>>>>
> > >>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
> > >>>>>>
> > >>>>>> ...
> > >>>>>>
> > >>>>>> net/core/netpoll.c: In function 'netpoll_poll':
> > >>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> > >>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
> > >>>>>> net/core/netpoll.c: In function 'netpoll_setup':
> > >>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
> > >>>>>> make[2]: *** [net/core/netpoll.o] Error 1
> > >>>>>> make[1]: *** [net/core] Error 2
> > >>>>>> make: *** [net] Error 2
> > >>>>>> make: *** Waiting for unfinished jobs....
> > >>>>>>
> > >>>>>> ...
> > >>>>>>
> > >>>>>>
> > >>>>>> I think is because KGDBOE selects just NETPOLL.
> > >>>>>>
> > >>>>> Looks like it.
> > >>>>>
> > >>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> > >>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> > >>>>> remains evil.
> > >>> ...
> > >>>> I think there may be a logical issue ( again if I got it right ).
> > >>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
> > >>>>
> > >>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ? 
> > >>> IMHO, the only logical issue here is netpoll.c mustn't use 
> > >>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
> > >>> add this dependency itself.
> > >>>
> > >> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
> > > 
> > > "does if XXX" means may "use if XXX".
> > 
> > From what I know means only use "if xxx" on !xxx everything inside the "if xxx" is n and "depends on <something inside the if xxx> 
> > does not work.
> > 
> > ...
> > 
> > menuconfig FOO
> > 	bool "FOO"
> > 	depends on BAR
> > 	default y 
> > 	-- help --
> > 	something
> > if FOO
> > 
> > config BAZ
> > 	depends on WHATEVR && !NOT_THIS
> > 
> > menuconfig SOMETHING_ELSE
> > 	....
> > if SOMETHING_ELSE
> > 
> > config BLUBB
> > 	depends on PCI && WHATNOT
> > 
> > endif # SOMETHING_ELSE
> > 
> > config NETPOLL
> >         def_bool NETCONSOLE
> >         
> > config NETPOLL_TRAP
> >         bool "Netpoll traffic trapping"
> >         default n
> >         depends on NETPOLL
> >           
> > config NET_POLL_CONTROLLER
> >         def_bool NETPOLL
> > 
> > endif # FOO
> > 
> > Now if you set FOO=n all is gone and your driver have to select whatever it needs from there.
> 
> Probably not exactly so...
> 
> >From drivers/net/Kconfig:
> 
> > # All the following symbols are dependent on NETDEVICES - do not repeat
> > # that for each of the symbols.
> > if NETDEVICES
> 
> So, according to this netpoll could presume NETDEVICES and
> NET_POLL_CONTROLLER are always on.
> 
> But, as you've found, it's possible to select NETPOLL and !NETDEVICES,
> so this comment is at least not precise.
> 
> On the other side something like this:
> 
> ...
> endif # NETDEVICES
> 
> config NETPOLL
>         depends on NETDEVICES
>         def_bool NETCONSOLE
> 
> config NETPOLL_TRAP
>         bool "Netpoll traffic trapping"
>         default n
>         depends on NETPOLL
> 
> config NET_POLL_CONTROLLER
>         def_bool NETPOLL
>         depends on NETPOLL
> 
> 
> seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> still doesn't check for NETDEVICES dependency.

That's odd. Adding Sam to the cc:.

> > >> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> > >> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
> > > 
> > > Why kgdboe should care what netpoll needs? So, I hope, you are adding
> > > this select under config NETPOLL. On the other hand, if NETPOLL should
> > > depend on NET_POLL_CONTROLLER there is probably no reason to have them
> > > both.
> > 
> > NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .
> > 
> > Net peoples ping ?:)

How about cc:ing the netpoll maintainer?
 
> OK, I wasn't right here: there is no visible reason for both in the
> kernel code, but I can imagine there could be some external users of
> NET_POLL_CONTROLLER without NETPOLL.

I don't know of any. As far as I can tell at this point,
NET_POLL_CONTROLLER == NETPOLL.

-- 
Mathematics is the supreme nostalgia of our time.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Wed, Aug 01, 2007 at 09:02:19PM -0500, Matt Mackall wrote:
> On Wed, Aug 01, 2007 at 11:59:21AM +0200, Jarek Poplawski wrote:
> > On Tue, Jul 31, 2007 at 05:05:00PM +0200, Gabriel C wrote:
> > > Jarek Poplawski wrote:
> > > > On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> > > >> Jarek Poplawski wrote:
> > > >>> On 28-07-2007 20:42, Gabriel C wrote:
> > > >>>> Andrew Morton wrote:
> > > >>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
...
> > > >>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
...
> > > >>>>>> I think is because KGDBOE selects just NETPOLL.
> > > >>>>>>
> > > >>>>> Looks like it.
> > > >>>>>
> > > >>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> > > >>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> > > >>>>> remains evil.
...
> > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > still doesn't check for NETDEVICES dependency.
> 
> That's odd. Adding Sam to the cc:.

Looks right, but after reading Andrew's opinion about select I'd be
astonished if he doesn't know this problem already.

> 
> > > >> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> > > >> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
> > > > 
> > > > Why kgdboe should care what netpoll needs? So, I hope, you are adding
> > > > this select under config NETPOLL. On the other hand, if NETPOLL should
> > > > depend on NET_POLL_CONTROLLER there is probably no reason to have them
> > > > both.
> > > 
> > > NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .
> > > 
> > > Net peoples ping ?:)
> 
> How about cc:ing the netpoll maintainer?

Is there a new one or do you suggest possibility of abusing the
authority of the netpoll's author with such trifles...?!

BTW, I can't find any official meaning of def_bool, but it's name
suggests only default value, so logically it should be not enough
to assure NET_POLL_CONTROLLER=y, and netpoll should use "depends",
"require" or "select" (IMHO more readable too), but on the other
hand this could be practially wrong...

>  
> > OK, I wasn't right here: there is no visible reason for both in the
> > kernel code, but I can imagine there could be some external users of
> > NET_POLL_CONTROLLER without NETPOLL.
> 
> I don't know of any. As far as I can tell at this point,
> NET_POLL_CONTROLLER == NETPOLL.

There are some notions about "other diagnostic tools" in some
net drivers, eg. 3c509.c, so there would be a little bit of
work if, after changing this, they really exist (and even if
not - maybe it's reasonable to save such possibility for the
future?).

Best regards,
Jarek P.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Matt Mackall]

On Thu, Aug 02, 2007 at 11:00:08AM +0200, Jarek Poplawski wrote:
> On Wed, Aug 01, 2007 at 09:02:19PM -0500, Matt Mackall wrote:
> > On Wed, Aug 01, 2007 at 11:59:21AM +0200, Jarek Poplawski wrote:
> > > On Tue, Jul 31, 2007 at 05:05:00PM +0200, Gabriel C wrote:
> > > > Jarek Poplawski wrote:
> > > > > On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> > > > >> Jarek Poplawski wrote:
> > > > >>> On 28-07-2007 20:42, Gabriel C wrote:
> > > > >>>> Andrew Morton wrote:
> > > > >>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> ...
> > > > >>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> ...
> > > > >>>>>> I think is because KGDBOE selects just NETPOLL.
> > > > >>>>>>
> > > > >>>>> Looks like it.
> > > > >>>>>
> > > > >>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> > > > >>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset.  `select'
> > > > >>>>> remains evil.
> ...
> > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > > still doesn't check for NETDEVICES dependency.
> > 
> > That's odd. Adding Sam to the cc:.
> 
> Looks right, but after reading Andrew's opinion about select I'd be
> astonished if he doesn't know this problem already.
> 
> > 
> > > > >> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> > > > >> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
> > > > > 
> > > > > Why kgdboe should care what netpoll needs? So, I hope, you are adding
> > > > > this select under config NETPOLL. On the other hand, if NETPOLL should
> > > > > depend on NET_POLL_CONTROLLER there is probably no reason to have them
> > > > > both.
> > > > 
> > > > NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .
> > > > 
> > > > Net peoples ping ?:)
> > 
> > How about cc:ing the netpoll maintainer?
> 
> Is there a new one or do you suggest possibility of abusing the
> authority of the netpoll's author with such trifles...?!

I'm just subtly suggesting that if you're going to have a discussion
about netpoll, you ought to cc: me.

> There are some notions about "other diagnostic tools" in some
> net drivers, eg. 3c509.c, so there would be a little bit of
> work if, after changing this, they really exist (and even if
> not - maybe it's reasonable to save such possibility for the
> future?).

I created it for netpoll, only netpoll clients have ever cared.

-- 
Mathematics is the supreme nostalgia of our time.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Thu, Aug 02, 2007 at 10:59:23AM -0500, Matt Mackall wrote:
> On Thu, Aug 02, 2007 at 11:00:08AM +0200, Jarek Poplawski wrote:
> > On Wed, Aug 01, 2007 at 09:02:19PM -0500, Matt Mackall wrote:
...
> > > How about cc:ing the netpoll maintainer?
> > 
> > Is there a new one or do you suggest possibility of abusing the
> > authority of the netpoll's author with such trifles...?!
> 
> I'm just subtly suggesting that if you're going to have a discussion
> about netpoll, you ought to cc: me.

Thanks! I'm very honored. I've suspected there is some subtlety, but
wasn't sure of possible new patches to MAINTAINERS, so tried to be
subtle too...

> 
> > There are some notions about "other diagnostic tools" in some
> > net drivers, eg. 3c509.c, so there would be a little bit of
> > work if, after changing this, they really exist (and even if
> > not - maybe it's reasonable to save such possibility for the
> > future?).
> 
> I created it for netpoll, only netpoll clients have ever cared.

So, probably you're the best person to change this! Alas, it seems,
for some time any changes to netpoll could have a cold reception
here (pity for Ingo's laptop...).

Regards,
Jarek P.

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Sam Ravnborg]

> > 
> > ...
> > endif # NETDEVICES
> > 
> > config NETPOLL
> >         depends on NETDEVICES
> >         def_bool NETCONSOLE
> > 
> > config NETPOLL_TRAP
> >         bool "Netpoll traffic trapping"
> >         default n
> >         depends on NETPOLL
> > 
> > config NET_POLL_CONTROLLER
> >         def_bool NETPOLL
> >         depends on NETPOLL
> > 
> > 
> > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > still doesn't check for NETDEVICES dependency.
> 
> That's odd. Adding Sam to the cc:.

select is evil....
select will by brute force set a symbol equal to 'y' without
visiting the dependencies.
So abusing select you are able to select a symbol FOO even 
if FOO depends on BAR that is not set.

In general use select only for non-visible symbols (no promts anywhere)
and for symbols with no dependencies.
That will limit the suefullness but on the other hand avoid the illegal
configurations all over.

kconfig should one day warn about such things but I have not fel inclined
to dive into the matters hoping that Roman does one day.

	Sam

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Satyam Sharma]

Hi,


On Thu, 2 Aug 2007, Sam Ravnborg wrote:

> > > 
> > > ...
> > > endif # NETDEVICES
> > > 
> > > config NETPOLL
> > >         depends on NETDEVICES
> > >         def_bool NETCONSOLE
> > > 
> > > config NETPOLL_TRAP
> > >         bool "Netpoll traffic trapping"
> > >         default n
> > >         depends on NETPOLL
> > > 
> > > config NET_POLL_CONTROLLER
> > >         def_bool NETPOLL
> > >         depends on NETPOLL
> > > 
> > > 
> > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > > still doesn't check for NETDEVICES dependency.
> > 
> > That's odd. Adding Sam to the cc:.

I just noticed this thread, but I wonder what the fuss is all
about :-) Kconfig dependencies are easy, really -- any code that
pulls in code from elsewhere, must explicitly "depends on" it.
It is possible to use "select" as well, but could lead to breakages
as discussed to death on at least 64592 other threads on LKML already
and hence should only be used for library-like code that does not
have any dependencies itself.


> select is evil....
> select will by brute force set a symbol equal to 'y' without
> visiting the dependencies.
> So abusing select you are able to select a symbol FOO even 
> if FOO depends on BAR that is not set.
> 
> In general use select only for non-visible symbols (no promts anywhere)
> and for symbols with no dependencies.
> That will limit the suefullness but on the other hand avoid the illegal
> configurations all over.

The problem with using "depends on" is that your config symbol becomes
invisible unless the dependency has already been selected.

So, there's a workaround: make the ultimate config symbol itself depend
upon the grand-dependency (excuse the nomenclature) and just "select"
the immediate-parent-dependency, i.e. the following:

CONFIG_BAZ
	...

CONFIG BAR
	depends on BAZ

CONFIG_FOO
	depends on BAZ
	select BAR

is perfectly legal, and doesn't cause any build problems. Perhaps such a
solution makes sense here as well?


> kconfig should one day warn about such things but I have not fel inclined
> to dive into the matters hoping that Roman does one day.

Yup, I've wanted to do this myself, in fact I wanted to implement an idea
I had in mind ( http://lkml.org/lkml/2007/5/16/257 ) but for some reason
I tend to stay away from stuff in scripts/ :-)


Satyam

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Satyam Sharma]

[ Read through the thread, looked at Kconfig files,
  did some tests. Adding Kconfig experts to Cc: list. ]


> On Thu, 2 Aug 2007, Sam Ravnborg wrote:
> 
> > > > 
> > > > ...
> > > > endif # NETDEVICES
> > > > 
> > > > config NETPOLL
> > > >         depends on NETDEVICES
> > > >         def_bool NETCONSOLE
> > > > 
> > > > config NETPOLL_TRAP
> > > >         bool "Netpoll traffic trapping"
> > > >         default n
> > > >         depends on NETPOLL
> > > > 
> > > > config NET_POLL_CONTROLLER
> > > >         def_bool NETPOLL
> > > >         depends on NETPOLL


Gargh, what we're seeing here is a whole bunch of bugs, I think. First
I thought this must be one of those randconfig-producing-wrong-configs
issues, but surprisingly, running "make oldconfig" on this .config on
a fresh 2.6.23-rc1-mm1 tree didn't change anything in the .config.


Kconfig bug #1:
===============

Which means, although:

*****
menuconfig BAZ

if BAZ

config BAR

endif
*****

is widely believed (by most folks, I've heard this on several threads,
and as written in the comment in drivers/net/Kconfig) to be equivalent to:

*****
menuconfig BAZ

if BAZ
endif

config BAR
	depends on BAZ
*****

this is *not* enforced by "make oldconfig"! And hence, the NETPOLL &&
!NETDEVICES situation we're seeing here.

[ We could also categorize this as a bug in Kconfig's "if", fwiw. ]


Kconfig bug #2:
===============

config FOO
	def_bool BAR

is supposed to ensure that FOO == BAR (as Matt mentioned earlier).

However, even this is *not* enforced by "make oldconfig". And hence,
the NETPOLL && !NET_POLL_CONTROLLER situation we're seeing here.

In fact, I believe it's possible to even pass a NETCONSOLE but
!NETPOLL kind of .config through "make oldconfig" but it still won't
catch it, and build breakages *will* occur.

[ We could also categorize this as a bug in Kconfig's "def_bool", fwiw. ]

Possibly, we could also decide to just blame "randconfig" for the whole
issue, and forget about these, because I think it's highly unlikely
(though not impossible) for people with "real" .configs to hit the
problems we saw above.


KGDBOE bug #1:
==============

config KGDBOE in lib/Kconfig.kgdb must also "depend on" NETDEVICES,
and select NET_POLL_CONTROLLER also.


KGDBOE bug #2:
==============

config KGDBOE_NOMODULE is a sad, sad option, and must be killed. The
"if !KGDBOE_NOMODULE" in KGDBOE must be removed, and it must lose its
dependency on "m".


Satyam

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Thu, Aug 02, 2007 at 04:02:21PM +0530, Satyam Sharma wrote:
> Hi,
> 
> 
> On Thu, 2 Aug 2007, Sam Ravnborg wrote:
> 
> > > > 
> > > > ...
> > > > endif # NETDEVICES
> > > > 
> > > > config NETPOLL
> > > >         depends on NETDEVICES
> > > >         def_bool NETCONSOLE
> > > > 
> > > > config NETPOLL_TRAP
> > > >         bool "Netpoll traffic trapping"
> > > >         default n
> > > >         depends on NETPOLL
> > > > 
> > > > config NET_POLL_CONTROLLER
> > > >         def_bool NETPOLL
> > > >         depends on NETPOLL
> > > > 
> > > > 
> > > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > > > still doesn't check for NETDEVICES dependency.
> > > 
> > > That's odd. Adding Sam to the cc:.
> 
> I just noticed this thread, but I wonder what the fuss is all
> about :-) Kconfig dependencies are easy, really -- any code that
> pulls in code from elsewhere, must explicitly "depends on" it.
> It is possible to use "select" as well, but could lead to breakages
> as discussed to death on at least 64592 other threads on LKML already
> and hence should only be used for library-like code that does not
> have any dependencies itself.

So, it seems at least one time not enough (or maybe it would be better
to write this 1 time only, but in Documentation/).

> 
> 
> > select is evil....
> > select will by brute force set a symbol equal to 'y' without
> > visiting the dependencies.
> > So abusing select you are able to select a symbol FOO even 
> > if FOO depends on BAR that is not set.
> > 
> > In general use select only for non-visible symbols (no promts anywhere)
> > and for symbols with no dependencies.
> > That will limit the suefullness but on the other hand avoid the illegal
> > configurations all over.
> 
> The problem with using "depends on" is that your config symbol becomes
> invisible unless the dependency has already been selected.
> 
> So, there's a workaround: make the ultimate config symbol itself depend
> upon the grand-dependency (excuse the nomenclature) and just "select"
> the immediate-parent-dependency, i.e. the following:
> 
> CONFIG_BAZ
> 	...
> 
> CONFIG BAR
> 	depends on BAZ
> 
> CONFIG_FOO
> 	depends on BAZ
> 	select BAR
> 
> is perfectly legal, and doesn't cause any build problems. Perhaps such a
> solution makes sense here as well?
> 
> 
> > kconfig should one day warn about such things but I have not fel inclined
> > to dive into the matters hoping that Roman does one day.
> 
> Yup, I've wanted to do this myself, in fact I wanted to implement an idea
> I had in mind ( http://lkml.org/lkml/2007/5/16/257 ) but for some reason
> I tend to stay away from stuff in scripts/ :-)

How often "common" developer has to make such decisions in Kconfig?
Probably no more than once per year. So, it's fair to blame anybody
for not reading lkml to find if there are some bugs or
recommendations before using apparently simple tool? I think there
is usually some README for such things (maybe in Documentation/)?

Thanks,
Jarek P.

PS: if it's so easy and it's enough to read only 64592 lkml messages,
I wonder why Andrew, who knows all lkml, and reads more messages per
hour, cared to remember mainly one short conclusion...

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Satyam Sharma]

On Thu, 2 Aug 2007, Jarek Poplawski wrote:

> On Thu, Aug 02, 2007 at 04:02:21PM +0530, Satyam Sharma wrote:
> [...]
> How often "common" developer has to make such decisions in Kconfig?
> Probably no more than once per year. So, it's fair to blame anybody
> for not reading lkml to find if there are some bugs or
> recommendations before using apparently simple tool? I think there
> is usually some README for such things (maybe in Documentation/)?

Whoops, I only said that in humour, probably should've snuck in a
smiley or two. Definitely not blaming anybody. Apologies to anyone
who felt offended, sorry, nothing such was intended, I assure.

Satyam

Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )

[Jarek Poplawski]

On Thu, Aug 02, 2007 at 05:26:12PM +0530, Satyam Sharma wrote:
...
> Whoops, I only said that in humour, probably should've snuck in a
> smiley or two. Definitely not blaming anybody. Apologies to anyone
> who felt offended, sorry, nothing such was intended, I assure.

I see you probably didn't notice my smileys too. I need them so often
that I've to abbreviate them with something like this: ",.?!"
But, I'm also sorry if you felt confused I felt offended etc...

Jarek P.

[PATCH] docs: note about select in kconfig-language.txt

[Jarek Poplawski]

Hi,

If there are no other plans of kconfig docs update I think this
message of yours should be useful enough. I made minimal fixes,
so if they are wrong or you prefer it otherwise, I'm OK with any
change to this proposal, including replacement with something
else.

Thanks & regards,
Jarek P.

On Thu, Aug 02, 2007 at 11:36:59AM +0200, Sam Ravnborg wrote:
...
> > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > > still doesn't check for NETDEVICES dependency.
> > 
> > That's odd. Adding Sam to the cc:.
> 
> select is evil....
> select will by brute force set a symbol equal to 'y' without
> visiting the dependencies.
> So abusing select you are able to select a symbol FOO even 
> if FOO depends on BAR that is not set.
> 
> In general use select only for non-visible symbols (no promts anywhere)
> and for symbols with no dependencies.
> That will limit the suefullness but on the other hand avoid the illegal
> configurations all over.
> 
> kconfig should one day warn about such things but I have not fel inclined
> to dive into the matters hoping that Roman does one day.
> 
> 	Sam
> 
----------->

Subject: docs: a warning note about select in kconfig-language.txt

A warning note of Sam Ravnborg about kconfig's select evilness,
dependencies and the future (slightly corrected).

Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
Cc: Sam Ravnborg <sam@ravnborg.org>

---

diff -Nu9r 2.6.23-rc1-/Documentation/kbuild/kconfig-language.txt 2.6.23-rc1/Documentation/kbuild/kconfig-language.txt
--- 2.6.23-rc1-/Documentation/kbuild/kconfig-language.txt	2007-07-09 01:32:17.000000000 +0200
+++ 2.6.23-rc1/Documentation/kbuild/kconfig-language.txt	2007-08-06 12:50:34.000000000 +0200
@@ -92,18 +92,27 @@
 
 - reverse dependencies: "select" <symbol> ["if" <expr>]
   While normal dependencies reduce the upper limit of a symbol (see
   below), reverse dependencies can be used to force a lower limit of
   another symbol. The value of the current menu symbol is used as the
   minimal value <symbol> can be set to. If <symbol> is selected multiple
   times, the limit is set to the largest selection.
   Reverse dependencies can only be used with boolean or tristate
   symbols.
+  Note:
+	select is evil.... select will by brute force set a symbol
+	equal to 'y' without visiting the dependencies. So abusing
+	select you are able to select a symbol FOO even if FOO depends
+	on BAR that is not set. In general use select only for
+	non-visible symbols (no promts anywhere) and for symbols with
+	no dependencies. That will limit the usefulness but on the
+	other hand avoid the illegal configurations all over. kconfig
+	should one day warn about such things.
 
 - numerical ranges: "range" <symbol> <symbol> ["if" <expr>]
   This allows to limit the range of possible input values for int
   and hex symbols. The user can only input a value which is larger than
   or equal to the first symbol and smaller than or equal to the second
   symbol.
 
 - help text: "help" or "---help---"
   This defines a help text. The end of the help text is determined by