Re: [ovs-dev] [PATCH ovn v5] northd: add configuration option for enabling parallelization
I pushed this change to main. Thanks, Xavier. On 5/19/22 09:15, Mark Michelson wrote: Thanks for your persistence on this, Xavier. Acked-by: Mark Michelson On 5/19/22 07:20, Xavier Simonart wrote: This patch is intended to change the way to enable northd lflow build parallelization, as well as enable runtime change of number of threads. Before this patch, the following was needed to use parallelization: - enable parallelization through use_parallel_build in NBDB - use --dummy-numa to select number of threads. This second part was needed as otherwise as many threads as cores in the system were used, while parallelization showed some performance improvement only until using around 4 (or maybe 8) threads. With this patch, the number of threads used for lflow parallel build can be specified either: - at startup, using --n-threads= as ovn-northd command line option - using unixctl If the number of threads specified is > 1, then parallelization is enabled. If the number is 1, parallelization is disabled. If the number is < 1, parallelization is disabled at startup and a warning is logged. If the number is > 256, parallelization is enabled (with 256 threads) and a warning is logged. The following unixctl have been added: - set-n-threads : set the number of treads used. - get-n-threads: returns the number of threads used If the number of threads is within <2-256> bounds, parallelization is enabled. If the number of thread is 1, parallelization is disabled. Otherwise an error is thrown. Note that, if set-n-threads failed for any reason (e.g. failure to setup some semaphore), parallelization is disabled, and get-n-thread will return 1. A new command line argument (--ovn-northd-n-threads) has also been added to ovn-ctl. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2078552 Signed-off-by: Xavier Simonart --- v2: - handled Dumitru's comments - added missing mutex_destroy - fixed issue when use_logical_dp_group is enabled after northd startup - rebased on top of main v3: - fix mutex_destroy issue v4: - handled Mark's comments - rebased on top of main v5 - handled Dumitru's comment - added --ovn-northd-n-threads option in ovn-ctl --- NEWS | 8 ++ lib/ovn-parallel-hmap.c | 291 +- lib/ovn-parallel-hmap.h | 31 ++-- northd/northd.c | 176 --- northd/northd.h | 1 + northd/ovn-northd-ddlog.c | 6 - northd/ovn-northd.8.xml | 70 + northd/ovn-northd.c | 68 - tests/ovn-macros.at | 59 ++-- tests/ovn-northd.at | 109 ++ utilities/ovn-ctl | 5 + 11 files changed, 502 insertions(+), 322 deletions(-) diff --git a/NEWS b/NEWS index 244824e3f..578d281d7 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,14 @@ Post v22.03.0 implicit drop behavior on logical switches with ACLs applied. - Support (LSP.options:qos_min_rate) to guarantee minimal bandwidth available for a logical port. + - Changed the way to enable northd parallelization. + Removed support for: + - use_parallel_build in NBDB. + - --dummy-numa in northd cmdline. + Added support for: + - --n-threads= in northd cmdline. + - set-n-threads/get-n-threads unixctls. + - --ovn-northd-n-threads command line argument in ovn-ctl OVN v22.03.0 - 11 Mar 2022 -- diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c index 7edc4c0b6..828e5a0e2 100644 --- a/lib/ovn-parallel-hmap.c +++ b/lib/ovn-parallel-hmap.c @@ -38,14 +38,10 @@ VLOG_DEFINE_THIS_MODULE(ovn_parallel_hmap); #ifndef OVS_HAS_PARALLEL_HMAP -#define WORKER_SEM_NAME "%x-%p-%x" +#define WORKER_SEM_NAME "%x-%p-%"PRIxSIZE #define MAIN_SEM_NAME "%x-%p-main" -/* These are accessed under mutex inside add_worker_pool(). - * They do not need to be atomic. - */ static atomic_bool initial_pool_setup = ATOMIC_VAR_INIT(false); -static bool can_parallelize = false; /* This is set only in the process of exit and the set is * accompanied by a fence. It does not need to be atomic or be @@ -57,18 +53,18 @@ static struct ovs_list worker_pools = OVS_LIST_INITIALIZER(&worker_pools); static struct ovs_mutex init_mutex = OVS_MUTEX_INITIALIZER; -static int pool_size; +static size_t pool_size = 1; static int sembase; static void worker_pool_hook(void *aux OVS_UNUSED); -static void setup_worker_pools(bool force); +static void setup_worker_pools(void); static void merge_list_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size_t index); static void merge_hash_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size
Re: [ovs-dev] [PATCH ovn v5] northd: add configuration option for enabling parallelization
Thanks for your persistence on this, Xavier. Acked-by: Mark Michelson On 5/19/22 07:20, Xavier Simonart wrote: This patch is intended to change the way to enable northd lflow build parallelization, as well as enable runtime change of number of threads. Before this patch, the following was needed to use parallelization: - enable parallelization through use_parallel_build in NBDB - use --dummy-numa to select number of threads. This second part was needed as otherwise as many threads as cores in the system were used, while parallelization showed some performance improvement only until using around 4 (or maybe 8) threads. With this patch, the number of threads used for lflow parallel build can be specified either: - at startup, using --n-threads= as ovn-northd command line option - using unixctl If the number of threads specified is > 1, then parallelization is enabled. If the number is 1, parallelization is disabled. If the number is < 1, parallelization is disabled at startup and a warning is logged. If the number is > 256, parallelization is enabled (with 256 threads) and a warning is logged. The following unixctl have been added: - set-n-threads : set the number of treads used. - get-n-threads: returns the number of threads used If the number of threads is within <2-256> bounds, parallelization is enabled. If the number of thread is 1, parallelization is disabled. Otherwise an error is thrown. Note that, if set-n-threads failed for any reason (e.g. failure to setup some semaphore), parallelization is disabled, and get-n-thread will return 1. A new command line argument (--ovn-northd-n-threads) has also been added to ovn-ctl. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2078552 Signed-off-by: Xavier Simonart --- v2: - handled Dumitru's comments - added missing mutex_destroy - fixed issue when use_logical_dp_group is enabled after northd startup - rebased on top of main v3: - fix mutex_destroy issue v4: - handled Mark's comments - rebased on top of main v5 - handled Dumitru's comment - added --ovn-northd-n-threads option in ovn-ctl --- NEWS | 8 ++ lib/ovn-parallel-hmap.c | 291 +- lib/ovn-parallel-hmap.h | 31 ++-- northd/northd.c | 176 --- northd/northd.h | 1 + northd/ovn-northd-ddlog.c | 6 - northd/ovn-northd.8.xml | 70 + northd/ovn-northd.c | 68 - tests/ovn-macros.at | 59 ++-- tests/ovn-northd.at | 109 ++ utilities/ovn-ctl | 5 + 11 files changed, 502 insertions(+), 322 deletions(-) diff --git a/NEWS b/NEWS index 244824e3f..578d281d7 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,14 @@ Post v22.03.0 implicit drop behavior on logical switches with ACLs applied. - Support (LSP.options:qos_min_rate) to guarantee minimal bandwidth available for a logical port. + - Changed the way to enable northd parallelization. +Removed support for: +- use_parallel_build in NBDB. +- --dummy-numa in northd cmdline. +Added support for: +- --n-threads= in northd cmdline. +- set-n-threads/get-n-threads unixctls. +- --ovn-northd-n-threads command line argument in ovn-ctl OVN v22.03.0 - 11 Mar 2022 -- diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c index 7edc4c0b6..828e5a0e2 100644 --- a/lib/ovn-parallel-hmap.c +++ b/lib/ovn-parallel-hmap.c @@ -38,14 +38,10 @@ VLOG_DEFINE_THIS_MODULE(ovn_parallel_hmap); #ifndef OVS_HAS_PARALLEL_HMAP -#define WORKER_SEM_NAME "%x-%p-%x" +#define WORKER_SEM_NAME "%x-%p-%"PRIxSIZE #define MAIN_SEM_NAME "%x-%p-main" -/* These are accessed under mutex inside add_worker_pool(). - * They do not need to be atomic. - */ static atomic_bool initial_pool_setup = ATOMIC_VAR_INIT(false); -static bool can_parallelize = false; /* This is set only in the process of exit and the set is * accompanied by a fence. It does not need to be atomic or be @@ -57,18 +53,18 @@ static struct ovs_list worker_pools = OVS_LIST_INITIALIZER(&worker_pools); static struct ovs_mutex init_mutex = OVS_MUTEX_INITIALIZER; -static int pool_size; +static size_t pool_size = 1; static int sembase; static void worker_pool_hook(void *aux OVS_UNUSED); -static void setup_worker_pools(bool force); +static void setup_worker_pools(void); static void merge_list_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size_t index); static void merge_hash_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size_t index); bool ovn_stop_parallel_processing(void) @@ -76,107 +72,
Re: [ovs-dev] [PATCH ovn v5] northd: add configuration option for enabling parallelization
Bleep bloop. Greetings Xavier Simonart, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 northd: add configuration option for enabling parallelization When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v5] northd: add configuration option for enabling parallelization
This patch is intended to change the way to enable northd lflow build parallelization, as well as enable runtime change of number of threads. Before this patch, the following was needed to use parallelization: - enable parallelization through use_parallel_build in NBDB - use --dummy-numa to select number of threads. This second part was needed as otherwise as many threads as cores in the system were used, while parallelization showed some performance improvement only until using around 4 (or maybe 8) threads. With this patch, the number of threads used for lflow parallel build can be specified either: - at startup, using --n-threads= as ovn-northd command line option - using unixctl If the number of threads specified is > 1, then parallelization is enabled. If the number is 1, parallelization is disabled. If the number is < 1, parallelization is disabled at startup and a warning is logged. If the number is > 256, parallelization is enabled (with 256 threads) and a warning is logged. The following unixctl have been added: - set-n-threads : set the number of treads used. - get-n-threads: returns the number of threads used If the number of threads is within <2-256> bounds, parallelization is enabled. If the number of thread is 1, parallelization is disabled. Otherwise an error is thrown. Note that, if set-n-threads failed for any reason (e.g. failure to setup some semaphore), parallelization is disabled, and get-n-thread will return 1. A new command line argument (--ovn-northd-n-threads) has also been added to ovn-ctl. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2078552 Signed-off-by: Xavier Simonart --- v2: - handled Dumitru's comments - added missing mutex_destroy - fixed issue when use_logical_dp_group is enabled after northd startup - rebased on top of main v3: - fix mutex_destroy issue v4: - handled Mark's comments - rebased on top of main v5 - handled Dumitru's comment - added --ovn-northd-n-threads option in ovn-ctl --- NEWS | 8 ++ lib/ovn-parallel-hmap.c | 291 +- lib/ovn-parallel-hmap.h | 31 ++-- northd/northd.c | 176 --- northd/northd.h | 1 + northd/ovn-northd-ddlog.c | 6 - northd/ovn-northd.8.xml | 70 + northd/ovn-northd.c | 68 - tests/ovn-macros.at | 59 ++-- tests/ovn-northd.at | 109 ++ utilities/ovn-ctl | 5 + 11 files changed, 502 insertions(+), 322 deletions(-) diff --git a/NEWS b/NEWS index 244824e3f..578d281d7 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,14 @@ Post v22.03.0 implicit drop behavior on logical switches with ACLs applied. - Support (LSP.options:qos_min_rate) to guarantee minimal bandwidth available for a logical port. + - Changed the way to enable northd parallelization. +Removed support for: +- use_parallel_build in NBDB. +- --dummy-numa in northd cmdline. +Added support for: +- --n-threads= in northd cmdline. +- set-n-threads/get-n-threads unixctls. +- --ovn-northd-n-threads command line argument in ovn-ctl OVN v22.03.0 - 11 Mar 2022 -- diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c index 7edc4c0b6..828e5a0e2 100644 --- a/lib/ovn-parallel-hmap.c +++ b/lib/ovn-parallel-hmap.c @@ -38,14 +38,10 @@ VLOG_DEFINE_THIS_MODULE(ovn_parallel_hmap); #ifndef OVS_HAS_PARALLEL_HMAP -#define WORKER_SEM_NAME "%x-%p-%x" +#define WORKER_SEM_NAME "%x-%p-%"PRIxSIZE #define MAIN_SEM_NAME "%x-%p-main" -/* These are accessed under mutex inside add_worker_pool(). - * They do not need to be atomic. - */ static atomic_bool initial_pool_setup = ATOMIC_VAR_INIT(false); -static bool can_parallelize = false; /* This is set only in the process of exit and the set is * accompanied by a fence. It does not need to be atomic or be @@ -57,18 +53,18 @@ static struct ovs_list worker_pools = OVS_LIST_INITIALIZER(&worker_pools); static struct ovs_mutex init_mutex = OVS_MUTEX_INITIALIZER; -static int pool_size; +static size_t pool_size = 1; static int sembase; static void worker_pool_hook(void *aux OVS_UNUSED); -static void setup_worker_pools(bool force); +static void setup_worker_pools(void); static void merge_list_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size_t index); static void merge_hash_results(struct worker_pool *pool OVS_UNUSED, void *fin_result, void *result_frags, - int index); + size_t index); bool ovn_stop_parallel_processing(void) @@ -76,107 +72,184 @@ ovn_stop_parallel_processing(void) return workers_must_exit; } -bool -ovn_can_parallelize_hashes(bool force_parallel) +size_t +ovn_get_worker_pool_size(void)
