| Message ID | 20210419085541.22310-4-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Block layer thread-safety, continued | expand |
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().
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 @@ #include "qemu/osdep.h" #include "qemu/notify.h" +#include "qemu/rcu_queue.h" void notifier_list_init(NotifierList *list) { @@ -23,19 +24,19 @@ void notifier_list_init(NotifierList *list) void notifier_list_add(NotifierList *list, Notifier *notifier) { - QLIST_INSERT_HEAD(&list->notifiers, notifier, node); + QLIST_INSERT_HEAD_RCU(&list->notifiers, notifier, node); } void notifier_remove(Notifier *notifier) { - QLIST_REMOVE(notifier, node); + QLIST_REMOVE_RCU(notifier, node); } void notifier_list_notify(NotifierList *list, void *data) { Notifier *notifier, *next; - QLIST_FOREACH_SAFE(notifier, &list->notifiers, node, next) { + QLIST_FOREACH_SAFE_RCU(notifier, &list->notifiers, node, next) { notifier->notify(notifier, data); } } @@ -53,12 +54,12 @@ void notifier_with_return_list_init(NotifierWithReturnList *list) void notifier_with_return_list_add(NotifierWithReturnList *list, NotifierWithReturn *notifier) { - QLIST_INSERT_HEAD(&list->notifiers, notifier, node); + QLIST_INSERT_HEAD_RCU(&list->notifiers, notifier, node); } void notifier_with_return_remove(NotifierWithReturn *notifier) { - QLIST_REMOVE(notifier, node); + QLIST_REMOVE_RCU(notifier, node); } int notifier_with_return_list_notify(NotifierWithReturnList *list, void *data) @@ -66,7 +67,7 @@ int notifier_with_return_list_notify(NotifierWithReturnList *list, void *data) NotifierWithReturn *notifier, *next; int ret = 0; - QLIST_FOREACH_SAFE(notifier, &list->notifiers, node, next) { + QLIST_FOREACH_SAFE_RCU(notifier, &list->notifiers, node, next) { ret = notifier->notify(notifier, data); if (ret != 0) { break;