Skip to contentSkip to conversation

netpoll: use static branch

RFC In review Stephen Hemminger 2 participants 4 messages Sep 18, 2012 – Sep 20, 2012

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

RFC 1/1 netpoll: use static branch

This is an attempt to optimize netpoll when not used.

Since distro's enable everything and netpoll is only occasionally used, improve performance by getting netpoll condition check out of the Rx fastpath.

Compile tested only, I have no real use for netpoll.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/linux/netpoll.h |   28 ++++++++++++++++++++--------
 net/core/netpoll.c      |    8 +++++++-
 2 files changed, 27 insertions(+), 9 deletions(-)
include/linux/netpoll.h +20 -8
@@ -66,10 +66,16 @@ static inline void netpoll_send_skb(stru
 
 
 #ifdef CONFIG_NETPOLL
+extern struct static_key netpoll_needed;
+
 static inline bool netpoll_rx_on(struct sk_buff *skb)
 {
-	struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	struct netpoll_info *npinfo;
+
+	if (static_key_true(&netpoll_needed))
+		return false;
 
+	npinfo = rcu_dereference_bh(skb->dev->npinfo);
 	return npinfo && (!list_empty(&npinfo->rx_np) || npinfo->rx_flags);
 }
 

@@ -79,12 +85,14 @@ static inline bool netpoll_rx(struct sk_
 	unsigned long flags;
 	bool ret = false;
 
-	local_irq_save(flags);
+	if (static_key_true(&netpoll_needed))
+		return false;
 
-	if (!netpoll_rx_on(skb))
+	local_irq_save(flags);
+	npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	if (!npinfo || (list_empty(&npinfo->rx_np) && !npinfo->rx_flags))
 		goto out;
 
-	npinfo = rcu_dereference_bh(skb->dev->npinfo);
 	spin_lock(&npinfo->rx_lock);
 	/* check rx_flags again with the lock held */
 	if (npinfo->rx_flags && __netpoll_rx(skb, npinfo))

@@ -96,11 +104,15 @@ out:
 	return ret;
 }
 
-static inline int netpoll_receive_skb(struct sk_buff *skb)
+static inline bool netpoll_receive_skb(struct sk_buff *skb)
 {
+	if (static_key_true(&netpoll_needed))
+		return false;
+
 	if (!list_empty(&skb->dev->napi_list))
 		return netpoll_rx(skb);
-	return 0;
+
+	return false;
 }
 
 static inline void *netpoll_poll_lock(struct napi_struct *napi)

@@ -139,9 +151,9 @@ static inline bool netpoll_rx_on(struct
 {
 	return false;
 }
-static inline int netpoll_receive_skb(struct sk_buff *skb)
+static inline bool netpoll_receive_skb(struct sk_buff *skb)
 {
-	return 0;
+	return false;
 }
 static inline void *netpoll_poll_lock(struct napi_struct *napi)
 {
net/core/netpoll.c +7 -1
@@ -67,6 +67,8 @@ module_param(carrier_timeout, uint, 0644
 #define np_notice(np, fmt, ...)				\
 	pr_notice("%s: " fmt, np->name, ##__VA_ARGS__)
 
+struct static_key netpoll_needed __read_mostly;
+
 static void queue_process(struct work_struct *work)
 {
 	struct netpoll_info *npinfo =

@@ -786,6 +788,8 @@ int __netpoll_setup(struct netpoll *np,
 		npinfo->rx_flags |= NETPOLL_RX_ENABLED;
 		list_add_tail(&np->rx, &npinfo->rx_np);
 		spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+
+		static_key_slow_inc(&netpoll_needed);
 	}
 
 	/* last thing to do is link it to the net device structure */

@@ -926,8 +930,10 @@ void __netpoll_cleanup(struct netpoll *n
 	if (!list_empty(&npinfo->rx_np)) {
 		spin_lock_irqsave(&npinfo->rx_lock, flags);
 		list_del(&np->rx);
-		if (list_empty(&npinfo->rx_np))
+		if (list_empty(&npinfo->rx_np)) {
 			npinfo->rx_flags &= ~NETPOLL_RX_ENABLED;
+			static_key_slow_dec(&netpoll_needed);
+		}
 		spin_unlock_irqrestore(&npinfo->rx_lock, flags);
 	}
#1 lore raw
Quoted 22 lines · Stephen Hemminger

This is an attempt to optimize netpoll when not used.

Since distro's enable everything and netpoll is only occasionally used, improve performance by getting netpoll condition check out of the Rx fastpath.

Compile tested only, I have no real use for netpoll.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/linux/netpoll.h |   28 ++++++++++++++++++++--------
 net/core/netpoll.c      |    8 +++++++-
 2 files changed, 27 insertions(+), 9 deletions(-)
include/linux/netpoll.h +0 -0
@@ -66,10 +66,16 @@ static inline void netpoll_send_skb(stru
 
 
 #ifdef CONFIG_NETPOLL

include/linux/netpoll.h static inline void netpoll_send_skb(stru · line 77

Quoted 10 lines
include/linux/netpoll.h +6 -1
+extern struct static_key netpoll_needed;
+
 static inline bool netpoll_rx_on(struct sk_buff *skb)
 {
-	struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	struct netpoll_info *npinfo;
+
+	if (static_key_true(&netpoll_needed))
+		return false;

I think we should use static_key_false() here, as netpoll is an "unlikely" code path.

Using static branch is a good idea though.

Thanks.

author
#2 lore raw
Quoted 37 lines · On Wed, 19 Sep 2012 12:50:10 +0800 Cong Wang <[email protected]> wrote:
Quoted 22 lines · Stephen Hemminger

This is an attempt to optimize netpoll when not used.

Since distro's enable everything and netpoll is only occasionally used, improve performance by getting netpoll condition check out of the Rx fastpath.

Compile tested only, I have no real use for netpoll.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/linux/netpoll.h |   28 ++++++++++++++++++++--------
 net/core/netpoll.c      |    8 +++++++-
 2 files changed, 27 insertions(+), 9 deletions(-)
include/linux/netpoll.h +0 -0
@@ -66,10 +66,16 @@ static inline void netpoll_send_skb(stru
 
 
 #ifdef CONFIG_NETPOLL

include/linux/netpoll.h static inline void netpoll_send_skb(stru · line 77

Quoted 10 lines
include/linux/netpoll.h +6 -1
+extern struct static_key netpoll_needed;
+
 static inline bool netpoll_rx_on(struct sk_buff *skb)
 {
-	struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	struct netpoll_info *npinfo;
+
+	if (static_key_true(&netpoll_needed))
+		return false;

I think we should use static_key_false() here, as netpoll is an "unlikely" code path.

Quoted 3 lines

Using static branch is a good idea though.

Thanks.

But static_key_true is just a wrapper around !static_key_false()

#3 lore raw
Quoted 40 lines · Stephen Hemminger
Quoted 37 lines · On Wed, 19 Sep 2012 12:50:10 +0800 Cong Wang <[email protected]> wrote:
Quoted 22 lines · Stephen Hemminger

This is an attempt to optimize netpoll when not used.

Since distro's enable everything and netpoll is only occasionally used, improve performance by getting netpoll condition check out of the Rx fastpath.

Compile tested only, I have no real use for netpoll.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/linux/netpoll.h |   28 ++++++++++++++++++++--------
 net/core/netpoll.c      |    8 +++++++-
 2 files changed, 27 insertions(+), 9 deletions(-)
include/linux/netpoll.h +0 -0
@@ -66,10 +66,16 @@ static inline void netpoll_send_skb(stru
 
 
 #ifdef CONFIG_NETPOLL

include/linux/netpoll.h static inline void netpoll_send_skb(stru · line 77

Quoted 10 lines
include/linux/netpoll.h +6 -1
+extern struct static_key netpoll_needed;
+
 static inline bool netpoll_rx_on(struct sk_buff *skb)
 {
-	struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+	struct netpoll_info *npinfo;
+
+	if (static_key_true(&netpoll_needed))
+		return false;

I think we should use static_key_false() here, as netpoll is an "unlikely" code path.

Quoted 5 lines
Quoted 3 lines

Using static branch is a good idea though.

Thanks.

But static_key_true is just a wrapper around !static_key_false()

For !HAVE_JUMP_LABEL arch, the definition is below:

static __always_inline bool static_key_false(struct static_key *key)

{
        if (unlikely(atomic_read(&key->enabled)) > 0)
                return true;
        return false;
}

static __always_inline bool static_key_true(struct static_key *key)

{       
        if (likely(atomic_read(&key->enabled)) > 0)
                return true;
        return false;
}