Re: [PATCH 0/1] trace2: fix tracing when NO_PTHREADS is defined
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- May 23, 2019, 13:55 UTC
- Message-ID
- <50badedb-7856-374e-b14c-1eb1951c63f0@jeffhostetler.com>
- In-Reply-To
- <20190523055108.GA20871@sigill.intra.peff.net>
On 5/23/2019 1:51 AM, Jeff King wrote:
Show 88 quoted lines
> On Wed, May 22, 2019 at 09:23:39AM -0400, Jeff Hostetler wrote:
>
>> I was wondering about that too as the proper long term solution.
>> We would need (as the discussion suggests [1]) to properly
>> respect/represent the pthread_key_t argument.
>>
>> For now, I've guarded my usage of pthread_getspecific() in the trace2
>> (similar to what index-pack does), so its not urgent that we update it.
>> And I'd rather we take this simple trace2 fix now and not try to combine
>> it with fixes for the pthread macros. Especially now as we're in the RC
>> cycle for 2.22.
>
> Yeah, I think that makes sense.
>
>> I'll make a note to revisit the pthread code after 2.22.
>
> For fun, here's a constant-time zero-allocation implementation that I
> came up with. It passes t0211 with NO_PTHREADS, but I didn't test it
> beyond that.
>
> diff --git a/thread-utils.h b/thread-utils.h
> index 4961487ed9..f466215742 100644
> --- a/thread-utils.h
> +++ b/thread-utils.h
> @@ -18,7 +18,7 @@
> #define pthread_t int
> #define pthread_mutex_t int
> #define pthread_cond_t int
> -#define pthread_key_t int
> +#define pthread_key_t git_pthread_key_t
>
> #define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)
> #define pthread_mutex_lock(mutex)
> @@ -31,16 +31,49 @@
> #define pthread_cond_broadcast(cond)
> #define pthread_cond_destroy(cond)
>
> -#define pthread_key_create(key, attr) dummy_pthread_init(key)
> -#define pthread_key_delete(key)
> +#define pthread_key_create(key, destroy) git_pthread_key_create(key, destroy)
> +#define pthread_key_delete(key) git_pthread_key_delete(key)
>
> #define pthread_create(thread, attr, fn, data) \
> dummy_pthread_create(thread, attr, fn, data)
> #define pthread_join(thread, retval) \
> dummy_pthread_join(thread, retval)
>
> -#define pthread_setspecific(key, data)
> -#define pthread_getspecific(key) NULL
> +#define pthread_setspecific(key, data) git_pthread_setspecific(key, data)
> +#define pthread_getspecific(key) git_pthread_getspecific(key)
> +
> +typedef struct {
> + void *data;
> + /* extra indirection because setspecific is passed key by value */
> + void **vdata;
> +} git_pthread_key_t;
> +
> +static inline int git_pthread_key_create(git_pthread_key_t *key,
> + void (*destroy)(void *))
> +{
> + key->data = NULL;
> + key->vdata = &key->data;
> + /* We don't use this; alternatively we could all via atexit(). */
> + if (destroy)
> + BUG("NO_PTHREADS does not support pthread key destructors");
> + return 0;
> +}
> +
> +static inline int git_pthread_key_delete(git_pthread_key_t key)
> +{
> + /* noop */
> + return 0;
> +}
> +
> +static inline void git_pthread_setspecific(git_pthread_key_t key, void *data)
> +{
> + *(key.vdata) = data;
> +}
> +
> +static inline void *git_pthread_getspecific(git_pthread_key_t key)
> +{
> + return key.data;
> +}
>
> int dummy_pthread_create(pthread_t *pthread, const void *attr,
> void *(*fn)(void *), void *data);
> Thanks! I'll play with this and submit something after 2.22 and things slow down a bit.
Jeff