Re: [RFC PATCH glibc 3/3] XXX: Reimplement gscope

[Samuel Thibault]

Sergey Bugaev, le dim. 09 mai 2021 16:54:04 +0300, a ecrit:
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 4604d00b..af16162f 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1324,10 +1324,8 @@ link_map_audit_state (struct link_map *l, size_t index)
>  }
>  #endif /* SHARED */
>  
> -#if THREAD_GSCOPE_IN_TCB
>  void __thread_gscope_wait (void) attribute_hidden;
>  # define THREAD_GSCOPE_WAIT() __thread_gscope_wait ()
> -#endif

Mmm, your patch 1/3 should keep THREAD_GSCOPE_IN_TCB then and only add
THREAD_GSCOPE_LINK_MAP. This 3/3 is the real patch that removes the
THREAD_GSCOPE_IN_TCB flag, as in replacing the only implementation that
was still using it.

> + again:
> +      if (atomic_load_acquire (flag_ptr) == THREAD_GSCOPE_FLAG_UNUSED)
> +        continue;
> +
> +      if (atomic_compare_and_exchange_bool_acq (flag_ptr,
> +                                                THREAD_GSCOPE_FLAG_WAIT,
> +                                                THREAD_GSCOPE_FLAG_USED))
> +        goto again;
> +
> +      lll_wait (flag_ptr, THREAD_GSCOPE_FLAG_WAIT, 0);

This is not like the nptl implementation. Better follow the nptl
implementation so both can be maintained likewise (possibly even merged
at some point).

> diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
> index 057b2613..0ca4e641 100644
> --- a/sysdeps/mach/hurd/i386/tls.h
> +++ b/sysdeps/mach/hurd/i386/tls.h
> @@ -369,6 +369,28 @@ _hurd_tls_new (thread_t child, struct i386_thread_state 
> *state, tcbhead_t *tcb)
>    return err;
>  }
>  
> +/* Global scope switch support.  */
> +#define THREAD_GSCOPE_LINK_MAP      0
> +
> +# define THREAD_GSCOPE_FLAG_UNUSED 0
> +# define THREAD_GSCOPE_FLAG_USED   1
> +# define THREAD_GSCOPE_FLAG_WAIT   2
> +
> +#define THREAD_GSCOPE_SET_FLAG() \
> +  THREAD_SETMEM (THREAD_SELF, gscope_flag, THREAD_GSCOPE_FLAG_USED)
> +
> +#define THREAD_GSCOPE_RESET_FLAG() \
> +  ({                                                         \
> +    int __flag;                                                      \
> +    asm volatile ("xchgl %0, %%gs:%P1"                               \
> +                  : "=r" (__flag)                            \
> +                  : "i" (offsetof (tcbhead_t, gscope_flag)), \
> +                    "0" (THREAD_GSCOPE_FLAG_UNUSED));                \
> +    if (__flag == THREAD_GSCOPE_FLAG_WAIT)                   \
> +      lll_wake (THREAD_SELF->gscope_flag, GSYNC_BROADCAST);  \

Is GSYNC_BROADCAST really needed? AIUI there is only one thread that is
waiting for the wake? Also, use LLL_PRIVATE.

Samuel