Re: [PATCH 7/7] qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict

Akihiko Odaki <[email protected]> Sun, 26 Jul 2026 16:31:15 +0900
Newsgroups org.kernel.vger.linux-cxl,org.nongnu.qemu-devel
Message-ID <[email protected]>
On 2026/07/25 23:00, Marc-André Lureau wrote:
> qmp_device_add() and hmp_device_add() drain pending RCU callbacks after
> a failed device_add, since some bus teardown (e.g. bus_remove_child())
> is deferred to call_rcu(). -device on the command line reaches the same
> qdev_device_add_from_qdict() with errp pointing at error_fatal, whose
> ERRP_GUARD() exits before control returns to the caller, so the caller's
> drain_call_rcu() never runs.
> 
> Move the drain into qdev_device_add_from_qdict()'s own err_del_dev path
> so it always runs, and drop the now-redundant calls in qmp_device_add()
> and hmp_device_add().

It is reachable from virtio-net failover during guest feature 
negotiation. MMIO dispatch holds an RCU read lock at 
address_space_write(). If deferred primary creation fails, 
drain_call_rcu() waits for a grace period while its own thread remains a 
reader, deadlocking the VM.

Regards,
Akihiko Odaki

> 
> Signed-off-by: Marc-AndrĂ© Lureau <[email protected]>
> ---
>   system/qdev-monitor.c | 33 ++++++++++-----------------------
>   1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 00fed791cce1..884d407d60d4 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -746,6 +746,16 @@ err_del_dev:
>       object_unparent(OBJECT(dev));
>       object_unref(OBJECT(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 a caller might expect
> +     * the failed device to be fully torn down once this function
> +     * returns.
> +     */
> +    drain_call_rcu();
> +
>       return NULL;
>   }
>   
> @@ -870,18 +880,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
>       DeviceState *dev;
>   
>       dev = qdev_device_add_from_qdict(qdict, true, errp);
> -    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();
> -    }
>       object_unref(OBJECT(dev));
>   }
>   
> @@ -1017,17 +1015,6 @@ void hmp_device_add(Monitor *mon, const QDict *qdict)
>       }
>       dev = qdev_device_add(opts, &err);
>       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);
>       }
>       object_unref(dev);
>