On Thu, Oct 27, 2016 at 8:40 AM, Alexander Duyck
<alexander.h.du...@intel.com> wrote:
> This patch updates the code for removing queues from the XPS map and makes
> it so that we can apply the code any time we change either the number of
> traffic classes or the mapping of a given block of queues. This way we
> avoid having queues pulling traffic from a foreign traffic class.
>
> Signed-off-by: Alexander Duyck <alexander.h.du...@intel.com>
> ---
> net/core/dev.c | 79
> ++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 56 insertions(+), 23 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d4d45bf..d124081 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1953,32 +1953,56 @@ static void netif_setup_tc(struct net_device *dev,
> unsigned int txq)
> #define xmap_dereference(P) \
> rcu_dereference_protected((P), lockdep_is_held(&xps_map_mutex))
>
> -static struct xps_map *remove_xps_queue(struct xps_dev_maps *dev_maps,
> - int cpu, u16 index)
> +static bool remove_xps_queue(struct xps_dev_maps *dev_maps,
> + int tci, u16 index)
> {
> struct xps_map *map = NULL;
> int pos;
>
> if (dev_maps)
> - map = xmap_dereference(dev_maps->cpu_map[cpu]);
> + map = xmap_dereference(dev_maps->cpu_map[tci]);
> + if (!map)
> + return false;
>
> - for (pos = 0; map && pos < map->len; pos++) {
> - if (map->queues[pos] == index) {
> - if (map->len > 1) {
> - map->queues[pos] = map->queues[--map->len];
> - } else {
> - RCU_INIT_POINTER(dev_maps->cpu_map[cpu],
> NULL);
> - kfree_rcu(map, rcu);
> - map = NULL;
> - }
> + for (pos = map->len; pos--;) {
> + if (map->queues[pos] != index)
> + continue;
> +
> + if (map->len > 1) {
> + map->queues[pos] = map->queues[--map->len];
> break;
> }
> +
> + RCU_INIT_POINTER(dev_maps->cpu_map[tci], NULL);
> + kfree_rcu(map, rcu);
> + return false;
> }
>
> - return map;
> + return true;
> }
>
> -static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
> +static bool remove_xps_queue_cpu(struct net_device *dev,
> + struct xps_dev_maps *dev_maps,
> + int cpu, u16 offset, u16 count)
> +{
> + bool active = false;
> + int i;
> +
> + count += offset;
> + i = count;
> +
> + do {
> + if (i-- == offset) {
> + active = true;
> + break;
> + }
> + } while (remove_xps_queue(dev_maps, cpu, i));
> +
IMO do/while's are hard to read. Does something like this work:
static bool remove_xps_queue_cpu(struct net_device *dev,
struct xps_dev_maps *dev_maps,
int cpu, u16 offset, u16 count)
{
int i;
for (i = count + offset; i > offset; i--)
if (!remove_xps_queue(dev_maps, cpu, i - 1))
break;
return (i == offset);
}
> + return active;
> +}
> +
> +static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
> + u16 count)
> {
> struct xps_dev_maps *dev_maps;
> int cpu, i;
> @@ -1990,21 +2014,16 @@ static void netif_reset_xps_queues_gt(struct
> net_device *dev, u16 index)
> if (!dev_maps)
> goto out_no_maps;
>
> - for_each_possible_cpu(cpu) {
> - for (i = index; i < dev->num_tx_queues; i++) {
> - if (!remove_xps_queue(dev_maps, cpu, i))
> - break;
> - }
> - if (i == dev->num_tx_queues)
> - active = true;
> - }
> + for_each_possible_cpu(cpu)
> + active |= remove_xps_queue_cpu(dev, dev_maps, cpu, offset,
> + count);
Maybe just do dumb "if (remove_xps...) active = true;"
>
> if (!active) {
> RCU_INIT_POINTER(dev->xps_maps, NULL);
> kfree_rcu(dev_maps, rcu);
> }
>
> - for (i = index; i < dev->num_tx_queues; i++)
> + for (i = offset + (count - 1); count--; i--)
> netdev_queue_numa_node_write(netdev_get_tx_queue(dev, i),
> NUMA_NO_NODE);
>
> @@ -2012,6 +2031,11 @@ static void netif_reset_xps_queues_gt(struct
> net_device *dev, u16 index)
> mutex_unlock(&xps_map_mutex);
> }
>
> +static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
> +{
> + netif_reset_xps_queues(dev, index, dev->num_tx_queues - index);
> +}
> +
> static struct xps_map *expand_xps_map(struct xps_map *map,
> int cpu, u16 index)
> {
> @@ -2175,6 +2199,9 @@ int netif_set_xps_queue(struct net_device *dev, const
> struct cpumask *mask,
> #endif
> void netdev_reset_tc(struct net_device *dev)
> {
> +#ifdef CONFIG_XPS
> + netif_reset_xps_queues_gt(dev, 0);
> +#endif
> dev->num_tc = 0;
> memset(dev->tc_to_txq, 0, sizeof(dev->tc_to_txq));
> memset(dev->prio_tc_map, 0, sizeof(dev->prio_tc_map));
> @@ -2186,6 +2213,9 @@ int netdev_set_tc_queue(struct net_device *dev, u8 tc,
> u16 count, u16 offset)
> if (tc >= dev->num_tc)
> return -EINVAL;
>
> +#ifdef CONFIG_XPS
> + netif_reset_xps_queues(dev, offset, count);
> +#endif
> dev->tc_to_txq[tc].count = count;
> dev->tc_to_txq[tc].offset = offset;
> return 0;
> @@ -2197,6 +2227,9 @@ int netdev_set_num_tc(struct net_device *dev, u8 num_tc)
> if (num_tc > TC_MAX_QUEUE)
> return -EINVAL;
>
> +#ifdef CONFIG_XPS
> + netif_reset_xps_queues_gt(dev, 0);
> +#endif
> dev->num_tc = num_tc;
> return 0;
> }
>
