Commit 04157469b7b8 ("net: Use static_key for XPS maps") introduced a
static key for XPS, but the increments/decrements don't match.

First, the static key's counter is incremented once for each queue, but
only decremented once for a whole batch of queues, leading to large
unbalances.

Second, the xps_rxqs_needed key is decremented whenever we reset a batch
of queues, whether they had any rxqs mapping or not, so that if we setup
cpu-XPS on em1 and RXQS-XPS on em2, resetting the queues on em1 would
decrement the xps_rxqs_needed key.

This reworks the accounting scheme so that the xps_needed key is
incremented only once for each type of XPS for all the queues on a
device, and the xps_rxqs_needed key is incremented only once for all
queues. This is sufficient to let us retrieve queues via
get_xps_queue().

This patch introduces a new reset_xps_maps(), which reinitializes and
frees the appropriate map (xps_rxqs_map or xps_cpus_map), and drops a
reference to the needed keys:
 - both xps_needed and xps_rxqs_needed, in case of rxqs maps,
 - only xps_needed, in case of CPU maps.

Now, we also need to call reset_xps_maps() at the end of
__netif_set_xps_queue() when there's no active map left, for example
when writing '00000000,00000000' to all queues' xps_rxqs setting.

Fixes: 04157469b7b8 ("net: Use static_key for XPS maps")
Signed-off-by: Sabrina Dubroca <s...@queasysnail.net>
---
 net/core/dev.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 32a63f4c3a92..3470e7fff1f4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2175,6 +2175,20 @@ static bool remove_xps_queue_cpu(struct net_device *dev,
        return active;
 }
 
+static void reset_xps_maps(struct net_device *dev,
+                          struct xps_dev_maps *dev_maps,
+                          bool is_rxqs_map)
+{
+       if (is_rxqs_map) {
+               static_key_slow_dec_cpuslocked(&xps_rxqs_needed);
+               RCU_INIT_POINTER(dev->xps_rxqs_map, NULL);
+       } else {
+               RCU_INIT_POINTER(dev->xps_cpus_map, NULL);
+       }
+       static_key_slow_dec_cpuslocked(&xps_needed);
+       kfree_rcu(dev_maps, rcu);
+}
+
 static void clean_xps_maps(struct net_device *dev, const unsigned long *mask,
                           struct xps_dev_maps *dev_maps, unsigned int nr_ids,
                           u16 offset, u16 count, bool is_rxqs_map)
@@ -2186,13 +2200,8 @@ static void clean_xps_maps(struct net_device *dev, const 
unsigned long *mask,
             j < nr_ids;)
                active |= remove_xps_queue_cpu(dev, dev_maps, j, offset,
                                               count);
-       if (!active) {
-               if (is_rxqs_map)
-                       RCU_INIT_POINTER(dev->xps_rxqs_map, NULL);
-               else
-                       RCU_INIT_POINTER(dev->xps_cpus_map, NULL);
-               kfree_rcu(dev_maps, rcu);
-       }
+       if (!active)
+               reset_xps_maps(dev, dev_maps, is_rxqs_map);
 
        if (!is_rxqs_map) {
                for (i = offset + (count - 1); count--; i--) {
@@ -2236,10 +2245,6 @@ static void netif_reset_xps_queues(struct net_device 
*dev, u16 offset,
                       false);
 
 out_no_maps:
-       if (static_key_enabled(&xps_rxqs_needed))
-               static_key_slow_dec_cpuslocked(&xps_rxqs_needed);
-
-       static_key_slow_dec_cpuslocked(&xps_needed);
        mutex_unlock(&xps_map_mutex);
        cpus_read_unlock();
 }
@@ -2357,9 +2362,12 @@ int __netif_set_xps_queue(struct net_device *dev, const 
unsigned long *mask,
        if (!new_dev_maps)
                goto out_no_new_maps;
 
-       static_key_slow_inc_cpuslocked(&xps_needed);
-       if (is_rxqs_map)
-               static_key_slow_inc_cpuslocked(&xps_rxqs_needed);
+       if (!dev_maps) {
+               /* Increment static keys at most once per type */
+               static_key_slow_inc_cpuslocked(&xps_needed);
+               if (is_rxqs_map)
+                       static_key_slow_inc_cpuslocked(&xps_rxqs_needed);
+       }
 
        for (j = -1; j = netif_attrmask_next(j, possible_mask, nr_ids),
             j < nr_ids;) {
@@ -2457,13 +2465,8 @@ int __netif_set_xps_queue(struct net_device *dev, const 
unsigned long *mask,
        }
 
        /* free map if not active */
-       if (!active) {
-               if (is_rxqs_map)
-                       RCU_INIT_POINTER(dev->xps_rxqs_map, NULL);
-               else
-                       RCU_INIT_POINTER(dev->xps_cpus_map, NULL);
-               kfree_rcu(dev_maps, rcu);
-       }
+       if (!active)
+               reset_xps_maps(dev, dev_maps, is_rxqs_map);
 
 out_no_maps:
        mutex_unlock(&xps_map_mutex);
-- 
2.19.2

Reply via email to