* [-mm patch] one e1000 driver should be enough for everyone
[not found] <20070725040304.111550f4.akpm@linux-foundation.org>
@ 2007-07-25 13:36 ` Adrian Bunk
2007-07-25 13:48 ` Jeff Garzik
2007-07-25 18:15 ` 2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error Adrian Bunk
2007-07-28 15:44 ` NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 ) Gabriel C
2 siblings, 1 reply; 40+ messages in thread
From: Adrian Bunk @ 2007-07-25 13:36 UTC (permalink / raw)
To: Andrew Morton, Auke Kok, jgarzik; +Cc: linux-kernel, e1000-devel, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 13:36 ` [-mm patch] one e1000 driver should be enough for everyone Adrian Bunk
@ 2007-07-25 13:48 ` Jeff Garzik
2007-07-25 14:46 ` Adrian Bunk
0 siblings, 1 reply; 40+ messages in thread
From: Jeff Garzik @ 2007-07-25 13:48 UTC (permalink / raw)
To: Adrian Bunk; +Cc: e1000-devel, netdev, Andrew Morton, Auke Kok, linux-kernel
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/
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 13:48 ` Jeff Garzik
@ 2007-07-25 14:46 ` Adrian Bunk
2007-07-25 15:05 ` Jeff Garzik
0 siblings, 1 reply; 40+ messages in thread
From: Adrian Bunk @ 2007-07-25 14:46 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, Auke Kok, linux-kernel, e1000-devel, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 14:46 ` Adrian Bunk
@ 2007-07-25 15:05 ` Jeff Garzik
2007-07-25 15:21 ` Kok, Auke
0 siblings, 1 reply; 40+ messages in thread
From: Jeff Garzik @ 2007-07-25 15:05 UTC (permalink / raw)
To: Adrian Bunk; +Cc: e1000-devel, netdev, Andrew Morton, Auke Kok, linux-kernel
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/
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 15:05 ` Jeff Garzik
@ 2007-07-25 15:21 ` Kok, Auke
2007-07-25 15:23 ` Jeff Garzik
2007-07-25 20:50 ` Andrew Morton
0 siblings, 2 replies; 40+ messages in thread
From: Kok, Auke @ 2007-07-25 15:21 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Adrian Bunk, Andrew Morton, linux-kernel, e1000-devel, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 15:21 ` Kok, Auke
@ 2007-07-25 15:23 ` Jeff Garzik
2007-07-25 20:50 ` Andrew Morton
1 sibling, 0 replies; 40+ messages in thread
From: Jeff Garzik @ 2007-07-25 15:23 UTC (permalink / raw)
To: Kok, Auke; +Cc: Adrian Bunk, Andrew Morton, linux-kernel, e1000-devel, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* 2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error
[not found] <20070725040304.111550f4.akpm@linux-foundation.org>
2007-07-25 13:36 ` [-mm patch] one e1000 driver should be enough for everyone Adrian Bunk
@ 2007-07-25 18:15 ` Adrian Bunk
2007-07-26 8:46 ` [RFT] fib_trie: cleanup Stephen Hemminger
2007-07-28 15:44 ` NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 ) Gabriel C
2 siblings, 1 reply; 40+ messages in thread
From: Adrian Bunk @ 2007-07-25 18:15 UTC (permalink / raw)
To: Andrew Morton, Paul E. McKenney, Ingo Molnar, Josh Triplett
Cc: linux-kernel, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [-mm patch] one e1000 driver should be enough for everyone
2007-07-25 15:21 ` Kok, Auke
2007-07-25 15:23 ` Jeff Garzik
@ 2007-07-25 20:50 ` Andrew Morton
1 sibling, 0 replies; 40+ messages in thread
From: Andrew Morton @ 2007-07-25 20:50 UTC (permalink / raw)
To: Kok, Auke; +Cc: e1000-devel, netdev, linux-kernel, Jeff Garzik, Adrian Bunk
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/
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFT] fib_trie: cleanup
2007-07-25 18:15 ` 2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error Adrian Bunk
@ 2007-07-26 8:46 ` Stephen Hemminger
2007-07-26 8:49 ` David Miller
` (2 more replies)
0 siblings, 3 replies; 40+ messages in thread
From: Stephen Hemminger @ 2007-07-26 8:46 UTC (permalink / raw)
To: Adrian Bunk, Robert Olsson
Cc: Andrew Morton, Paul E. McKenney, Ingo Molnar, Josh Triplett,
netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFT] fib_trie: cleanup
2007-07-26 8:46 ` [RFT] fib_trie: cleanup Stephen Hemminger
@ 2007-07-26 8:49 ` David Miller
2007-07-26 10:32 ` Robert Olsson
2007-07-26 9:04 ` Andrew Morton
2007-07-26 10:43 ` [RFT] fib_trie: macro cleanup Stephen Hemminger
2 siblings, 1 reply; 40+ messages in thread
From: David Miller @ 2007-07-26 8:49 UTC (permalink / raw)
To: shemminger; +Cc: bunk, Robert.Olsson, akpm, paulmck, mingo, josh, netdev
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?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFT] fib_trie: cleanup
2007-07-26 8:46 ` [RFT] fib_trie: cleanup Stephen Hemminger
2007-07-26 8:49 ` David Miller
@ 2007-07-26 9:04 ` Andrew Morton
2007-07-26 9:15 ` Stephen Hemminger
2007-07-26 10:49 ` [RFC] fib_trie: whitespace cleanup Stephen Hemminger
2007-07-26 10:43 ` [RFT] fib_trie: macro cleanup Stephen Hemminger
2 siblings, 2 replies; 40+ messages in thread
From: Andrew Morton @ 2007-07-26 9:04 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Adrian Bunk, Robert Olsson, Paul E. McKenney, Ingo Molnar,
Josh Triplett, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFT] fib_trie: cleanup
2007-07-26 9:04 ` Andrew Morton
@ 2007-07-26 9:15 ` Stephen Hemminger
2007-07-26 10:49 ` [RFC] fib_trie: whitespace cleanup Stephen Hemminger
1 sibling, 0 replies; 40+ messages in thread
From: Stephen Hemminger @ 2007-07-26 9:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Adrian Bunk, Robert Olsson, Paul E. McKenney, Ingo Molnar,
Josh Triplett, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFT] fib_trie: cleanup
2007-07-26 8:49 ` David Miller
@ 2007-07-26 10:32 ` Robert Olsson
0 siblings, 0 replies; 40+ messages in thread
From: Robert Olsson @ 2007-07-26 10:32 UTC (permalink / raw)
To: David Miller
Cc: shemminger, bunk, Robert.Olsson, akpm, paulmck, mingo, josh,
netdev
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%
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFT] fib_trie: macro cleanup
2007-07-26 8:46 ` [RFT] fib_trie: cleanup Stephen Hemminger
2007-07-26 8:49 ` David Miller
2007-07-26 9:04 ` Andrew Morton
@ 2007-07-26 10:43 ` Stephen Hemminger
2007-07-26 10:54 ` Andrew Morton
2 siblings, 1 reply; 40+ messages in thread
From: Stephen Hemminger @ 2007-07-26 10:43 UTC (permalink / raw)
To: David S. Miller
Cc: Adrian Bunk, Robert Olsson, Andrew Morton, Paul E. McKenney,
Ingo Molnar, Josh Triplett, netdev
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",
^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC] fib_trie: whitespace cleanup
2007-07-26 9:04 ` Andrew Morton
2007-07-26 9:15 ` Stephen Hemminger
@ 2007-07-26 10:49 ` Stephen Hemminger
2007-07-26 15:44 ` Paul E. McKenney
1 sibling, 1 reply; 40+ messages in thread
From: Stephen Hemminger @ 2007-07-26 10:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Adrian Bunk, Robert Olsson, Paul E. McKenney, Ingo Molnar,
Josh Triplett, netdev
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:
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFT] fib_trie: macro cleanup
2007-07-26 10:43 ` [RFT] fib_trie: macro cleanup Stephen Hemminger
@ 2007-07-26 10:54 ` Andrew Morton
0 siblings, 0 replies; 40+ messages in thread
From: Andrew Morton @ 2007-07-26 10:54 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Adrian Bunk, Robert Olsson, Paul E. McKenney,
Ingo Molnar, Josh Triplett, netdev
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>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC] fib_trie: whitespace cleanup
2007-07-26 10:49 ` [RFC] fib_trie: whitespace cleanup Stephen Hemminger
@ 2007-07-26 15:44 ` Paul E. McKenney
2007-07-27 4:56 ` Andrew Morton
0 siblings, 1 reply; 40+ messages in thread
From: Paul E. McKenney @ 2007-07-26 15:44 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andrew Morton, Adrian Bunk, Robert Olsson, Ingo Molnar,
Josh Triplett, netdev
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:
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC] fib_trie: whitespace cleanup
2007-07-26 15:44 ` Paul E. McKenney
@ 2007-07-27 4:56 ` Andrew Morton
2007-07-30 17:07 ` Paul E. McKenney
0 siblings, 1 reply; 40+ messages in thread
From: Andrew Morton @ 2007-07-27 4:56 UTC (permalink / raw)
To: paulmck
Cc: Stephen Hemminger, Adrian Bunk, Robert Olsson, Ingo Molnar,
Josh Triplett, netdev
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);
}
^ permalink raw reply [flat|nested] 40+ messages in thread
* NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
[not found] <20070725040304.111550f4.akpm@linux-foundation.org>
2007-07-25 13:36 ` [-mm patch] one e1000 driver should be enough for everyone Adrian Bunk
2007-07-25 18:15 ` 2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error Adrian Bunk
@ 2007-07-28 15:44 ` Gabriel C
2007-07-28 17:26 ` Andrew Morton
2 siblings, 1 reply; 40+ messages in thread
From: Gabriel C @ 2007-07-28 15:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-28 15:44 ` NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 ) Gabriel C
@ 2007-07-28 17:26 ` Andrew Morton
2007-07-28 18:42 ` Gabriel C
0 siblings, 1 reply; 40+ messages in thread
From: Andrew Morton @ 2007-07-28 17:26 UTC (permalink / raw)
To: Gabriel C; +Cc: linux-kernel, netdev, jason.wessel, amitkale
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
_
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-28 17:26 ` Andrew Morton
@ 2007-07-28 18:42 ` Gabriel C
2007-07-31 8:32 ` Jarek Poplawski
0 siblings, 1 reply; 40+ messages in thread
From: Gabriel C @ 2007-07-28 18:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC] fib_trie: whitespace cleanup
2007-07-27 4:56 ` Andrew Morton
@ 2007-07-30 17:07 ` Paul E. McKenney
0 siblings, 0 replies; 40+ messages in thread
From: Paul E. McKenney @ 2007-07-30 17:07 UTC (permalink / raw)
To: Andrew Morton
Cc: Stephen Hemminger, Adrian Bunk, Robert Olsson, Ingo Molnar,
Josh Triplett, netdev
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-28 18:42 ` Gabriel C
@ 2007-07-31 8:32 ` Jarek Poplawski
2007-07-31 10:14 ` Gabriel C
0 siblings, 1 reply; 40+ messages in thread
From: Jarek Poplawski @ 2007-07-31 8:32 UTC (permalink / raw)
To: Gabriel C; +Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 8:32 ` Jarek Poplawski
@ 2007-07-31 10:14 ` Gabriel C
2007-07-31 11:44 ` Jason Wessel
2007-07-31 12:17 ` Jarek Poplawski
0 siblings, 2 replies; 40+ messages in thread
From: Gabriel C @ 2007-07-31 10:14 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 10:14 ` Gabriel C
@ 2007-07-31 11:44 ` Jason Wessel
2007-07-31 12:47 ` Jarek Poplawski
2007-07-31 12:17 ` Jarek Poplawski
1 sibling, 1 reply; 40+ messages in thread
From: Jason Wessel @ 2007-07-31 11:44 UTC (permalink / raw)
To: Gabriel C; +Cc: Jarek Poplawski, Andrew Morton, linux-kernel, netdev, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 10:14 ` Gabriel C
2007-07-31 11:44 ` Jason Wessel
@ 2007-07-31 12:17 ` Jarek Poplawski
2007-07-31 15:05 ` Gabriel C
1 sibling, 1 reply; 40+ messages in thread
From: Jarek Poplawski @ 2007-07-31 12:17 UTC (permalink / raw)
To: Gabriel C; +Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 11:44 ` Jason Wessel
@ 2007-07-31 12:47 ` Jarek Poplawski
0 siblings, 0 replies; 40+ messages in thread
From: Jarek Poplawski @ 2007-07-31 12:47 UTC (permalink / raw)
To: Jason Wessel; +Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 12:17 ` Jarek Poplawski
@ 2007-07-31 15:05 ` Gabriel C
2007-08-01 9:59 ` Jarek Poplawski
0 siblings, 1 reply; 40+ messages in thread
From: Gabriel C @ 2007-07-31 15:05 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-07-31 15:05 ` Gabriel C
@ 2007-08-01 9:59 ` Jarek Poplawski
2007-08-02 2:02 ` Matt Mackall
0 siblings, 1 reply; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-01 9:59 UTC (permalink / raw)
To: Gabriel C; +Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-01 9:59 ` Jarek Poplawski
@ 2007-08-02 2:02 ` Matt Mackall
2007-08-02 9:00 ` Jarek Poplawski
2007-08-02 9:36 ` Sam Ravnborg
0 siblings, 2 replies; 40+ messages in thread
From: Matt Mackall @ 2007-08-02 2:02 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, jason.wessel,
amitkale, Sam Ravnborg
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 2:02 ` Matt Mackall
@ 2007-08-02 9:00 ` Jarek Poplawski
2007-08-02 15:59 ` Matt Mackall
2007-08-02 9:36 ` Sam Ravnborg
1 sibling, 1 reply; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-02 9:00 UTC (permalink / raw)
To: Matt Mackall
Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, jason.wessel,
amitkale, Sam Ravnborg
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 2:02 ` Matt Mackall
2007-08-02 9:00 ` Jarek Poplawski
@ 2007-08-02 9:36 ` Sam Ravnborg
2007-08-02 10:32 ` Satyam Sharma
2007-08-06 11:51 ` [PATCH] docs: note about select in kconfig-language.txt Jarek Poplawski
1 sibling, 2 replies; 40+ messages in thread
From: Sam Ravnborg @ 2007-08-02 9:36 UTC (permalink / raw)
To: Matt Mackall
Cc: Jarek Poplawski, Gabriel C, Andrew Morton, linux-kernel, netdev,
jason.wessel, amitkale
> >
> > ...
> > 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 9:36 ` Sam Ravnborg
@ 2007-08-02 10:32 ` Satyam Sharma
2007-08-02 11:40 ` Satyam Sharma
2007-08-02 11:40 ` Jarek Poplawski
2007-08-06 11:51 ` [PATCH] docs: note about select in kconfig-language.txt Jarek Poplawski
1 sibling, 2 replies; 40+ messages in thread
From: Satyam Sharma @ 2007-08-02 10:32 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Matt Mackall, Jarek Poplawski, Gabriel C, Andrew Morton,
linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 10:32 ` Satyam Sharma
@ 2007-08-02 11:40 ` Satyam Sharma
2007-08-02 11:40 ` Jarek Poplawski
1 sibling, 0 replies; 40+ messages in thread
From: Satyam Sharma @ 2007-08-02 11:40 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Matt Mackall, Jarek Poplawski, Gabriel C, Andrew Morton,
Linux Kernel Mailing List, netdev, jason.wessel, amitkale, zippel,
jengelh
[ 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 10:32 ` Satyam Sharma
2007-08-02 11:40 ` Satyam Sharma
@ 2007-08-02 11:40 ` Jarek Poplawski
2007-08-02 11:56 ` Satyam Sharma
1 sibling, 1 reply; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-02 11:40 UTC (permalink / raw)
To: Satyam Sharma
Cc: Sam Ravnborg, Matt Mackall, Gabriel C, Andrew Morton,
linux-kernel, netdev, jason.wessel, amitkale
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...
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 11:40 ` Jarek Poplawski
@ 2007-08-02 11:56 ` Satyam Sharma
2007-08-02 12:52 ` Jarek Poplawski
0 siblings, 1 reply; 40+ messages in thread
From: Satyam Sharma @ 2007-08-02 11:56 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Sam Ravnborg, Matt Mackall, Gabriel C, Andrew Morton,
linux-kernel, netdev, jason.wessel, amitkale
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 11:56 ` Satyam Sharma
@ 2007-08-02 12:52 ` Jarek Poplawski
0 siblings, 0 replies; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-02 12:52 UTC (permalink / raw)
To: Satyam Sharma
Cc: Sam Ravnborg, Matt Mackall, Gabriel C, Andrew Morton,
linux-kernel, netdev, jason.wessel, amitkale
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 9:00 ` Jarek Poplawski
@ 2007-08-02 15:59 ` Matt Mackall
2007-08-03 7:30 ` Jarek Poplawski
0 siblings, 1 reply; 40+ messages in thread
From: Matt Mackall @ 2007-08-02 15:59 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, jason.wessel,
amitkale, Sam Ravnborg
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
2007-08-02 15:59 ` Matt Mackall
@ 2007-08-03 7:30 ` Jarek Poplawski
0 siblings, 0 replies; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-03 7:30 UTC (permalink / raw)
To: Matt Mackall
Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, jason.wessel,
amitkale, Sam Ravnborg
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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] docs: note about select in kconfig-language.txt
2007-08-02 9:36 ` Sam Ravnborg
2007-08-02 10:32 ` Satyam Sharma
@ 2007-08-06 11:51 ` Jarek Poplawski
1 sibling, 0 replies; 40+ messages in thread
From: Jarek Poplawski @ 2007-08-06 11:51 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Matt Mackall, Gabriel C, Andrew Morton, linux-kernel, netdev,
jason.wessel, amitkale, Satyam Sharma
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
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2007-08-06 11:51 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20070725040304.111550f4.akpm@linux-foundation.org>
2007-07-25 13:36 ` [-mm patch] one e1000 driver should be enough for everyone Adrian Bunk
2007-07-25 13:48 ` Jeff Garzik
2007-07-25 14:46 ` Adrian Bunk
2007-07-25 15:05 ` Jeff Garzik
2007-07-25 15:21 ` Kok, Auke
2007-07-25 15:23 ` Jeff Garzik
2007-07-25 20:50 ` Andrew Morton
2007-07-25 18:15 ` 2.6.23-rc1-mm1: net/ipv4/fib_trie.c compile error Adrian Bunk
2007-07-26 8:46 ` [RFT] fib_trie: cleanup Stephen Hemminger
2007-07-26 8:49 ` David Miller
2007-07-26 10:32 ` Robert Olsson
2007-07-26 9:04 ` Andrew Morton
2007-07-26 9:15 ` Stephen Hemminger
2007-07-26 10:49 ` [RFC] fib_trie: whitespace cleanup Stephen Hemminger
2007-07-26 15:44 ` Paul E. McKenney
2007-07-27 4:56 ` Andrew Morton
2007-07-30 17:07 ` Paul E. McKenney
2007-07-26 10:43 ` [RFT] fib_trie: macro cleanup Stephen Hemminger
2007-07-26 10:54 ` Andrew Morton
2007-07-28 15:44 ` NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 ) Gabriel C
2007-07-28 17:26 ` Andrew Morton
2007-07-28 18:42 ` Gabriel C
2007-07-31 8:32 ` Jarek Poplawski
2007-07-31 10:14 ` Gabriel C
2007-07-31 11:44 ` Jason Wessel
2007-07-31 12:47 ` Jarek Poplawski
2007-07-31 12:17 ` Jarek Poplawski
2007-07-31 15:05 ` Gabriel C
2007-08-01 9:59 ` Jarek Poplawski
2007-08-02 2:02 ` Matt Mackall
2007-08-02 9:00 ` Jarek Poplawski
2007-08-02 15:59 ` Matt Mackall
2007-08-03 7:30 ` Jarek Poplawski
2007-08-02 9:36 ` Sam Ravnborg
2007-08-02 10:32 ` Satyam Sharma
2007-08-02 11:40 ` Satyam Sharma
2007-08-02 11:40 ` Jarek Poplawski
2007-08-02 11:56 ` Satyam Sharma
2007-08-02 12:52 ` Jarek Poplawski
2007-08-06 11:51 ` [PATCH] docs: note about select in kconfig-language.txt Jarek Poplawski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).