2016-04-25 13:25:32

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes

This function compiles to 473 bytes of machine code.
21 callsites.

text data bss dec hex filename
95903266 20860288 35991552 152755106 91adba2 vmlinux_before
95894242 20860288 35991552 152746082 91ab862 vmlinux

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Marek Lindner <[email protected]>
CC: Simon Wunderlich <[email protected]>
CC: Antonio Quartulli <[email protected]>
CC: Sven Eckelmann <[email protected]>
CC: [email protected]
CC: [email protected]
---
net/batman-adv/originator.c | 29 +++++++++++++++++++++++++++++
net/batman-adv/originator.h | 31 ++-----------------------------
2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index e4cbb07..bcf78f1 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -47,6 +47,36 @@
/* hash class keys */
static struct lock_class_key batadv_orig_hash_lock_class_key;

+struct batadv_orig_node *
+batadv_orig_hash_find(struct batadv_priv *bat_priv, const void *data)
+{
+ struct batadv_hashtable *hash = bat_priv->orig_hash;
+ struct hlist_head *head;
+ struct batadv_orig_node *orig_node, *orig_node_tmp = NULL;
+ int index;
+
+ if (!hash)
+ return NULL;
+
+ index = batadv_choose_orig(data, hash->size);
+ head = &hash->table[index];
+
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
+ if (!batadv_compare_eth(orig_node, data))
+ continue;
+
+ if (!kref_get_unless_zero(&orig_node->refcount))
+ continue;
+
+ orig_node_tmp = orig_node;
+ break;
+ }
+ rcu_read_unlock();
+
+ return orig_node_tmp;
+}
+
static void batadv_purge_orig(struct work_struct *work);

/**
diff --git a/net/batman-adv/originator.h b/net/batman-adv/originator.h
index 4e8b67f..db7a87d 100644
--- a/net/batman-adv/originator.h
+++ b/net/batman-adv/originator.h
@@ -96,34 +96,7 @@ static inline u32 batadv_choose_orig(const void *data, u32 size)
return hash % size;
}

-static inline struct batadv_orig_node *
-batadv_orig_hash_find(struct batadv_priv *bat_priv, const void *data)
-{
- struct batadv_hashtable *hash = bat_priv->orig_hash;
- struct hlist_head *head;
- struct batadv_orig_node *orig_node, *orig_node_tmp = NULL;
- int index;
-
- if (!hash)
- return NULL;
-
- index = batadv_choose_orig(data, hash->size);
- head = &hash->table[index];
-
- rcu_read_lock();
- hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
- if (!batadv_compare_eth(orig_node, data))
- continue;
-
- if (!kref_get_unless_zero(&orig_node->refcount))
- continue;
-
- orig_node_tmp = orig_node;
- break;
- }
- rcu_read_unlock();
-
- return orig_node_tmp;
-}
+struct batadv_orig_node *
+batadv_orig_hash_find(struct batadv_priv *bat_priv, const void *data);

#endif /* _NET_BATMAN_ADV_ORIGINATOR_H_ */
--
1.8.1.4


2016-04-25 13:45:25

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes

On 04/25/2016 03:39 PM, Antonio Quartulli wrote:
> On Mon, Apr 25, 2016 at 03:25:22PM +0200, Denys Vlasenko wrote:
>> This function compiles to 473 bytes of machine code.
>> 21 callsites.
>>
>> text data bss dec hex filename
>> 95903266 20860288 35991552 152755106 91adba2 vmlinux_before
>> 95894242 20860288 35991552 152746082 91ab862 vmlinux
>
> Hi Danys,
>
> thanks for your patch. This function is used in a several performance critical
> code paths (i.e. packet forwarding).
>
> Are we sure we are not losing in performance here?

Is this a common case?

if (!hash)
return NULL;

If yes, then we can inline this part only.

2016-04-25 14:06:19

by Antonio Quartulli

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes

On Mon, Apr 25, 2016 at 03:25:22PM +0200, Denys Vlasenko wrote:
> This function compiles to 473 bytes of machine code.
> 21 callsites.
>
> text data bss dec hex filename
> 95903266 20860288 35991552 152755106 91adba2 vmlinux_before
> 95894242 20860288 35991552 152746082 91ab862 vmlinux

Hi Danys,

thanks for your patch. This function is used in a several performance critical
code paths (i.e. packet forwarding).

Are we sure we are not losing in performance here?

Cheers,

--
Antonio Quartulli


Attachments:
(No filename) (523.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2016-04-25 14:20:09

by Antonio Quartulli

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes

On Mon, Apr 25, 2016 at 03:45:20PM +0200, Denys Vlasenko wrote:
> On 04/25/2016 03:39 PM, Antonio Quartulli wrote:
> > On Mon, Apr 25, 2016 at 03:25:22PM +0200, Denys Vlasenko wrote:
> >> This function compiles to 473 bytes of machine code.
> >> 21 callsites.
> >>
> >> text data bss dec hex filename
> >> 95903266 20860288 35991552 152755106 91adba2 vmlinux_before
> >> 95894242 20860288 35991552 152746082 91ab862 vmlinux
> >
> > Hi Danys,
> >
> > thanks for your patch. This function is used in a several performance critical
> > code paths (i.e. packet forwarding).
> >
> > Are we sure we are not losing in performance here?
>
> Is this a common case?
>
> if (!hash)
> return NULL;
>
> If yes, then we can inline this part only.

Unfortunately not: this case is rather rare at runtime.
These hash tables are initialized when the batman virtual interface is created
and should be freed only upon interface shutdown.

(actually I believe this might be a good candidate for an unlikely())

Cheers,

--
Antonio Quartulli


Attachments:
(No filename) (1.03 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2016-04-29 21:15:40

by Sven Eckelmann

[permalink] [raw]
Subject: Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes

On Monday 25 April 2016 15:25:22 Denys Vlasenko wrote:
> This function compiles to 473 bytes of machine code.
> 21 callsites.
>
> text data bss dec hex filename
> 95903266 20860288 35991552 152755106 91adba2 vmlinux_before
> 95894242 20860288 35991552 152746082 91ab862 vmlinux
>
> Signed-off-by: Denys Vlasenko <[email protected]>
> CC: Marek Lindner <[email protected]>
> CC: Simon Wunderlich <[email protected]>
> CC: Antonio Quartulli <[email protected]>
> CC: Sven Eckelmann <[email protected]>
> CC: [email protected]
> CC: [email protected]
> ---
> net/batman-adv/originator.c | 29 +++++++++++++++++++++++++++++
> net/batman-adv/originator.h | 31 ++-----------------------------
> 2 files changed, 31 insertions(+), 29 deletions(-)
>

This patch should also remove following includes from originator.h:

-#include <linux/kref.h>
-#include <linux/rculist.h>
-#include <linux/rcupdate.h>
-#include <linux/stddef.h>
-#include "hash.h"

and add following includes to originator.c (please keep them in alphabetical
order):

+#include <linux/rcupdate.h>
+#include <linux/stddef.h>

Kind regards,
Sven


Attachments:
signature.asc (819.00 B)
This is a digitally signed message part.