This provides more flexibility for its users.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/notify.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/util/notify.c b/util/notify.c
index 06de63a839..16fff8d07d 100644
--- a/util/notify.c
+++ b/util/notify.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/notify.h"
+#include "qemu/rcu_queue.h"
 
 void notifier_list_init(NotifierList *list)
 {
@@ -24,19 +25,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);
     }
 }
@@ -49,12 +50,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)
@@ -62,7 +63,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;
-- 
2.13.0

On Thu, Jul 06, 2017 at 06:38:20PM +0200, Paolo Bonzini wrote:
>  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);
>      }
>  }

Who calls rcu_read_lock() or is it unnecessary?

On 10/07/2017 17:52, Stefan Hajnoczi wrote:
> On Thu, Jul 06, 2017 at 06:38:20PM +0200, Paolo Bonzini wrote:
>>  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);
>>      }
>>  }
> 
> Who calls rcu_read_lock() or is it unnecessary?

It depends.

If the notifier is really only used within the BQL, it's unnecessary.

If the notifier's readers want to protect the notifier with RCU, it's up
to the callers indeed.

However, 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).

For a similar example in Linux, see drivers/vhost/net.c.  It replaces
rcu_read_lock/unlock with "always run readers for a workqueue", and
synchronize_rcu with vhost_poll_flush (which calls vhost_work_flush).

Paolo

On Mon, Jul 10, 2017 at 06:06:32PM +0200, Paolo Bonzini wrote:
> On 10/07/2017 17:52, Stefan Hajnoczi wrote:
> > On Thu, Jul 06, 2017 at 06:38:20PM +0200, Paolo Bonzini wrote:
> >>  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);
> >>      }
> >>  }
> > 
> > Who calls rcu_read_lock() or is it unnecessary?
> 
> It depends.
> 
> If the notifier is really only used within the BQL, it's unnecessary.
> 
> If the notifier's readers want to protect the notifier with RCU, it's up
> to the callers indeed.
> 
> However, 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).
> 
> For a similar example in Linux, see drivers/vhost/net.c.  It replaces
> rcu_read_lock/unlock with "always run readers for a workqueue", and
> synchronize_rcu with vhost_poll_flush (which calls vhost_work_flush).

Thanks for the explanation.  I see how that is safe.