Signed-off-by: Denis Cheng <[email protected]>
---
net/netlink/af_netlink.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 5681ce3..8bb14e3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -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;
--
1.5.3.rc7
with the macro max provided by <linux/kernel.h>, so changed its name to a more proper one: limit
Signed-off-by: Denis Cheng <[email protected]>
---
net/netlink/af_netlink.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 8bb14e3..ac3b100 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -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;
--
1.5.3.rc7
if the table is always fixed size with MAX_LINKS entries, why not use a statically
allocated table straightforwardly?
Signed-off-by: Denis Cheng <[email protected]>
---
net/netlink/af_netlink.c | 11 ++---------
1 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index ac3b100..c527f87 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -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);
--
1.5.3.rc7
On 9/2/07, David Newall <[email protected]> wrote:
> Denis Cheng wrote
> > + 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].
--
Denis Cheng
From: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:57 +0800
> Signed-off-by: Denis Cheng <[email protected]>
Applied to net-2.6.24, thanks.
From: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:58 +0800
> with the macro max provided by <linux/kernel.h>, so changed its name to a more proper one: limit
>
> Signed-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.
From: Denis Cheng <[email protected]>
Date: Sun, 2 Sep 2007 03:45:59 +0800
> if the table is always fixed size with MAX_LINKS entries, 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.
On 9/17/07, David Miller <[email protected]> wrote:
> From: Denis Cheng <[email protected]>
> Date: Sun, 2 Sep 2007 03:45:58 +0800
>
> > with the macro max provided by <linux/kernel.h>, so changed its name to a more proper one: limit
> >
> > Signed-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.
>
> Nonetheless, applied to net-2.6.24, thanks.
>
--
Denis Cheng