On Tue, Aug 27, 2013 at 03:50:06PM -0700, Jarno Rajahalme wrote:
>
> On Aug 27, 2013, at 3:13 PM, Ben Pfaff <b...@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 <jrajaha...@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:
One specific race that I was concerned about is:
1. This code in udpif_miss_handler() checks n_upcalls and sees that it
is zero.
if (!handler->n_upcalls) {
2. This code in recv_upcalls() signals wake_cond:
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);
}
}
3. This code in udpif_miss_handler() starts waiting on wake_cond:
ovs_mutex_cond_wait(&handler->wake_cond, &handler->mutex);
Maybe this race cannot happen, because n_upcalls only changes with the
mutex taken. I guess that's the case.
Ethan, unless you see something, I'm happy with this. (I'll be at
VMworld tomorrow and probably not continuing the conversation.)
Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
