[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] system/qdev-monitor: move drain_call_rcu call under if (!dev
From: |
Igor Mammedov |
Subject: |
Re: [PATCH] system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add() |
Date: |
Tue, 30 Apr 2024 16:27:39 +0200 |
On Fri, 3 Nov 2023 13:56:02 +0300
Dmitrii Gavrilov <ds-gavr@yandex-team.ru> wrote:
Seems related to cpu hotpug issues,
CCing Boris for awareness.
> Original goal of addition of drain_call_rcu to qmp_device_add was to cover
> the failure case of qdev_device_add. It seems call of drain_call_rcu was
> misplaced in 7bed89958bfbf40df what led to waiting for pending RCU callbacks
> under happy path too. What led to overall performance degradation of
> qmp_device_add.
>
> In this patch call of drain_call_rcu moved under handling of failure of
> qdev_device_add.
>
> Signed-off-by: Dmitrii Gavrilov <ds-gavr@yandex-team.ru>
> ---
> system/qdev-monitor.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 1b8005a..dc7b02d 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -856,19 +856,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> Error **errp)
> return;
> }
> dev = qdev_device_add(opts, errp);
> -
> - /*
> - * Drain all pending RCU callbacks. This is done because
> - * some bus related operations can delay a device removal
> - * (in this case this can happen if device is added and then
> - * removed due to a configuration error)
> - * to a RCU callback, but user might expect that this interface
> - * will finish its job completely once qmp command returns result
> - * to the user
> - */
> - drain_call_rcu();
> -
> if (!dev) {
> + /*
> + * Drain all pending RCU callbacks. This is done because
> + * some bus related operations can delay a device removal
> + * (in this case this can happen if device is added and then
> + * removed due to a configuration error)
> + * to a RCU callback, but user might expect that this interface
> + * will finish its job completely once qmp command returns result
> + * to the user
> + */
> + drain_call_rcu();
> +
> qemu_opts_del(opts);
> return;
> }