[ovs-dev] [PATCH V3 1/2] ofproto-dpif-upcall: Allow main thread to pause all revalidators.

[Joe Stringer]

On 29 August 2015 at 17:42, ALeX Wang <ee07b291 at gmail.com> wrote:
>
>
> On 29 August 2015 at 16:45, Joe Stringer <joestringer at nicira.com> wrote:
>>
>> On 29 August 2015 at 15:24, Alex Wang <alexw at nicira.com> wrote:
>> >
>> >
>> > On Sat, Aug 29, 2015 at 11:34 AM, Joe Stringer <joestringer at nicira.com>
>> > wrote:
>> >>
>> >> Thanks for working on this, Alex. I've considered implementing an
>> >> approach like this before, but haven't had a strong reason to.
>> >>
>> >> On 29 August 2015 at 00:42, Alex Wang <ee07b291 at gmail.com> wrote:
>> >> > This commit adds logic using ovs barrier to allow main thread pause
>> >> > all revalidators.  This new feature will be used in a later patch.
>> >> >
>> >> > Signed-off-by: Alex Wang <ee07b291 at gmail.com>
>> >>
>> >> <snip>
>> >>
>> >> > @@ -762,6 +791,11 @@ udpif_revalidator(void *arg)
>> >> >
>> >> >      revalidator->id = ovsthread_id_self();
>> >> >      for (;;) {
>> >> > +        /* Pauses all revalidators if wanted. */
>> >> > +        if (latch_is_set(&udpif->pause_latch)) {
>> >> > +            revalidator_pause(revalidator);
>> >> > +        }
>> >> > +
>> >>
>> >> Is there anything that ensures all revalidators are either before this
>> >> statement, or after this statement, when the latch is modified?
>> >
>> >
>> > I think this should not matter, since the latch_wait() will cause
>> > revalidator
>> > waking up immediately...  And latch has only two states (set or not),
>> > unlike
>> > the sequence number, so we do not need to worry about missing the
>> > seq_change,
>>
>> What if the threads don't wake up due to latch_wait(), but due to
>> another reason? Each revalidator thread individually checks the value
>> at a different time from all the others, so without a critical section
>> that ensures they all check the same value, I think it's possible for
>> two revalidators to wake up (eg due to timer expiring), one checks the
>> latch & skips the barrier, then main thread changes the latch, then
>> the second revalidator checks the latch & blocks on the barrier.
>
>
>
> In that case, I think the first revalidator should proceed to
> latch_wait(&udpif->pause_latch);
> and wakes up immediately since the latch is set.

In this case, one revalidator would wait on the pause_barrier, then
another would process through and wait on a reval_barrier. No-one can
continue, because the threads are waiting on different barriers.