Re: [PATCH v2 2/7] Implement drain_call_rcu and use it in hmp_device_del

[Paolo Bonzini]

On 09/07/20 13:56, Maxim Levitsky wrote:
> On Thu, 2020-07-09 at 13:42 +0200, Markus Armbruster wrote:
>> Maxim Levitsky <mlevitsk@redhat.com> writes:
>>
>>> This allows to preserve the semantics of hmp_device_del,
>>> that the device is deleted immediatly which was changed by previos
>>> patch that delayed this to RCU callback
>>>
>>> Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
>>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>>> ---
>>>  include/qemu/rcu.h |  1 +
>>>  qdev-monitor.c     |  3 +++
>>>  util/rcu.c         | 33 +++++++++++++++++++++++++++++++++
>>>  3 files changed, 37 insertions(+)
>>>
>>> diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
>>> index 570aa603eb..0e375ebe13 100644
>>> --- a/include/qemu/rcu.h
>>> +++ b/include/qemu/rcu.h
>>> @@ -133,6 +133,7 @@ struct rcu_head {
>>>  };
>>>  
>>>  extern void call_rcu1(struct rcu_head *head, RCUCBFunc *func);
>>> +extern void drain_call_rcu(void);
>>>  
>>>  /* The operands of the minus operator must have the same type,
>>>   * which must be the one that we specify in the cast.
>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>> index 56cee1483f..70877840a2 100644
>>> --- a/qdev-monitor.c
>>> +++ b/qdev-monitor.c
>>> @@ -812,6 +812,8 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, 
>>> Error **errp)
>>>          return;
>>>      }
>>>      dev = qdev_device_add(opts, &local_err);
>>> +    drain_call_rcu();
>>> +
>>>      if (!dev) {
>>>          error_propagate(errp, local_err);
>>>          qemu_opts_del(opts);
>>> @@ -904,6 +906,7 @@ void qmp_device_del(const char *id, Error **errp)
>>>          }
>>>  
>>>          qdev_unplug(dev, errp);
>>> +        drain_call_rcu();
>>>      }
>>>  }
>>>  
>>
>> Subject claims "in hmp_device_del", code has it in qmp_device_add() and
>> qmp_device_del().  Please advise.
> 
> I added it in both, because addition of a device can fail and trigger removal,
> which can also be now delayed due to RCU.
> Since both device_add and device_del aren't used often, the overhead won't
> be a problem IMHO.

Ok, just mention this in the commit message.  It may also be a good idea
to move it from qmp_device_add to the error-propagation section of
qdev_device_add.

Paolo