On 25/01/2021 20:23, Dumitru Ceara wrote:
On 1/15/21 4:29 PM, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov <anton.iva...@cambridgegreys.com>

This adds a set of functions and macros intended to process
hashes in parallel.

The principles of operation are documented in the fasthmap.h

If these one day go into the OVS tree, the OVS tree versions
would be used in preference.

Signed-off-by: Anton Ivanov <anton.iva...@cambridgegreys.com>
---

Hi Anton,

This is not a full review, just something I noticed while trying to run this patch set through CI on github (with parallelization enabled).

  lib/automake.mk |   2 +
  lib/fasthmap.c  | 327 ++++++++++++++++++++++++++++++++++++++++++++++++
  lib/fasthmap.h  | 212 +++++++++++++++++++++++++++++++
  3 files changed, 541 insertions(+)
  create mode 100644 lib/fasthmap.c
  create mode 100644 lib/fasthmap.h

diff --git a/lib/automake.mk b/lib/automake.mk

[...]

+
+static void setup_worker_pools(void) {
+    int cores, nodes;
+
+    nodes = ovs_numa_get_n_numas();
+    if (nodes == OVS_NUMA_UNSPEC || nodes <= 0) {
+        nodes = 1;
+    }
+    cores = ovs_numa_get_n_cores();
+
+    /* If there is no NUMA config, use 4 cores.
+     * If there is NUMA config use half the cores on
+     * one node so that the OS does not start pushing
+     * threads to other nodes.
+     */
+    if (cores == OVS_CORE_UNSPEC || cores <= 0) {
+        /* If there is no NUMA we can try the ovs-threads routine.
+         * It falls back to sysconf and/or affinity mask.
+         */
+        cores = count_cpu_cores();
+        pool_size = cores;
+    } else {
+        pool_size = cores / nodes;
+    }
+    if (pool_size > 16) {
+        pool_size = 16;
+    }
+    can_parallelize = (pool_size >= 3);

If pool_size < 3 then ovn_add_worker_pool() returns NULL and ovn-northd will crash.

Regards,
Dumitru


Thanks, I thought I had that working in one of the earlier revisions.

Looks like something was not moved while redoing the patchset.

I need to update it anyway to named semaphores at least for OSX (if not for everything), because OSX build fails without that.

I will release a fixed version tomorrow.

--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to