On 1 May 2022, at 4:17, Peng He wrote:
> 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.0...@bytedance.com>
> Co-authored-by: Eelco Chaudron <echau...@redhat.com>
> ---
> lib/ovs-rcu.c | 28 ++++++++++++++++++++++++++++
> lib/ovs-rcu.h | 4 ++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index 1866bd308..258073d01 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -444,3 +444,31 @@ ovsrcu_init_module(void)
> ovsthread_once_done(&once);
> }
> }
> +
> +static void
> +ovsrcu_barrier_func(void *seq_)
> +{
> + struct seq *seq = (struct seq*)seq_;
Maybe add some spaces:
struct seq *seq = (struct seq *) seq_;
> + seq_change(seq);
> +}
> +
> +void
> +ovsrcu_barrier(void)
> +{
> + struct seq *seq = seq_create();
> + /* first let all threads flush their cbset */
Comments should start with a capital letter and end with a dot. So:
/* First let all threads flush their cbset’s. */
> + ovsrcu_synchronize();
> +
> + /* then register a new cbset, ensure this cbset
> + * is at the tail of global list
> + */
See above:
/* 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..2273af869 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -159,6 +159,7 @@
>
> #include "compiler.h"
> #include "ovs-atomic.h"
> +#include "seq.h"
Don’t think this is needed.
>
> #if __GNUC__
> #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
> @@ -310,4 +311,7 @@ void ovsrcu_synchronize(void);
>
> void ovsrcu_exit(void);
We should add some documentation on how this function should be used. Something
similar to the Linux kernel documentation.
> +void ovsrcu_barrier(void);
> +
> +
One empty line should be enough.
> #endif /* ovs-rcu.h */
> --
> 2.25.1
As suggested before maybe we should add a warning to the checkpatch.py utility.
Something like this:
WARNING: Are you sure you need to use ovsrcu_barrier(), in most cases
ovsrcu_synchronize() will be fine?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
