Re: [PATCH] eal: factorize lcore main loop

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Wed, Mar 23, 2022 at 10:30:01AM +0100, David Marchand wrote:
> All OS implementations provide the same main loop.
> Introduce helpers (shared for Linux and FreeBSD) to handle synchronisation
> between main and threads and factorize the rest as common code.
> Thread id are now logged as string in a common format across OS.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> I had this patch in store for a long time.
> I don't particularly care about it, it's not fixing anything.
> But it seems a good cleanup/consolidation, so I rebased it and I am
> sending it to get feedback.
> 

... snip ...

> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index 684bea166c..256de91abc 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -9,6 +9,7 @@
>  #include <assert.h>
>  #include <string.h>
>  
> +#include <rte_eal_trace.h>
>  #include <rte_errno.h>
>  #include <rte_lcore.h>
>  #include <rte_log.h>
> @@ -163,6 +164,77 @@ __rte_thread_uninit(void)
>  	RTE_PER_LCORE(_lcore_id) = LCORE_ID_ANY;
>  }
>  
> +/* main loop of threads */
> +__rte_noreturn void *
> +eal_thread_loop(__rte_unused void *arg)
> +{
> +	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
> +	pthread_t thread_id = pthread_self();
> +	unsigned int lcore_id;
> +	int ret;
> +
> +	/* retrieve our lcore_id from the configuration structure */
> +	RTE_LCORE_FOREACH_WORKER(lcore_id) {
> +		if (thread_id == lcore_config[lcore_id].thread_id)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

i can see that in practice this isn't a problem since the linux
implementation of pthread_create(3) stores to pthread_t *thread before
executing start_routine.

but strictly speaking i don't think the pthread_create api contractually
guarantees that the thread id is stored before start_routine runs. so this
is relying on an internal implementation detail.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_create.html

  "Upon successful completion, pthread_create() shall store the ID of the
   created thread in the location referenced by thread."

https://man7.org/linux/man-pages/man3/pthread_create.3.html

  "Before returning, a successful call to pthread_create() stores
   the ID of the new thread in the buffer pointed to by thread; this
   identifier is used to refer to the thread in subsequent calls to
   other pthreads functions."

it doesn't really say when it does this in relation to start_routine running.
depends how hair splitty you want to be about it. but since you're revamping
the code you might be interested in addressing it.

ty