Which race? The one that I described as maybe not possible (do you see
how?) Or some other race?
On Aug 27, 2013 11:48 PM, "Ethan Jackson" <et...@nicira.com> wrote:
> I'm happy with it as long as we fix the race.
>
> Ethan
>
> On Tue, Aug 27, 2013 at 11:02 PM, Ben Pfaff <b...@nicira.com> wrote:
> > 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
