net: get rid of spin_trylock() in net_tx_action()

Note: Tom Herbert posted almost same patch 3 months back, but for
different reasons.

The reasons we want to get rid of this spin_trylock() are :

1) Under high qdisc pressure, the spin_trylock() has almost no
chance to succeed.

2) We loop multiple times in softirq handler, eventually reaching
the max retry count (10), and we schedule ksoftirqd.

Since we want to adhere more strictly to ksoftirqd being waked up in
the future (https://lwn.net/Articles/687617/), better avoid spurious
wakeups.

3) calls to __netif_reschedule() dirty the cache line containing
q->next_sched, slowing down the owner of qdisc.

4) RT kernels can not use the spin_trylock() here.

With help of busylock, we get the qdisc spinlock fast enough, and
the trylock trick brings only performance penalty.

Depending on qdisc setup, I observed a gain of up to 19 % in qdisc
performance (1016600 pps instead of 853400 pps, using prio+tbf+fq_codel)

("mpstat -I SCPU 1" is much happier now)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Tom Herbert <tom@herbertland.com>
Acked-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/core/dev.c b/net/core/dev.c
index 904ff431d570e3cfc0e6255480bd25f3c9dec2f3..896b686d19669d61a04bdccf6b0b71c0537cca81 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2253,7 +2253,7 @@ int netif_get_num_default_rss_queues(void)
 }
 EXPORT_SYMBOL(netif_get_num_default_rss_queues);
 
-static inline void __netif_reschedule(struct Qdisc *q)
+static void __netif_reschedule(struct Qdisc *q)
 {
        struct softnet_data *sd;
        unsigned long flags;
@@ -3898,22 +3898,14 @@ static void net_tx_action(struct softirq_action *h)
                        head = head->next_sched;
 
                        root_lock = qdisc_lock(q);
-                       if (spin_trylock(root_lock)) {
-                               smp_mb__before_atomic();
-                               clear_bit(__QDISC_STATE_SCHED,
-                                         &q->state);
-                               qdisc_run(q);
-                               spin_unlock(root_lock);
-                       } else {
-                               if (!test_bit(__QDISC_STATE_DEACTIVATED,
-                                             &q->state)) {
-                                       __netif_reschedule(q);
-                               } else {
-                                       smp_mb__before_atomic();
-                                       clear_bit(__QDISC_STATE_SCHED,
-                                                 &q->state);
-                               }
-                       }
+                       spin_lock(root_lock);
+                       /* We need to make sure head->next_sched is read
+                        * before clearing __QDISC_STATE_SCHED
+                        */
+                       smp_mb__before_atomic();
+                       clear_bit(__QDISC_STATE_SCHED, &q->state);
+                       qdisc_run(q);
+                       spin_unlock(root_lock);
                }
        }
 }