diff mbox series

[ovs-dev] ofproto-dpif-upcall: Only call ovsrcu_postpone() on active actions

Message ID 371de981d1aeabb8746e2732bb38e02aac6d75e2.1524137013.git.echaudro@redhat.com
State Accepted
Headers show
Series [ovs-dev] ofproto-dpif-upcall: Only call ovsrcu_postpone() on active actions | expand

Commit Message

Eelco Chaudron April 19, 2018, 11:24 a.m. UTC
Currently, ovsrcu_postpone() is called even with a NULL argument,
i.e. when there is no data to be freed. This is causing additional
overhead because work is scheduled for the urcu thread. This change
avoids adding the postpone callback if no work needs to be done.

This especially helps for the OVS-DPDK case where the PMD threads
might no longer have to do a write() due to the latch_set(), and thus
saving a syscall.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 ofproto/ofproto-dpif-upcall.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Ben Pfaff April 19, 2018, 4:30 p.m. UTC | #1
On Thu, Apr 19, 2018 at 01:24:06PM +0200, Eelco Chaudron wrote:
> Currently, ovsrcu_postpone() is called even with a NULL argument,
> i.e. when there is no data to be freed. This is causing additional
> overhead because work is scheduled for the urcu thread. This change
> avoids adding the postpone callback if no work needs to be done.
> 
> This especially helps for the OVS-DPDK case where the PMD threads
> might no longer have to do a write() due to the latch_set(), and thus
> saving a syscall.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>

Wow, good catch!

Thanks, I applied this to master.
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 00160e1ee..d26f201f4 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1621,8 +1621,13 @@  ukey_get_actions(struct udpif_key *ukey, const struct nlattr **actions, size_t *
 static void
 ukey_set_actions(struct udpif_key *ukey, const struct ofpbuf *actions)
 {
-    ovsrcu_postpone(ofpbuf_delete,
-                    ovsrcu_get_protected(struct ofpbuf *, &ukey->actions));
+    struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *,
+                                                      &ukey->actions);
+
+    if (old_actions) {
+        ovsrcu_postpone(ofpbuf_delete, old_actions);
+    }
+
     ovsrcu_set(&ukey->actions, ofpbuf_clone(actions));
 }