Hi Mark

Thanks for fixing this. Nice catch...
Should the --dummy-numa option not also be added to northd documentation?
Does the --dummy-numa option have any other effect than setting the number
of threads? I mean does it have for instance any "numa" or pinning effect?
e.g. is there any difference between --dummy-numa=(0,0,0,0) and
dummy-numa=(0,1,0,1,0,1,0,1) ?
I do not think so, but I think it should be documented as --dummy-numa
might seem a complex option for specifying the number of threads.
Thanks
Xavier

On Thu, Jan 20, 2022 at 10:32 PM Mark Michelson <[email protected]> wrote:

> ovn-parallel-hmap attempted to determine the number of threads to
> allocate for its pool based on the NUMA and core count reported by OVS.
> The problem is that since ovn-northd never called ovs_numa_init(), OVS
> would always return OVS_NUMA_UNSPEC for the number of NUMAs, and
> OVS_CORE_UNSPEC for the number of cores. Parallelization still was
> operational because we would fall back to a CPU core count. On a single
> NUMA machine, this is identical to what was expected, so everything
> seemed fine.
>
> In 37ad427cfb78a9e59d7e7ef249b2f2b3ac68c65a, the --dumy-numa option was
> added to ovn-northd to allow for parallel operation on machines with
> fewer NUMAs and cores than the parallelization code otherwise would
> require. However, because of the missing call to ovs_numa_init(), this
> option did not function as intended.
>
> This commit adds a call to ovs_numa_init() when setting up the parallel
> hmap thread pool so that accurate NUMA and core counts are reported, and
> dummy NUMA and core counts can be used.
>
> The referenced bugzilla is not actually reporting this as an issue, but
> this particular fix will enable the functionality requested.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1975345
> Signed-off-by: Mark Michelson <[email protected]>
> ---
>  lib/ovn-parallel-hmap.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
> index 56ceed8e8..7edc4c0b6 100644
> --- a/lib/ovn-parallel-hmap.c
> +++ b/lib/ovn-parallel-hmap.c
> @@ -404,6 +404,7 @@ static void
>  setup_worker_pools(bool force) {
>      int cores, nodes;
>
> +    ovs_numa_init();
>      nodes = ovs_numa_get_n_numas();
>      if (nodes == OVS_NUMA_UNSPEC || nodes <= 0) {
>          nodes = 1;
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to