On Aug 28, 2013 11:54 AM, "Jarno Rajahalme" <jrajaha...@nicira.com> wrote: > > > On Aug 28, 2013, at 9:39 AM, Jarno Rajahalme <jrajaha...@nicira.com> wrote: > >> 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. > > > > On a second thought, I think it is better to be conservative here and lock while cond_signaling. That way there will be no nasty surprises if some of the other code gets reorganized.
That way requires less careful thought so I support it in the absence of evidence that there is a performance benefit to avoiding the lock. Will you post a version that always takes the lock then? Thanks, Ben.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev