Re: [PATCH v2 3/8] util: use RCU accessors for notifiers

[Stefan Hajnoczi]

On Mon, Apr 19, 2021 at 10:55:36AM +0200, Emanuele Giuseppe Esposito wrote:

What is the goal? Making the notifier APIs usable from multiple threads
(when callers respect RCU)?

> Note that calling rcu_read_lock() is left to the caller.  In fact,
> if the notifier is really only used within the BQL, it's unnecessary.
> 
> Even outside the BQL, RCU accessors can also be used with any API that has
> the same contract as synchronize_rcu, i.e. it stops until all concurrent
> readers complete, no matter how "readers" are defined.  In the next patch,
> for example, synchronize_rcu's role is taken by bdrv_drain (which is a
> superset of synchronize_rcu, since it also blocks new incoming readers).
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>  util/notify.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

No doc comments are added/updated so the RCU support is kind of hidden.
Please document it explicitly so that it's easy to write and review code
in the future without remembering implementation details of APIs. Using
"rcu" in function names would be most obvious and require no extra
documentation, but it probably makes more sense to have doc comments in
"qemu/notify.h" to avoid renaming everything.

> 
> diff --git a/util/notify.c b/util/notify.c
> index 76bab212ae..529f1d121e 100644
> --- a/util/notify.c
> +++ b/util/notify.c
> @@ -15,6 +15,7 @@

notifier_list_empty(NotifierList *list) should be updated to use
QLIST_EMPTY_RCU().

signature.asc
Description: PGP signature