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

[Joe Stringer]

Hi Alex, a minor comment, otherwise

On 29 August 2015 at 18:44, 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>

Acked-by: Joe Stringer <joestringer at nicira.com>

> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -110,7 +110,11 @@ struct udpif {
>      /* Revalidation. */
>      struct seq *reval_seq;             /* Incremented to force revalidation. */
>      bool reval_exit;                   /* Set by leader on 'exit_latch. */
> +    bool pause;                        /* Set by leader on 'pause_latch. */
>      struct ovs_barrier reval_barrier;  /* Barrier used by revalidators. */
> +    struct latch pause_latch;          /* Set to force revalidators pause. */
> +    struct ovs_barrier pause_barrier;  /* Barrier used to pause all */
> +                                       /* revalidators by main thread. */
>      struct dpif_flow_dump *dump;       /* DPIF flow dump state. */
>      long long int dump_duration;       /* Duration of the last flow dump. */
>      struct seq *dump_seq;              /* Increments each dump iteration. */

As a style thing, these could probably be grouped together just below
this with a slightly longer comment explaining something like:

/* These variables provide a mechanism for the main thread to pause
all revalidation without having to completely shut the threads down.
'pause_latch' is shared between the main thread and the lead
revalidator thread, so when it is desirable to halt revalidation, the
main thread will set the latch. 'pause' and 'pause_barrier' are shared
by revalidator threads. The lead revalidator will set 'pause' when it
observes the latch has been set, and this will cause all revalidator
threads to wait on 'pause_barrier' at the beginning of the next
revalidation round. */

If you think that makes sense. (Also furthers my task of trying to
better document the revalidation code ;-) )