[ovs-dev] [PATCH 1/5] ovs-thread: Add a comment

YAMAMOTO Takashi yamamoto at valinux.co.jp
Fri Feb 7 01:24:45 UTC 2014


> On Wed, Feb 05, 2014 at 11:47:17AM +0900, YAMAMOTO Takashi wrote:
>> > On Wed, Jan 15, 2014 at 12:41:20PM +0900, YAMAMOTO Takashi wrote:
>> >> Add a comment about implicit synchronization which
>> >> fat-rwlock seems to rely on.
>> >> 
>> >> Signed-off-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
>> >> ---
>> >>  lib/ovs-thread.h | 8 ++++++++
>> >>  1 file changed, 8 insertions(+)
>> >> 
>> >> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
>> >> index 8cf2ecc..dab7510 100644
>> >> --- a/lib/ovs-thread.h
>> >> +++ b/lib/ovs-thread.h
>> >> @@ -447,6 +447,14 @@ void xpthread_join(pthread_t, void **);
>> >>   * Compared to pthread_key_t, ovsthread_key_t has the follow limitations:
>> >>   *
>> >>   *    - Destructors must not access thread-specific data (via ovsthread_key).
>> >> + *
>> >> + * Unlike pthread_key_t, this API implicitly serializes key removal
>> >> + * (ovsthread_key_delete) and thread exit. (destructor)
>> >> + * I.e.
>> >> + *    - ovsthread_key_delete ensures the completion of destructors for
>> >> + *      the key.
>> >> + *    - Once ovsthread_key_delete completed, no destructors for the key
>> >> + *      will be executed on thread exit.
>> >>   */
>> >>  typedef struct ovsthread_key *ovsthread_key_t;
>> > 
>> > I think I understand what you are saying, but I think that I would
>> > phrase it differently.  Does the following express what you mean?
>> > 
>> > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
>> > index a20b2fd..18e524e 100644
>> > --- a/lib/ovs-thread.c
>> > +++ b/lib/ovs-thread.c
>> > @@ -592,7 +592,8 @@ ovsthread_key_create(ovsthread_key_t *keyp, void (*destructor)(void *))
>> >  }
>> >  
>> >  /* Frees 'key'.  The destructor supplied to ovsthread_key_create(), if any, is
>> > - * not called.
>> > + * not called.  This function returns only after any running destructors for
>> > + * 'key' complete execution.
>> 
>> 'running destructors' is not exact.
>> i believe pthread api allows for concurrent exiting threads to
>> start executing the destructor after pthread_key_delete completed.
>> but this ovsthread one doesn't.  and fat-rwlock relies on it.
>> 
>> i think it's better to say the difference explicitly as
>> it guarantees more than pthread while using similar-looking names.
>> i was confused when reading fat-rwlock.
> 
> Do you like the following version?

looks good to me.  thanks.

YAMAMOTO Takashi

> 
> --8<--------------------------cut here-------------------------->8--
> 
> From: Ben Pfaff <blp at nicira.com>
> Date: Thu, 6 Feb 2014 17:06:00 -0800
> Subject: [PATCH] ovs-thread: Add a comment.
> 
> Reported-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/ovs-thread.h |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> index 8cf2ecc..f031894 100644
> --- a/lib/ovs-thread.h
> +++ b/lib/ovs-thread.h
> @@ -444,9 +444,15 @@ void xpthread_join(pthread_t, void **);
>   * (by recompiling).  Thus, one may more freely use this form of
>   * thread-specific data.
>   *
> - * Compared to pthread_key_t, ovsthread_key_t has the follow limitations:
> + * ovsthread_key_t also differs from pthread_key_t in the following ways:
>   *
>   *    - Destructors must not access thread-specific data (via ovsthread_key).
> + *
> + *    - The pthread_key_t API allows concurrently exiting threads to start
> + *      executing the destructor after pthread_key_delete() returns.  The
> + *      ovsthread_key_t API guarantees that, when ovsthread_key_delete()
> + *      returns, all destructors have returned and no new ones will start
> + *      execution.
>   */
>  typedef struct ovsthread_key *ovsthread_key_t;
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list