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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1286962799.3876.132.camel@edumazet-laptop>
Date:	Wed, 13 Oct 2010 11:39:59 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH v2] xps-mp: Transmit Packet Steering for multiqueue

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];



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html