*Ping*
2014-09-19 15:41 GMT+04:00 Varvara Rainchik <varvara.s.rainc...@gmail.com>:
> I've corrected my patch accordingly to what you said. To diffirentiate
> second case in destructor I've added pthread_setspecific
> (gomp_tls_key, NULL) at the end of gomp_thread_start. So, destructor
> can simply skip the case when pthread_getspecific (gomp_tls_key)
> returns 0. I also think that it's better to set 0 in gomp_thread_start
> explicitly as thread data is initialized by a local variable in this
> function.
>
> But, I see that pthread_getspecific always returns 0 in destrucor
> because data pointer is implicitly set to 0 before destructor call in
> glibc:
>
> (pthread_create.c):
>
> /* Always clear the data. */
> level2[inner].data = NULL;
>
> /* Make sure the data corresponds to a valid
> key. This test fails if the key was
> deallocated and also if it was
> re-allocated. It is the user's
> responsibility to free the memory in this
> case. */
> if (level2[inner].seq
> == __pthread_keys[idx].seq
> /* It is not necessary to register a destructor
> function. */
> && __pthread_keys[idx].destr != NULL)
> /* Call the user-provided destructor. */
> __pthread_keys[idx].destr (data);
>
> I suppose it's not necessary if everything is cleaned up in
> gomp_thread_start and destructor. What do you think?
>
>
> Changes are bootstrapped and regtested on x86_64-linux.
>
> 2014-09-19 Varvara Rainchik <varvara.rainc...@intel.com>
>
> * libgomp.h (gomp_thread): For non TLS case create thread data.
> * team.c (non_tls_thread_data_destructor,
> create_non_tls_thread_data): New functions.
>
>
> ---
> diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
> index bcd5b34..2f33d99 100644
> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -467,9 +467,15 @@ static inline struct gomp_thread *gomp_thread (void)
> }
> #else
> extern pthread_key_t gomp_tls_key;
> -static inline struct gomp_thread *gomp_thread (void)
> +extern struct gomp_thread *create_non_tls_thread_data (void);
> +static struct gomp_thread *gomp_thread (void)
> {
> - return pthread_getspecific (gomp_tls_key);
> + struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
> + if (thr == NULL)
> + {
> + thr = create_non_tls_thread_data ();
> + }
> + return thr;
> }
> #endif
>
> diff --git a/libgomp/team.c b/libgomp/team.c
> index e6a6d8f..a692df8 100644
> --- a/libgomp/team.c
> +++ b/libgomp/team.c
> @@ -41,6 +41,7 @@ pthread_key_t gomp_thread_destructor;
> __thread struct gomp_thread gomp_tls_data;
> #else
> pthread_key_t gomp_tls_key;
> +struct gomp_thread initial_thread_tls_data;
> #endif
>
>
> @@ -130,6 +131,7 @@ gomp_thread_start (void *xdata)
> gomp_sem_destroy (&thr->release);
> thr->thread_pool = NULL;
> thr->task = NULL;
> + pthread_setspecific (gomp_tls_key, NULL);
> return NULL;
> }
>
> @@ -222,8 +224,16 @@ gomp_free_pool_helper (void *thread_pool)
> void
> gomp_free_thread (void *arg __attribute__((unused)))
> {
> - struct gomp_thread *thr = gomp_thread ();
> - struct gomp_thread_pool *pool = thr->thread_pool;
> + struct gomp_thread *thr;
> + struct gomp_thread_pool *pool;
> +#ifdef HAVE_TLS
> + thr = gomp_thread ();
> +#else
> + thr = pthread_getspecific (gomp_tls_key);
> + if (thr == NULL)
> + return;
> +#endif
> + pool = thr->thread_pool;
> if (pool)
> {
> if (pool->threads_used > 0)
> @@ -910,6 +920,21 @@ gomp_team_end (void)
> }
> }
>
> +/* Destructor for data created in create_non_tls_thread_data. */
> +
> +#ifndef HAVE_TLS
> +void
> +non_tls_thread_data_destructor (void *arg __attribute__((unused)))
> +{
> + struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
> + if (thr != NULL && thr != &initial_thread_tls_data)
> + {
> + gomp_free_thread (arg);
> + free (thr);
> + pthread_setspecific (gomp_tls_key, NULL);
> + }
> +}
> +#endif
>
> /* Constructors for this file. */
>
> @@ -917,9 +942,7 @@ static void __attribute__((constructor))
> initialize_team (void)
> {
> #ifndef HAVE_TLS
> - static struct gomp_thread initial_thread_tls_data;
> -
> - pthread_key_create (&gomp_tls_key, NULL);
> + pthread_key_create (&gomp_tls_key, non_tls_thread_data_destructor);
> pthread_setspecific (gomp_tls_key, &initial_thread_tls_data);
> #endif
>
> @@ -927,6 +950,19 @@ initialize_team (void)
> gomp_fatal ("could not create thread pool destructor.");
> }
>
> +/* Create data for thread created by pthread_create. */
> +
> +#ifndef HAVE_TLS
> +struct gomp_thread *create_non_tls_thread_data (void)
> +{
> + struct gomp_thread *thr = gomp_malloc_cleared (sizeof (struct
> gomp_thread));
> + pthread_setspecific (gomp_tls_key, thr);
> + gomp_sem_init (&thr->release, 0);
> +
> + return thr;
> +}
> +#endif
> +
> static void __attribute__((destructor))
> team_destructor (void)
> {
>
> 2014-09-02 14:36 GMT+04:00 Varvara Rainchik <varvara.s.rainc...@gmail.com>:
>> May I use gomp_free_thread as a destructor for pthread_key_create?
>> Then I'll make initial_thread_tls_data global for the first case, but
>> how can I differentiate thread created by gomp_thread_start (second
>> case)?
>>
>> 2014-09-01 14:51 GMT+04:00 Jakub Jelinek <ja...@redhat.com>:
>>> On Fri, Aug 29, 2014 at 10:40:57AM -0700, Richard Henderson wrote:
>>>> On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
>>>> > * libgomp.h (gomp_thread): For non TLS case create thread data.
>>>> > * team.c (create_non_tls_thread_data): New function.
>>>> >
>>>> >
>>>> > ---
>>>> > diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
>>>> > index a1482cc..cf3ec8f 100644
>>>> > --- a/libgomp/libgomp.h
>>>> > +++ b/libgomp/libgomp.h
>>>> > @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
>>>> > }
>>>> > #else
>>>> > extern pthread_key_t gomp_tls_key;
>>>> > +extern struct gomp_thread *create_non_tls_thread_data (void);
>>>> > static inline struct gomp_thread *gomp_thread (void)
>>>> > {
>>>> > - return pthread_getspecific (gomp_tls_key);
>>>> > + struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
>>>> > + if (thr == NULL)
>>>> > + {
>>>> > + thr = create_non_tls_thread_data ();
>>>> > + }
>>>> > + return thr;
>>>> > }
>>>>
>>>> This should never happen.
>>>
>>> I guess it can happen if you mix up explicit pthread_create and libgomp
>>> APIs.
>>> initialize_team will only initialize it in the initial thread, while if you
>>> use #pragma omp ... or omp_* calls from a thread created with
>>> pthread_create, in the !HAVE_TLS case pthread_getspecific will return NULL.
>>>
>>> Now, the patch doesn't handle that case completely though (and is badly
>>> formatted), the problem is that if we allocate in the !HAVE_TLS case
>>> in non-initial thread the TLS data, we want to free them again, so that
>>> would mean pthread_key_create with non-NULL destructor, and then we need to
>>> differentiate in between the 3 cases - key equal to &initial_thread_tls_data
>>> (would need to move out of the block context), no freeing needed, thread
>>> created by gomp_thread_start, no freeing needed, otherwise free.
>>>
>>>> The thread-specific data is set in gomp_thread_start and initialize_team.
>>>>
>>>> Where are you getting a call to gomp_thread that hasn't been through one of
>>>> those functions?
>>>
>>> Jakub
