netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net core: optimize netdev_create_hash()
@ 2013-03-15 16:32 Wei Yang
  2013-03-15 21:39 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Wei Yang @ 2013-03-15 16:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: Wei Yang

netdev_create_hash() is divded into two steps:
1. allocate space for hash_head
2. initialize hash_head->first to NULL for each hash_head

This patch merge the two steps into one step.

When allocating the space for hash_head, it will use kzalloc() instead of
kmalloc(). Then hash_head->first is set to NULL during the allocation step,
which means it is not necessary to call INIT_HLIST_HEAD() for each hash_head.

This will:
1. reduce the code size
2. reduce the run time of iteration on initializing hash_head array
---
 net/core/dev.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index f64e439..79f0666 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6564,13 +6564,15 @@ EXPORT_SYMBOL(netdev_increment_features);
 
 static struct hlist_head *netdev_create_hash(void)
 {
-	int i;
 	struct hlist_head *hash;
 
-	hash = kmalloc(sizeof(*hash) * NETDEV_HASHENTRIES, GFP_KERNEL);
-	if (hash != NULL)
-		for (i = 0; i < NETDEV_HASHENTRIES; i++)
-			INIT_HLIST_HEAD(&hash[i]);
+	hash = kzalloc(sizeof(*hash) * NETDEV_HASHENTRIES, GFP_KERNEL);
+
+	/*
+	 * hash[i]->first is set to NULL in kzalloc()
+	 *
+	 * INIT_HLIST_HEAD(&hash[i]) is not necessary now
+	 */
 
 	return hash;
 }
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] net core: optimize netdev_create_hash()
  2013-03-15 16:32 [PATCH] net core: optimize netdev_create_hash() Wei Yang
@ 2013-03-15 21:39 ` David Miller
       [not found]   ` <20130316000051.GA5941@richard.(null)>
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2013-03-15 21:39 UTC (permalink / raw)
  To: weiyang; +Cc: netdev

From: Wei Yang <weiyang@linux.vnet.ibm.com>
Date: Sat, 16 Mar 2013 00:32:11 +0800

> netdev_create_hash() is divded into two steps:
> 1. allocate space for hash_head
> 2. initialize hash_head->first to NULL for each hash_head
> 
> This patch merge the two steps into one step.
> 
> When allocating the space for hash_head, it will use kzalloc() instead of
> kmalloc(). Then hash_head->first is set to NULL during the allocation step,
> which means it is not necessary to call INIT_HLIST_HEAD() for each hash_head.
> 
> This will:
> 1. reduce the code size
> 2. reduce the run time of iteration on initializing hash_head array

Please do not ever post changes like this.

This makes assumtions about the list head implementation that
we should not make.  If someone adds a debugging facility that
causes lists to be initialized differently, it will be broken
for this hash table now.

The reason the abstractions exist is so that people do not
do things like you are trying to do.

If you want to add an interface in the generic list facilities which
performs this simplifcation, fine.  Implement it and post such a
suggestion to linux-kernel, and then once such a facility is present
in Linus's tree we can start using it in the networkng code.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] net core: optimize netdev_create_hash()
       [not found]   ` <20130316000051.GA5941@richard.(null)>
@ 2013-03-16  1:19     ` Wei Yang
  0 siblings, 0 replies; 3+ messages in thread
From: Wei Yang @ 2013-03-16  1:19 UTC (permalink / raw)
  To: David Miller, netdev

[resend for syntax error in Message-ID]

David, 

Thanks for pointing out the error, it really helps.

I guess last mail is still blocked by the mail server, so I resend it.
Hope it works now.

On Sat, Mar 16, 2013 at 08:00:51AM +0800, Wei Yang wrote:
On Fri, Mar 15, 2013 at 05:39:30PM -0400, David Miller wrote:
>From: Wei Yang <weiyang@linux.vnet.ibm.com>
>Date: Sat, 16 Mar 2013 00:32:11 +0800
>
>> netdev_create_hash() is divded into two steps:
>> 1. allocate space for hash_head
>> 2. initialize hash_head->first to NULL for each hash_head
>> 
>> This patch merge the two steps into one step.
>> 
>> When allocating the space for hash_head, it will use kzalloc() instead of
>> kmalloc(). Then hash_head->first is set to NULL during the allocation step,
>> which means it is not necessary to call INIT_HLIST_HEAD() for each hash_head.
>> 
>> This will:
>> 1. reduce the code size
>> 2. reduce the run time of iteration on initializing hash_head array
>
>Please do not ever post changes like this.
>
>This makes assumtions about the list head implementation that
>we should not make.  If someone adds a debugging facility that
>causes lists to be initialized differently, it will be broken
>for this hash table now.

Yes, this is based on the assumtion. 
And agree, this will break the behavior.

>
>The reason the abstractions exist is so that people do not
>do things like you are trying to do.
>
>If you want to add an interface in the generic list facilities which
>performs this simplifcation, fine.  Implement it and post such a
>suggestion to linux-kernel, and then once such a facility is present
>in Linus's tree we can start using it in the networkng code.

Thanks for your suggestion and kindness.

-- 
Richard Yang
Help you, Help me

-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-03-16  1:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-15 16:32 [PATCH] net core: optimize netdev_create_hash() Wei Yang
2013-03-15 21:39 ` David Miller
     [not found]   ` <20130316000051.GA5941@richard.(null)>
2013-03-16  1:19     ` Wei Yang

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).