Re: [PATCH v2] xps-mp: Transmit Packet Steering for multiqueue

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 12 octobre 2010 à 17:20 -0700, Tom Herbert a écrit :


+#define netdev_get_xps_maps(dev) ((dev)->_tx[0].xps_maps)

Why is xps_maps stored on all struct netdev_queue and not in net_device
itself ?

> +#define QUEUE_MASK_SIZE(dev) (BITS_TO_LONGS(dev->num_tx_queues))
> +#define XPS_MAP_SIZE(dev) (sizeof(struct xps_map) + (num_possible_cpus() * \
> +    QUEUE_MASK_SIZE(dev) * sizeof(unsigned long)))
> +#define XPS_ENTRY(map, offset, dev) \
> +    (&map->queues[offset * QUEUE_MASK_SIZE(dev)])
> +#define netdev_get_xps_maps(dev) ((dev)->_tx[0].xps_maps)
> +
> +

num_possible_cpus() is a bit too expensive on some machines
(NR_CPUS=4096), please find a way to cache its result in XPS structs.

+       queues = XPS_ENTRY(maps, cpu, dev);
+
+       if (queue_index >= 0) {
+               if (test_bit(queue_index, queues)) {
+                       rcu_read_unlock();
+                       return queue_index;
+               }
+       }
+
+       weight = bitmap_weight(queues, dev->real_num_tx_queues);


Same here : Cant you store bitmap_weight() somewhere ?

+               select = ((u64) hash * weight) >> 32;
+               queue_index =
+                   find_first_bit(queues, dev->real_num_tx_queues);
+               while (select--)
+                       queue_index = find_next_bit(queues,
+                           dev->real_num_tx_queues, queue_index + 1);
+               break;

Ouch, thats pretty expensive and not scalable (say device has 64 virtual
queues, on a 1024 cores machine), I think an array would be better

	queue_index = array[select];