linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
@ 2016-04-25 13:25 Denys Vlasenko
  2016-04-25 13:39 ` Antonio Quartulli
  2016-04-29 21:15 ` Sven Eckelmann
  0 siblings, 2 replies; 6+ messages in thread
From: Denys Vlasenko @ 2016-04-25 13:25 UTC (permalink / raw)
  To: Marek Lindner
  Cc: Denys Vlasenko, Simon Wunderlich, Antonio Quartulli,
	Sven Eckelmann, b.a.t.m.a.n, linux-kernel

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 <dvlasenk@redhat.com>
CC: Marek Lindner <mareklindner@neomailbox.ch>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Antonio Quartulli <a@unstable.cc>
CC: Sven Eckelmann <sven@narfation.org>
CC: b.a.t.m.a.n@lists.open-mesh.org
CC: linux-kernel@vger.kernel.org
---
 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

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

* Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
  2016-04-25 13:25 [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes Denys Vlasenko
@ 2016-04-25 13:39 ` Antonio Quartulli
  2016-04-25 13:45   ` Denys Vlasenko
  2017-10-23 16:41   ` Sven Eckelmann
  2016-04-29 21:15 ` Sven Eckelmann
  1 sibling, 2 replies; 6+ messages in thread
From: Antonio Quartulli @ 2016-04-25 13:39 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Marek Lindner, Simon Wunderlich, Sven Eckelmann, b.a.t.m.a.n,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 542 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
  2016-04-25 13:39 ` Antonio Quartulli
@ 2016-04-25 13:45   ` Denys Vlasenko
  2016-04-25 14:19     ` Antonio Quartulli
  2017-10-23 16:41   ` Sven Eckelmann
  1 sibling, 1 reply; 6+ messages in thread
From: Denys Vlasenko @ 2016-04-25 13:45 UTC (permalink / raw)
  To: Antonio Quartulli
  Cc: Marek Lindner, Simon Wunderlich, Sven Eckelmann, b.a.t.m.a.n,
	linux-kernel

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.

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

* Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
  2016-04-25 13:45   ` Denys Vlasenko
@ 2016-04-25 14:19     ` Antonio Quartulli
  0 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2016-04-25 14:19 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Marek Lindner, Simon Wunderlich, Sven Eckelmann, b.a.t.m.a.n,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1088 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
  2016-04-25 13:25 [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes Denys Vlasenko
  2016-04-25 13:39 ` Antonio Quartulli
@ 2016-04-29 21:15 ` Sven Eckelmann
  1 sibling, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2016-04-29 21:15 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Marek Lindner, Simon Wunderlich, Antonio Quartulli, b.a.t.m.a.n,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1205 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 <dvlasenk@redhat.com>
> CC: Marek Lindner <mareklindner@neomailbox.ch>
> CC: Simon Wunderlich <sw@simonwunderlich.de>
> CC: Antonio Quartulli <a@unstable.cc>
> CC: Sven Eckelmann <sven@narfation.org>
> CC: b.a.t.m.a.n@lists.open-mesh.org
> CC: linux-kernel@vger.kernel.org
> ---
>  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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes
  2016-04-25 13:39 ` Antonio Quartulli
  2016-04-25 13:45   ` Denys Vlasenko
@ 2017-10-23 16:41   ` Sven Eckelmann
  1 sibling, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2017-10-23 16:41 UTC (permalink / raw)
  To: Antonio Quartulli
  Cc: Denys Vlasenko, Marek Lindner, Simon Wunderlich, b.a.t.m.a.n,
	linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3253 bytes --]

On Montag, 25. April 2016 21:39:50 CEST 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?

Tested it with 2x OM5P-ACv2 (LEDE 17.01) which were connected via ethernet 
cable on port eth1. iperf was started with reduced MSS to increase the packet 
count on a PC which was connected to eth0:

    $ iperf -c 192.168.10.1 -t 30 -i 1 -P8 -M 536 -y C > test01.csv

Another PC (running on the eth0 of the other device) was running the iperf-server.

The network configuration was modified to automatically enable bat0 when link was detected on eth1:

    config interface 'loopback'
            option ifname 'lo'
            option proto 'static'
            option ipaddr '127.0.0.1'
            option netmask '255.0.0.0'
    
    config globals 'globals'
            option ula_prefix 'fdcb:4e2a:b274::/48'
    
    config interface 'lan'
            option type 'bridge'
            option ifname 'eth0 bat0'
            option proto 'static'
            option ipaddr '192.168.1.2'
            option netmask '255.255.255.0'
            option ip6assign '60'
    
    config interface 'batnet'
            option mtu '1532'
            option proto 'batadv'
            option ifname 'eth1'
            option mesh 'bat0'



test run | without patch (MiBit/s) | with patch (MiBit/s) | without batman-adv (MiBit/s)
---------+-------------------------+----------------------+-----------------------------
       1 |                     289 |                  271 |                          501
       2 |                     260 |                  271 |                          500
       3 |                     262 |                  272 |                          501
       4 |                     262 |                  270 |                          500
       5 |                     267 |                  270 |                          499
       6 |                     270 |                  269 |                          500
       7 |                     271 |                  268 |                          501
       8 |                     271 |                  268 |                          501
       9 |                     271 |                  269 |                          500
      10 |                     271 |                  268 |                          497
      11 |                     261 |                  269 |                          500
      12 |                     271 |                  264 |                          499
      13 |                     274 |                  268 |                          502
=========+=========================+======================+=============================
     avg |                     269 |                  269 |                          500   

Kind regards,
	Sven

[-- Attachment #1.2: tests.tar.xz --]
[-- Type: application/x-xz-compressed-tar, Size: 62496 bytes --]

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2017-10-23 16:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-25 13:25 [PATCH] batman-adv: Deinline batadv_orig_hash_find, save 9024 bytes Denys Vlasenko
2016-04-25 13:39 ` Antonio Quartulli
2016-04-25 13:45   ` Denys Vlasenko
2016-04-25 14:19     ` Antonio Quartulli
2017-10-23 16:41   ` Sven Eckelmann
2016-04-29 21:15 ` Sven Eckelmann

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