netlink: use the macro min(x,y) provided by <linux/kernel.h> instead
Subsystems (2)
PATCH 3/3 netlink: use a statically allocated nl_table instead
In reply to Denis Chengif the table is always fixed size with MAX_LINKS entries, why not use a statically allocated table straightforwardly?
--- net/netlink/af_netlink.c | 11 ++--------- 1 files changed, 2 insertions(+), 9 deletions(-)
net/netlink/af_netlink.c +2 -9
@@ -115,7 +115,7 @@ struct netlink_table {int registered; }; -static struct netlink_table *nl_table; +static struct netlink_table nl_table[MAX_LINKS ];static DECLARE_WAIT_QUEUE_HEAD (nl_table_wait);
@@ -1894,10 +1894,6 @@ static int __init netlink_proto_init(void)BUILD_BUG_ON (sizeof (struct netlink_skb_parms ) >sizeof (dummy_skb->cb)); - nl_table =kcalloc (MAX_LINKS ,sizeof (*nl_table),GFP_KERNEL ); -if (!nl_table) - goto panic; -if (num_physpages >= (128 *1024 )) limit = num_physpages >> (21 -PAGE_SHIFT );else
@@ -1915,8 +1911,7 @@ static int __init netlink_proto_init(void)while (i-- >0 )nl_pid_hash_free (nl_table[i].hash.table,1 *sizeof (*hash->table)); -kfree (nl_table); - goto panic; + goto out; }memset (hash->table,0 ,1 *sizeof (*hash->table)); hash->max_shift = order;
@@ -1933,8 +1928,6 @@ static int __init netlink_proto_init(void)rtnetlink_init (); out:return err; -panic: -panic ("netlink_init: Cannot allocate nl_table\n" ); }core_initcall (netlink_proto_init);
Signature · 1 line
1.5.3.rc7
PATCH 1/3 netlink: use the macro min(x,y) provided by <linux/kernel.h> instead
--- net/netlink/af_netlink.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
net/netlink/af_netlink.c +1 -1
@@ -1905,7 +1905,7 @@ static int __init netlink_proto_init(void) order =get_bitmask_order (max) -1 +PAGE_SHIFT ; max = (1UL << order) /sizeof (struct hlist_head ); - order =get_bitmask_order (max >UINT_MAX ?UINT_MAX : max) -1 ; + order =get_bitmask_order (min (max, (unsigned long )UINT_MAX )) -1 ;for (i =0 ; i <MAX_LINKS ; i++) {struct nl_pid_hash *hash = &nl_table[i].hash;
Signature · 1 line
1.5.3.rc7
PATCH 2/3 netlink: the temp variable name max is ambiguous
with the macro max provided by <linux/kernel.h>, so changed its name to a more proper one: limit
--- net/netlink/af_netlink.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
net/netlink/af_netlink.c +6 -6
@@ -1885,7 +1885,7 @@ static int __init netlink_proto_init(void) {struct sk_buff *dummy_skb;int i; -unsigned long max; +unsigned long limit;unsigned int order;int err =proto_register (&netlink_proto,0 );
@@ -1899,13 +1899,13 @@ static int __init netlink_proto_init(void) goto panic;if (num_physpages >= (128 *1024 )) - max = num_physpages >> (21 -PAGE_SHIFT ); + limit = num_physpages >> (21 -PAGE_SHIFT );else - max = num_physpages >> (23 -PAGE_SHIFT ); + limit = num_physpages >> (23 -PAGE_SHIFT ); - order =get_bitmask_order (max) -1 +PAGE_SHIFT ; - max = (1UL << order) /sizeof (struct hlist_head ); - order =get_bitmask_order (min (max, (unsigned long )UINT_MAX )) -1 ; + order =get_bitmask_order (limit) -1 +PAGE_SHIFT ; + limit = (1UL << order) /sizeof (struct hlist_head ); + order =get_bitmask_order (min (limit, (unsigned long )UINT_MAX )) -1 ;for (i =0 ; i <MAX_LINKS ; i++) {struct nl_pid_hash *hash = &nl_table[i].hash;
Signature · 1 line
1.5.3.rc7
Quoted 5 lines · David Newall
Denis Cheng wrote
Quoted 2 lines
+ order =
get_bitmask_order(min(max, (unsigned long)UINT_MAX))- 1;Why doesn't this clash with the max define, also in
linux/kernel.h?
They indeed don't clash,
the cpp included by gcc is intelligent enough, it know the function-style definition of max in kernel.h, that's different from the auto variable max here, so they don't clash with each other,
But I think the variable name "max" here is ambiguous, I changed it to "limit", see my following patch [PATCH 2/3].
Signature · 1 line
Denis Cheng
From: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:57 +0800
Quoted 1 line
Signed-off-by Denis Cheng <[email protected]>
Applied to net-2.6.24, thanks.
David Miller
maintainerRe: [PATCH 2/3] netlink: the temp variable name max is ambiguous
In reply to Denis ChengFrom: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:58 +0800
Quoted 3 lines
with the macro max provided by <
linux/kernel.h>, so changed its name to a more proper one: limitSigned-off-by Denis Cheng <[email protected]>
Not strictly necessary because CPP knows to differentiate between 'max(' and plain 'max' when evaluating if a CPP macro should be expanded or not.
Nonetheless, applied to net-2.6.24, thanks.
David Miller
maintainerRe: [PATCH 3/3] netlink: use a statically allocated nl_table instead
In reply to Denis ChengFrom: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:59 +0800
Quoted 4 lines
if the table is always fixed size with
MAX_LINKSentries, why not use a statically allocated table straightforwardly?Signed-off-by Denis Cheng <[email protected]>
I made the explicit decision to dynamically allocate because many systems have limits on how large the kernel image can be and therefore the less we statically allocate huge tables (constant size or not) the better.
Lockdep is the worst offender, for example, it's completely awful. It consumes 4MB of kernel BSS space when enabled on a 64-bit platform.
Patch not applied.
Re: [PATCH 2/3] netlink: the temp variable name max is ambiguous
In reply to David MillerQuoted 10 lines · David Miller
From: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:58 +0800Quoted 3 lines
with the macro max provided by <
linux/kernel.h>, so changed its name to a more proper one: limitSigned-off-by Denis Cheng <[email protected]>Not strictly necessary because CPP knows to differentiate between 'max(' and plain 'max' when evaluating if a CPP macro should be expanded or not.
I also know the GNU CPP is intelligent, but people are often not. I just think the avoidance to use human ambiguous names could give more readability.
Quoted 3 lines
Nonetheless, applied to net-2.6.24, thanks.
Signature · 1 line
Denis Cheng