From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Jiayu Hu <jiayu.hu@intel.com>, dev@dpdk.org
Cc: chenbo.xia@intel.com, yinan.wang@intel.com,
cheng1.jiang@intel.com, sunil.pai.g@intel.com
Subject: Re: [dpdk-dev] [PATCH 3/4] vhost: avoid deadlock on async register
Date: Mon, 29 Mar 2021 17:19:03 +0200 [thread overview]
Message-ID: <8cd04e21-8acd-7fe6-c0c3-dc162c137c4a@redhat.com> (raw)
In-Reply-To: <1615985773-406787-4-git-send-email-jiayu.hu@intel.com>
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@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) {
>
next prev parent reply other threads:[~2021-03-29 15:19 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-17 12:56 [dpdk-dev] [PATCH 0/4] Refactor async vhost control path Jiayu Hu
2021-03-17 12:56 ` [dpdk-dev] [PATCH 1/4] vhost: fix uninitialized vhost queue Jiayu Hu
2021-03-26 15:14 ` Maxime Coquelin
2021-03-17 12:56 ` [dpdk-dev] [PATCH 2/4] vhost: remove unnecessary free Jiayu Hu
2021-03-29 15:03 ` Maxime Coquelin
2021-03-17 12:56 ` [dpdk-dev] [PATCH 3/4] vhost: avoid deadlock on async register Jiayu Hu
2021-03-29 15:19 ` Maxime Coquelin [this message]
2021-03-30 1:20 ` Hu, Jiayu
2021-04-13 9:37 ` Maxime Coquelin
2021-03-17 12:56 ` [dpdk-dev] [PATCH 4/4] doc: update async vhost register/unregister Jiayu Hu
2021-04-02 13:03 ` [dpdk-dev] [PATCH v2 0/4] Refactor async vhost control path Jiayu Hu
2021-04-02 8:06 ` Wang, Yinan
2021-04-02 13:03 ` [dpdk-dev] [PATCH v2 1/4] vhost: fix uninitialized vhost queue Jiayu Hu
2021-04-13 11:30 ` Maxime Coquelin
2021-04-02 13:04 ` [dpdk-dev] [PATCH v2 2/4] vhost: remove unnecessary free Jiayu Hu
2021-04-02 13:04 ` [dpdk-dev] [PATCH v2 3/4] vhost: avoid deadlock on async register Jiayu Hu
2021-04-13 11:33 ` Maxime Coquelin
2021-04-14 1:40 ` Hu, Jiayu
2021-04-14 10:08 ` Maxime Coquelin
2021-04-15 1:08 ` Hu, Jiayu
2021-04-15 7:09 ` Maxime Coquelin
2021-04-16 2:19 ` Hu, Jiayu
2021-04-16 6:35 ` Maxime Coquelin
2021-04-16 8:18 ` Hu, Jiayu
2021-04-16 8:33 ` Maxime Coquelin
2021-04-19 4:10 ` Hu, Jiayu
2021-04-19 7:13 ` Maxime Coquelin
2021-04-19 9:02 ` Hu, Jiayu
2021-04-02 13:04 ` [dpdk-dev] [PATCH v2 4/4] doc: update async vhost register/unregister Jiayu Hu
2021-04-20 8:57 ` [dpdk-dev] [PATCH v3 0/4] Refactor async vhost control path Jiayu Hu
2021-04-20 8:57 ` [dpdk-dev] [PATCH v3 1/4] vhost: fix uninitialized vhost queue Jiayu Hu
2021-04-20 8:57 ` [dpdk-dev] [PATCH v3 2/4] vhost: remove unnecessary free Jiayu Hu
2021-04-20 8:57 ` [dpdk-dev] [PATCH v3 3/4] vhost: fix unnecessary vring_state_changed call Jiayu Hu
2021-04-20 9:39 ` Maxime Coquelin
2021-04-21 1:36 ` Xia, Chenbo
2021-04-20 8:57 ` [dpdk-dev] [PATCH v3 4/4] doc: update async vhost register/unregister Jiayu Hu
2021-04-20 9:31 ` Maxime Coquelin
2021-04-28 2:05 ` [dpdk-dev] [PATCH v3 0/4] Refactor async vhost control path Xia, Chenbo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8cd04e21-8acd-7fe6-c0c3-dc162c137c4a@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=chenbo.xia@intel.com \
--cc=cheng1.jiang@intel.com \
--cc=dev@dpdk.org \
--cc=jiayu.hu@intel.com \
--cc=sunil.pai.g@intel.com \
--cc=yinan.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).