This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: Fix blocking pthread_join.


On 04/30/2018 11:19 AM, Stefan Liebler wrote:
> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
> index 8326e2805c..bfbda99940 100644
> --- a/sysdeps/nptl/lowlevellock.h
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -181,11 +181,14 @@ extern int __lll_timedlock_wait (int *futex, const struct timespec *,
>     thread ID while the clone is running and is reset to zero by the kernel
>     afterwards.  The kernel up to version 3.16.3 does not use the private futex
>     operations for futex wake-up when the clone terminates.  */
> -#define lll_wait_tid(tid) \
> -  do {					\
> -    __typeof (tid) __tid;		\
> -    while ((__tid = (tid)) != 0)	\
> -      lll_futex_wait (&(tid), __tid, LLL_SHARED);\
> +#define lll_wait_tid(tid)				\
> +  do {							\
> +    __typeof (tid) __tid;				\
> +    /* We need acquire MO here so that we synchronize	\
> +       with the kernel's store to 0 when the clone	\
> +       terminates. (see above)  */			\
> +    while ((__tid = atomic_load_acquire (&(tid))) != 0)	\
> +      lll_futex_wait (&(tid), __tid, LLL_SHARED);	\
>    } while (0)
>  
>  extern int __lll_timedwait_tid (int *, const struct timespec *)

This looks good to me, and improves the situation.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

I haven't had a chance to review the other P&C issues discussed by Torvald,
but we should probably raise them in a new thread related to tid reloading
and the consequences.

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]