On 3/1/25 2:58 PM, Frank Wagner wrote:
> This patch fixes the parallel hmap to compile on Windows.
>
> Signed-off-by: Frank Wagner <[email protected]>
>
> ---
Hi Frank,
Thanks for the patch!
> lib/ovn-parallel-hmap.c | 15 ++++++++++++---
> lib/ovn-parallel-hmap.h | 1 +
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
> index 1b167e214..84d8e3f06 100644
> --- a/lib/ovn-parallel-hmap.c
> +++ b/lib/ovn-parallel-hmap.c
> @@ -93,9 +93,9 @@ stop_controls(struct worker_pool *pool)
>
> /* Wait for completion. */
> for (size_t i = 0; i < pool->size ; i++) {
> - if (pool->controls[i].worker) {
> + if (pool->controls[i].hasWorker) {
> pthread_join(pool->controls[i].worker, NULL);
> - pool->controls[i].worker = 0;
> + pool->controls[i].hasWorker = false;
> }
> }
> workers_must_exit = false;
> @@ -154,11 +154,15 @@ init_controls(struct worker_pool *pool)
> new_control->done = pool->done;
> new_control->data = NULL;
> new_control->pool = pool;
> - new_control->worker = 0;
> + new_control->hasWorker = false;
> ovs_mutex_init(&new_control->mutex);
> atomic_init(&new_control->finished, false);
> sprintf(sem_name, WORKER_SEM_NAME, sembase, pool, i);
> +#ifndef _WIN32
> new_control->fire = sem_open(sem_name, O_CREAT, S_IRWXU, 0);
> +#else
> + new_control->fire = sem_open(sem_name, O_CREAT, 0);
> +#endif
> if (new_control->fire == SEM_FAILED) {
> free_controls(pool);
> return -1;
> @@ -173,6 +177,7 @@ init_threads(struct worker_pool *pool, void
> *(*start)(void *))
> for (size_t i = 0; i < pool_size; i++) {
> pool->controls[i].worker =
> ovs_thread_create("worker pool helper", start,
> &pool->controls[i]);
> + pool->controls[i].hasWorker = true;
> }
> ovs_list_push_back(&worker_pools, &pool->list_node);
> }
> @@ -207,7 +212,11 @@ ovn_update_worker_pool(size_t requested_pool_size,
> (*pool)->size = pool_size;
> (*pool)->controls = NULL;
> sprintf(sem_name, MAIN_SEM_NAME, sembase, *pool);
> +#ifndef _WIN32
> (*pool)->done = sem_open(sem_name, O_CREAT, S_IRWXU, 0);
> +#else
> + (*pool)->done = sem_open(sem_name, O_CREAT, 0);
> +#endif
Should we add a platform specific semaphore wrapper in OVS instead (cc
Alin/Ilya)?
If that's overkill, let's just add a single ovn_sem_open()
function/macro in OVN in, e.g., ovn-util.[ch] so we don't sprinkle
ifndefs in different places.
> if ((*pool)->done == SEM_FAILED) {
> goto cleanup;
> }
> diff --git a/lib/ovn-parallel-hmap.h b/lib/ovn-parallel-hmap.h
> index 362b6fd9c..66c54ceba 100644
> --- a/lib/ovn-parallel-hmap.h
> +++ b/lib/ovn-parallel-hmap.h
> @@ -83,6 +83,7 @@ struct worker_control {
> void *data; /* Pointer to data to be processed. */
> pthread_t worker;
> struct worker_pool *pool;
> + bool hasWorker;
This is unfortunate, I guess it's because pthread_t size and
implementation is implementation dependent.
If we can't avoid it though we should probably move it to 'struct
worker_pool' instead. It's not a per worker state. Either all worker
threads are running or none are running.
> };
>
> struct worker_pool {
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
