Re: [PATCH 1/2] eal: fix failure race and behavior of thread create

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

On Thu, Mar 09, 2023 at 10:58:06AM +0100, Thomas Monjalon wrote:
> 09/03/2023 10:17, David Marchand:
> > On Tue, Mar 7, 2023 at 3:33 PM David Marchand <david.marchand@redhat.com> wrote:
> > > On Thu, Mar 2, 2023 at 7:44 PM Tyler Retzlaff
> > > <roretzla@linux.microsoft.com> wrote:
> > > >
> > > > In rte_thread_create setting affinity after pthread_create may fail.
> > > > Such a failure should result in the entire rte_thread_create failing
> > > > but doesn't.
> > > >
> > > > Additionally if there is a failure to set affinity a race exists where
> > > > the creating thread will free ctx and depending on scheduling of the new
> > > > thread it may also free ctx (double free).
> > > >
> > > > Resolve both of the above issues by using the pthread_setaffinity_np
> > > > prior to thread creation to set the affinity of the created thread. By
> > > > doing this no failure paths exist after pthread_create returns
> > > > successfully.
> > > >
> > > > Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
> > > > Cc: stable@dpdk.org
> > > > Cc: roretzla@linux.microsoft.com
> > > >
> > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > Reviewed-by: David Marchand <david.marchand@redhat.com>
> > 
> > Series applied, thanks.
> 
> Unfortunately we cannot merge this patch
> because it does not compile on Alpine Linux (musl libc):
> 
> lib/eal/unix/rte_thread.c:160:31: error:
> implicit declaration of function 'pthread_attr_setaffinity_np'

i didn't get any CI failure for this. did i just miss it?

> 
> Is it possible to fix the race without using pthread_attr_setaffinity_np?
> 

it seems we never allowed threads to be created with a set affinity when
using pthread_create directly (that was portable to alpine linux).  for worker
threads the start_routine is setting the affinity from the new thread.

certainly we can make this work by doing the same thing, but we'll have
to adjust the start routine wrapper to synchronize/wait for the new
thread to set the affinity and if it fails terminate the new thread
cleanly.

i don't have a way to build for alpine linux or run the unit tests, does
someone want to make the above suggested adjustment? or i can try and
make a patch but someone else will have to carefully review and test.

let me know how you'd like to proceed.

ty