From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 08FF5A0A0F; Mon, 29 Mar 2021 17:19:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9AFA2140DFC; Mon, 29 Mar 2021 17:19:13 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id B75EF406FF for ; Mon, 29 Mar 2021 17:19:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617031151; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e3ckUgvw/in+FO36HrNNLsUlQEYdQ4jE09JnVVn6xxA=; b=E7fYobdWjN9viVUoMHbIFljKe/eeh4pS3nKtMKeylPEwxhPLFAtXHQh0ARB0RTAi8qnMhw J4iGvXTWrb9FFNDCZDPbVPa3YBgnINJKjySm0EzYOpDaKmkogtoLcrT2YEr9rq2s6ndmR2 qlJTad5fkWBeVVDH3j/i6PfFC3y3D1A= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-374-Fhg6OqT9N7WlS5k2k1Y_rw-1; Mon, 29 Mar 2021 11:19:10 -0400 X-MC-Unique: Fhg6OqT9N7WlS5k2k1Y_rw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DF0531853026; Mon, 29 Mar 2021 15:19:08 +0000 (UTC) Received: from [10.36.110.27] (unknown [10.36.110.27]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BB44610023AC; Mon, 29 Mar 2021 15:19:05 +0000 (UTC) To: Jiayu Hu , dev@dpdk.org Cc: chenbo.xia@intel.com, yinan.wang@intel.com, cheng1.jiang@intel.com, sunil.pai.g@intel.com References: <1615985773-406787-1-git-send-email-jiayu.hu@intel.com> <1615985773-406787-4-git-send-email-jiayu.hu@intel.com> From: Maxime Coquelin Message-ID: <8cd04e21-8acd-7fe6-c0c3-dc162c137c4a@redhat.com> Date: Mon, 29 Mar 2021 17:19:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <1615985773-406787-4-git-send-email-jiayu.hu@intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 3/4] vhost: avoid deadlock on async register X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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) { >