On 09/07/20 13:56, Maxim Levitsky wrote:
> On Thu, 2020-07-09 at 13:42 +0200, Markus Armbruster wrote:
>> Maxim Levitsky <mlevi...@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 <stefa...@gmail.com>
>>> Signed-off-by: Maxim Levitsky <mlevi...@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
