[ovs-dev,ovs-dev,v6,1/3] ovs-rcu: add rcu_barrier

rcu_barrier will block the current thread until all the postponed
rcu job has been finished. it's like the OVS's version of
the kernel rcu_barrier()

Signed-off-by: Peng He <hepeng.0320@bytedance.com>
Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/ovs-rcu.c | 38 ++++++++++++++++++++++++++++++++++++++
 lib/ovs-rcu.h | 15 +++++++++++++++
 2 files changed, 53 insertions(+)

Title should be: "ovs-rcu: Add ovsrcu_barrier."

On Wed, May 25, 2022 at 3:36 AM Peng He <xnhp0320@gmail.com> wrote:
>
> rcu_barrier will block the current thread until all the postponed

ovsrcu_barrier*

> rcu job has been finished. it's like the OVS's version of
> the kernel rcu_barrier()

It's like a OVS version of the Linux kernel rcu_barrier().


>
> Signed-off-by: Peng He <hepeng.0320@bytedance.com>
> Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  lib/ovs-rcu.c | 38 ++++++++++++++++++++++++++++++++++++++
>  lib/ovs-rcu.h | 15 +++++++++++++++
>  2 files changed, 53 insertions(+)
>
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index 1866bd308..86741f2b4 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -444,3 +444,41 @@ ovsrcu_init_module(void)
>          ovsthread_once_done(&once);
>      }
>  }
> +
> +static void
> +ovsrcu_barrier_func(void *seq_)
> +{
> +    struct seq *seq = (struct seq *) seq_;
> +    seq_change(seq);
> +}
> +
> +

One blank line is enough.


> +/* Similar to the kernel rcu_barrier, ovsrcu_barrier waits for all outstanding
> + * RCU callbacks to complete. However, unlike the kernel rcu_barrier, which
> + * might retrun immediately if there are no outstanding RCU callbacks,

return*

> + * this API will at least wait for a grace period.
> + *
> + * Another issue the caller might need to know it's that the barrier is just
> + * for "one-shot", i.e. if inside some RCU callbacks, another RCU callback is
> + * registered, this API only guarantees the first round of RCU callbacks have
> + * been executed after it returns.

to know is* that


> + */
> +void
> +ovsrcu_barrier(void)
> +{
> +    struct seq *seq = seq_create();
> +    /* First let all threads flush their cbsets. */
> +    ovsrcu_synchronize();
> +
> +    /* Then register a new cbset, ensure this cbset
> +     * is at the tail of the global list. */
> +    uint64_t seqno = seq_read(seq);
> +    ovsrcu_postpone__(ovsrcu_barrier_func, (void *) seq);
> +
> +    do {
> +        seq_wait(seq, seqno);
> +        poll_block();
> +    } while (seqno == seq_read(seq));
> +
> +    seq_destroy(seq);
> +}
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index ecc4c9201..32a49069e 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -155,6 +155,19 @@
>   *         port_delete(id);
>   *     }
>   *
> + * Use ovsrcu_barrier() to wait for all the outstanding RCU callbacks to
> + * finish. This is useful when you have to destroy some resources however
> + * these resources are referenced in the outstanding RCU callbacks.
> + *
> + * void rcu_cb(void *A) {
> + *     Use_A_do_something(A);

do_something() is enough.


> + * }
> + *
> + * void destroy_A() {
> + *     ovsrcu_postpone(rcu_cb, A); // will use A later
> + *     ovsrcu_barrier(); // wait for rcu_cb done
> + *     do_destroy_A(); // free A
> + * }

In these comments, code snippets are indented with an additional level.
IOW:

+ * Use ovsrcu_barrier() to wait for all the outstanding RCU callbacks to
+ * finish. This is useful when you have to destroy some resources however
+ * these resources are referenced in the outstanding RCU callbacks.
+ *
+ *     void rcu_cb(void *A) {
+ *        do_something(A);
+ *     }
+ *
+ *     void destroy_A() {
+ *         ovsrcu_postpone(rcu_cb, A); // will use A later
+ *         ovsrcu_barrier(); // wait for rcu_cb done
+ *         do_destroy_A(); // free A
+ *     }


>   */
>
>  #include "compiler.h"
> @@ -310,4 +323,6 @@ void ovsrcu_synchronize(void);
>
>  void ovsrcu_exit(void);
>
> +void ovsrcu_barrier(void);
> +
>  #endif /* ovs-rcu.h */
> --
> 2.25.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>


I think we should have a unit test for this new part of the ovs-rcu API.

David Marchand <david.marchand@redhat.com> 于2022年5月25日周三 19:27写道：

