[dpdk-dev] [PATCH 3/4] vhost: avoid deadlock on async register

[Maxime Coquelin]

On 3/17/21 1:56 PM, Jiayu Hu wrote:
> Users register async copy device when vhost queue is enabled.
> However, if VHOST_USER_F_PROTOCOL_FEATURES is not supported,
> a deadlock occurs inside rte_vhost_async_channel_register(),
> as vhost_user_msg_handler() already takes vq->access_lock
> before processing VHOST_USER_SET_VRING_KICK message.
> 
> This patch removes calling vring_state_changed() in
> vhost_user_set_vring_kick() to avoid deadlock on async register.
> 
> Signed-off-by: Jiayu Hu <jiayu.hu at intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 399675c..a319c1c 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1919,9 +1919,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  	 */
>  	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
>  		vq->enabled = 1;
> -		if (dev->notify_ops->vring_state_changed)
> -			dev->notify_ops->vring_state_changed(
> -				dev->vid, file.index, 1);

That looks very wrong, as:
1. The apps want to receive this notification. It looks like breaking
existing apps in order to support the experimental async datapath. E.g.
OVS needs it to start polling the queues when protocol features is not
negotiated.

2. The fix in your case seems to indicate that your app's
vring_state_changed callback called rte_vhost_async_channel_register.
And your fix consists in no more calling the callback, and so no more
calling rte_vhost_async_channel_register?

>  	}
>  
>  	if (vq->ready) {
>