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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 4/2/21 3:04 PM, Jiayu Hu wrote:
> Users can register async copy device in vring_state_changed(),
> when vhost queue is enabled. However, a deadlock occurs inside
> rte_vhost_async_channel_register(), if VHOST_USER_F_PROTOCOL_FEATURES
> is not supported, as vhost_user_msg_handler() takes vq->access_lock
> before calling vhost_user_set_vring_kick().
> 
> This patch avoids async register deadlock by removing calling
> vring_state_changed() in vhost_user_set_vring_kick(). It's safe
> as vhost_user_msg_handler() will call vring_state_changed() anyway.
> 
> Signed-off-by: Jiayu Hu <jiayu.hu@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 44c0452..8f0eba6 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1918,9 +1918,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  	 */
>  	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
>  		vq->enabled = true;
> -		if (dev->notify_ops->vring_state_changed)
> -			dev->notify_ops->vring_state_changed(
> -				dev->vid, file.index, 1);
>  	}
>  
>  	if (vq->ready) {
> 

As replied earlier on v1, I agree the call to vring_state_changed here
is not needed. But it might not be enough, there are other cases where
you could have issues.

Please add stable and Fixes tag.

Thanks,
Maxime