> Title should be: "ovs-rcu: Add ovsrcu_barrier."
>
> On Wed, May 25, 2022 at 3:36 AM Peng He <xnhp0320@gmail.com> wrote:
> >
> > rcu_barrier will block the current thread until all the postponed
>
> ovsrcu_barrier*
>
> > rcu job has been finished. it's like the OVS's version of
> > the kernel rcu_barrier()
>
> It's like a OVS version of the Linux kernel rcu_barrier().
>
>
> >
> > Signed-off-by: Peng He <hepeng.0320@bytedance.com>
> > Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
> > Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> > ---
> >  lib/ovs-rcu.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  lib/ovs-rcu.h | 15 +++++++++++++++
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> > index 1866bd308..86741f2b4 100644
> > --- a/lib/ovs-rcu.c
> > +++ b/lib/ovs-rcu.c
> > @@ -444,3 +444,41 @@ ovsrcu_init_module(void)
> >          ovsthread_once_done(&once);
> >      }
> >  }
> > +
> > +static void
> > +ovsrcu_barrier_func(void *seq_)
> > +{
> > +    struct seq *seq = (struct seq *) seq_;
> > +    seq_change(seq);
> > +}
> > +
> > +
>
> One blank line is enough.
>
>
> > +/* Similar to the kernel rcu_barrier, ovsrcu_barrier waits for all
> outstanding
> > + * RCU callbacks to complete. However, unlike the kernel rcu_barrier,
> which
> > + * might retrun immediately if there are no outstanding RCU callbacks,
>
> return*
>
> > + * this API will at least wait for a grace period.
> > + *
> > + * Another issue the caller might need to know it's that the barrier is
> just
> > + * for "one-shot", i.e. if inside some RCU callbacks, another RCU
> callback is
> > + * registered, this API only guarantees the first round of RCU
> callbacks have
> > + * been executed after it returns.
>
> to know is* that
>
>
> > + */
> > +void
> > +ovsrcu_barrier(void)
> > +{
> > +    struct seq *seq = seq_create();
> > +    /* First let all threads flush their cbsets. */
> > +    ovsrcu_synchronize();
> > +
> > +    /* Then register a new cbset, ensure this cbset
> > +     * is at the tail of the global list. */
> > +    uint64_t seqno = seq_read(seq);
> > +    ovsrcu_postpone__(ovsrcu_barrier_func, (void *) seq);
> > +
> > +    do {
> > +        seq_wait(seq, seqno);
> > +        poll_block();
> > +    } while (seqno == seq_read(seq));
> > +
> > +    seq_destroy(seq);
> > +}
> > diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> > index ecc4c9201..32a49069e 100644
> > --- a/lib/ovs-rcu.h
> > +++ b/lib/ovs-rcu.h
> > @@ -155,6 +155,19 @@
> >   *         port_delete(id);
> >   *     }
> >   *
> > + * Use ovsrcu_barrier() to wait for all the outstanding RCU callbacks to
> > + * finish. This is useful when you have to destroy some resources
> however
> > + * these resources are referenced in the outstanding RCU callbacks.
> > + *
> > + * void rcu_cb(void *A) {
> > + *     Use_A_do_something(A);
>
> do_something() is enough.
>
>
> > + * }
> > + *
> > + * void destroy_A() {
> > + *     ovsrcu_postpone(rcu_cb, A); // will use A later
> > + *     ovsrcu_barrier(); // wait for rcu_cb done
> > + *     do_destroy_A(); // free A
> > + * }
>
> In these comments, code snippets are indented with an additional level.
> IOW:
>
> + * Use ovsrcu_barrier() to wait for all the outstanding RCU callbacks to
> + * finish. This is useful when you have to destroy some resources however
> + * these resources are referenced in the outstanding RCU callbacks.
> + *
> + *     void rcu_cb(void *A) {
> + *        do_something(A);
> + *     }
> + *
> + *     void destroy_A() {
> + *         ovsrcu_postpone(rcu_cb, A); // will use A later
> + *         ovsrcu_barrier(); // wait for rcu_cb done
> + *         do_destroy_A(); // free A
> + *     }
>
>
> >   */
> >
> >  #include "compiler.h"
> > @@ -310,4 +323,6 @@ void ovsrcu_synchronize(void);
> >
> >  void ovsrcu_exit(void);
> >
> > +void ovsrcu_barrier(void);
> > +
> >  #endif /* ovs-rcu.h */
> > --
> > 2.25.1
> >
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
>
>
> I think we should have a unit test for this new part of the ovs-rcu API.
>

OK. I am going to add it in test-rcu.c.
Thanks for the review!

>
>
> --
> David Marchand
>
>