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 C675142E52; Wed, 12 Jul 2023 10:06:16 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5B17A40A7D; Wed, 12 Jul 2023 10:06:16 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 4E86B406BA for ; Wed, 12 Jul 2023 10:06:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689149174; 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=rJJVJB721H5+ONBtAENoz7ZR68YGdK+SPcLYvlvG7Cg=; b=It+i7RnUMECblUlbCjLTln4m5OSqHshQokkXWgBTq5K7kFwAuqFzY3xcWU8oZmEdJwpUzE F2vs5fLU4u26c0TVd20FG0d5KW+vLLJHosPPKO0Fk2KMSfh0iSxO2q9ADyLj9La8JWDEP3 gfnZ6UcRTg2hwawqiTxJ+TydLcF9xs0= Received: from mimecast-mx02.redhat.com (66.187.233.73 [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-611-IvBQcauWMOK6ucUhdIqaFA-1; Wed, 12 Jul 2023 04:06:09 -0400 X-MC-Unique: IvBQcauWMOK6ucUhdIqaFA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4DCF81C16FE7; Wed, 12 Jul 2023 08:06:09 +0000 (UTC) Received: from [10.39.208.24] (unknown [10.39.208.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 42C0F112C062; Wed, 12 Jul 2023 08:06:07 +0000 (UTC) Message-ID: Date: Wed, 12 Jul 2023 10:06:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: =?UTF-8?B?UmU6IOWbnuWkjTog5Zue5aSNOiBbUEFUQ0ggdjNdIHZob3N0OiBhZGQg?= =?UTF-8?Q?notify_reply_ops_to_fix_message_deadlock?= To: Rma Ma , dpdk-dev Cc: Chenbo Xia , "Michael S . Tsirkin" References: <20230704023228.24879-1-rma.ma@jaguarmicro.com> <20230704025157.26024-1-rma.ma@jaguarmicro.com> <13dcdbde-df66-fc17-3eb5-d23bb6836a55@redhat.com> From: Maxime Coquelin In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On 7/12/23 04:17, Rma Ma wrote: > >  >  > Since backend and frontend message are synchronous in the same > thread, > >  >  > there will be a probability of message deadlock. > >  >  > Consider each driver to determine whether to wait for response. > >  >  > > >  >  > Fixes: d90cf7d111ac ("vhost: support host notifier") > >  >  > Cc: maxime.coquelin@redhat.com > >  >  > Signed-off-by: Rma Ma > >  >  > --- > >  >  > v2 - fix format error in commit message > >  >  > v3 - add --in-reply-to > >  >  > --- > >  > > >  > Hi Maxime, > >  > > >  > This patch helps to fix vhost-user message deadlock, could you help > >  > review it? > > > >  The patch introduces a new device op, but it is used nowhere in vDPA > >  drivers. > > > >  What vDPA driver is it going to be used with? > > > >  Regards, > >  Maxime > > > Hi, > > Our company's jmnd vdpa driver, which requires the > rte_vhost_host_notifier_ctrl interface, > replicates the problem with the following scenario: > > QEMU start vhost-user with modern net and blk, backend use dpdk-vdpa > process, > after live migration, dest QEMU deadlock with dpdk-vdpa. > > - QEMU sends VHOST_USER_SET_VRING_KICK to dpdk-vdpa net > - QEMU does not need to wait for a response to this message > - QEMU then sends VHOST_USER_SET_MEM_TABLE to dpdk-vdpa blk > - QEMU needs to wait reply in this message > - when dpdk-vdpa recv VHOST_USER_SET_VRING_KICK, > - it will send VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG to QEMU > - dpdk-vdpa needs to wait for a response to this message > - QEMU will deadlock with dpdk-vdpa > > I tried to add a patch to the qemu community that uses a new thread to > loop backend channel, > But there will be some multi-threaded synchronization issues > > I think this is a public issue, and other backend messages take this > into account, > so I think this message also needs a flag to fix it. > > and jmnd vdpa driver will subsequently be open-sourced to the community. > Thanks forthe clarification. Please submit this patch in the same series that introduce jmnd vDPA driver into upstream DPDK. Indeed, we need to have an internal driver that makes use of this new op for it to be accepted. Otherwise, we have no way to maintain it. Regards, Maxime > > Best wishes, > > Rma > > ------------------------------------------------------------------------ > *发件人:* Maxime Coquelin > *发送时间:* 2023年7月11日 21:25 > *收件人:* Rma Ma ; dpdk-dev > *抄送:* Chenbo Xia > *主题:* Re: 回复: [PATCH v3] vhost: add notify reply ops to fix message > deadlock > Hi, > > On 7/11/23 11:25, Rma Ma wrote: >>  > Since backend and frontend message are synchronous in the same thread, >>  > there will be a probability of message deadlock. >>  > Consider each driver to determine whether to wait for response. >>  > >>  > Fixes: d90cf7d111ac ("vhost: support host notifier") >>  > Cc: maxime.coquelin@redhat.com >>  > Signed-off-by: Rma Ma >>  > --- >>  > v2 - fix format error in commit message >>  > v3 - add --in-reply-to >>  > --- >> >> Hi Maxime, >> >> This patch helps to fix vhost-user message deadlock, could you help >> review it? > > The patch introduces a new device op, but it is used nowhere in vDPA > drivers. > > What vDPA driver is it going to be used with? > > Regards, > Maxime > >> Thanks. >> >> Best wishes, >> >> Rma >> >> ------------------------------------------------------------------------ >> *发件人:* Rma Ma >> *发送时间:* 2023年7月4日 10:52 >> *收件人:* dpdk-dev >> *抄送:* Maxime Coquelin ; Chenbo Xia >> ; Rma Ma >> *主题:* [PATCH v3] vhost: add notify reply ops to fix message deadlock >> Since backend and frontend message are synchronous in the same thread, >> there will be a probability of message deadlock. >> Consider each driver to determine whether to wait for response. >> >> Fixes: d90cf7d111ac ("vhost: support host notifier") >> Cc: maxime.coquelin@redhat.com >> Signed-off-by: Rma Ma >> --- >> v2 - fix format error in commit message >> v3 - add --in-reply-to >> --- >>   lib/vhost/vdpa_driver.h |  3 +++ >>   lib/vhost/vhost_user.c  | 23 ++++++++++++++++++----- >>   2 files changed, 21 insertions(+), 5 deletions(-) >> >> diff --git a/lib/vhost/vdpa_driver.h b/lib/vhost/vdpa_driver.h >> index 8db4ab9f4d..3d2ea3c90e 100644 >> --- a/lib/vhost/vdpa_driver.h >> +++ b/lib/vhost/vdpa_driver.h >> @@ -81,6 +81,9 @@ struct rte_vdpa_dev_ops { >> >>           /** get device type: net device, blk device... */ >>           int (*get_dev_type)(struct rte_vdpa_device *dev, uint32_t *type); >> + >> +       /** Get the notify reply flag */ >> +       int (*get_notify_reply_flag)(int vid, bool *need_reply); >>   }; >> >>   /** >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c >> index 901a80bbaa..aa61992939 100644 >> --- a/lib/vhost/vhost_user.c >> +++ b/lib/vhost/vhost_user.c >> @@ -3365,13 +3365,14 @@ rte_vhost_backend_config_change(int vid, bool >> need_reply) >>   static int vhost_user_backend_set_vring_host_notifier(struct >> virtio_net *dev, >>                                                       int index, int fd, >>                                                       uint64_t offset, >> -                                                   uint64_t size) >> +                                                   uint64_t size, >> +                                                       bool need_reply) >>   { >>           int ret; >>           struct vhu_msg_context ctx = { >>                   .msg = { >>                           .request.backend = >> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG, >> -                       .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY, >> +                       .flags = VHOST_USER_VERSION, >>                           .size = sizeof(ctx.msg.payload.area), >>                           .payload.area = { >>                                   .u64 = index & VHOST_USER_VRING_IDX_MASK, >> @@ -3388,7 +3389,13 @@ static int >> vhost_user_backend_set_vring_host_notifier(struct virtio_net *dev, >>                   ctx.fd_num = 1; >>           } >> >> -       ret = send_vhost_backend_message_process_reply(dev, &ctx); >> +       if (!need_reply) >> +               ret = send_vhost_backend_message(dev, &ctx); >> +       else { >> +               ctx.msg.flags |= VHOST_USER_NEED_REPLY; >> +               ret = send_vhost_backend_message_process_reply(dev, &ctx); >> +       } >> + >>           if (ret < 0) >>                   VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to set host >> notifier (%d)\n", ret); >> >> @@ -3402,6 +3409,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t >> qid, bool enable) >>           int vfio_device_fd, ret = 0; >>           uint64_t offset, size; >>           unsigned int i, q_start, q_last; >> +       bool need_reply; >> >>           dev = get_device(vid); >>           if (!dev) >> @@ -3440,6 +3448,11 @@ int rte_vhost_host_notifier_ctrl(int vid, >> uint16_t qid, bool enable) >>           if (vfio_device_fd < 0) >>                   return -ENOTSUP; >> >> +       if (vdpa_dev->ops->get_notify_reply_flag == NULL) >> +               need_reply = true; >> +       else >> +               vdpa_dev->ops->get_notify_reply_flag(vid, &need_reply); >> + >>           if (enable) { >>                   for (i = q_start; i <= q_last; i++) { >>                           if (vdpa_dev->ops->get_notify_area(vid, i, >> &offset, >> @@ -3449,7 +3462,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t >> qid, bool enable) >>                           } >> >>                           if >> (vhost_user_backend_set_vring_host_notifier(dev, i, >> -                                       vfio_device_fd, offset, size) < 0) { >> +                                       vfio_device_fd, offset, size, >> need_reply) < 0) { >>                                   ret = -EFAULT; >>                                   goto disable; >>                           } >> @@ -3458,7 +3471,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t >> qid, bool enable) >>   disable: >>                   for (i = q_start; i <= q_last; i++) { >> >> vhost_user_backend_set_vring_host_notifier(dev, i, -1, >> -                                       0, 0); >> +                                       0, 0, need_reply); >>                   } >>           } >> >> -- >> 2.17.1 >> >