[ovs-dev] [urcu-fixes 3/4] ovs-thread: Make caller provide thread name when creating a thread.

Ben Pfaff blp at nicira.com
Mon Apr 28 22:24:59 UTC 2014


On Mon, Apr 28, 2014 at 11:48:34AM -0700, Alex Wang wrote:
> This is helpful,??? just two comments,
> 
> On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > This patch is a prerequisite for making RCU report the name of a thread
> > that is blocking RCU synchronization,
> > *because the easiest way to do that gets the name of the thread in
> > ovsrcu_quiesce_end(), *which is called before
> > the thread function is called (so it won't get a name set within the thread
> > function itself).
> >
> 
> Just curious, should we change the sentence in bold to "to do that is to
> get ..."

This paragraph was not well written.  I think that this might be a
little clearer:

    This patch is a prerequisite for making RCU report the name of a thread
    that is blocking RCU synchronization, because the easiest way to do that is
    for ovsrcu_quiesce_end() to record the current thread's name.
    ovsrcu_quiesce_end() is called before the thread function is called, so it
    won't get a name set within the thread function itself.  Setting the thread
    name earlier, as in this patch, avoids the problem.

> do we still need the ovsrcu_quiesce_end() here?

Yes, this patch doesn't change any properties of RCU yet.

> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -293,7 +293,6 @@ ovs_thread_create(const char *name, void *(*start)(void
> *), void *arg)
> 
>      forbid_forking("multiple threads exist");
>      multithreaded = true;
> -    ovsrcu_quiesce_end();
> 
>      aux = xmalloc(sizeof *aux);
>      aux->start = start;
> 
> Acked-by: Alex Wang <alexw at nicira.com>

Thanks for the review!



More information about the dev mailing list