[ovs-dev] [PATCH] ofproto-dpif-upcall: Batch upcalls.

Ethan Jackson ethan at nicira.com
Wed Aug 28 00:01:29 UTC 2013


My thinking was that we need to take the lock when we do the
xpthread_cond_signal() because if we don't it's possible the thread
could go to sleep immediately after we make the signal call and
essentially never wake up (or at least not until it's called again).


On Tue, Aug 27, 2013 at 3:50 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>
> On Aug 27, 2013, at 3:13 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> On Tue, Aug 27, 2013 at 03:04:54PM -0700, Jarno Rajahalme wrote:
>
> Batching reduces overheads and enables upto 4 times the upcall processing
> performance in a specialized test case.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>
>
> Nice!
>
> +    for (n = 0; n < udpif->n_handlers; ++n) {
> +        handler = &udpif->handlers[n];
> +        if (handler->n_new_upcalls) {
> +            handler->n_new_upcalls = 0;
> +            xpthread_cond_signal(&handler->wake_cond);
> +        }
>
>
> Usually there's a race if one signals a condition variable without
> holding the corresponding mutex.  Did you check that there is no race
> here?
>
>
> I did not notice anything hanging or the like, if that's what you mean.
> Documentation says:
>
> "The pthread_cond_signal() or pthread_cond_broadcast() functions may be
> called by a thread whether or not it currently owns the mutex that threads
> calling pthread_cond_wait() or pthread_cond_timedwait() have associated with
> the condition variable during their waits; however, if predictable
> scheduling behaviour is required, then that mutex is locked by the thread
> calling pthread_cond_signal() or pthread_cond_broadcast()."
>
> I get that to mean that by locking you can control when the other thread
> starts running (i.e., when you release the lock), rather than immediately,
> possibly scheduling out the calling thread. In fact, having the scheduling
> behavior be less predictable should help find bugs sooner, which might be
> nice.
>
>   Jarno
>
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list