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 7791A45DA4; Mon, 25 Nov 2024 17:28:54 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 11F8640265; Mon, 25 Nov 2024 17:28:54 +0100 (CET) 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 6169A4021F for ; Mon, 25 Nov 2024 17:28:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1732552132; 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=uyfBtFt46+Lx54fmcfoyCaPmontE8KJMP/VomQXCVA8=; b=GIxVdtzSutLS5pcH5RCERtJMed+8iojSv0o6G9jO8+VN/xVifd4y4ojJsYH3FD78jva+19 vDMOuZQCiHxX/2gWWHWL+hyBgIN2pCUBnrdHxowwrJ3jv4+HCnM1V2UWUr6hRHbAN1Ir5N EzpBxk8dVn4YCVQZKr1Ih+FR1/PU6sU= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-119-bFsFL7OjMXuN4sOClc1_iw-1; Mon, 25 Nov 2024 11:28:50 -0500 X-MC-Unique: bFsFL7OjMXuN4sOClc1_iw-1 X-Mimecast-MFC-AGG-ID: bFsFL7OjMXuN4sOClc1_iw Received: by mail-lf1-f69.google.com with SMTP id 2adb3069b0e04-53de765030bso397385e87.1 for ; Mon, 25 Nov 2024 08:28:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732552129; x=1733156929; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uyfBtFt46+Lx54fmcfoyCaPmontE8KJMP/VomQXCVA8=; b=tZ2zXr+nR6tUW2Ml57b4oP1SMXfZ9/UefygpspIyqtJxRq5lDwabbVfS054TnllM/H QGH5k6iLMowxbtjDGzT4LrDHWmXUBMutCsxHGt62dpjmeBhaISTt5P5btEc+GkPanTNv nYXEFsu10E9gJ+7HA9S2AvvihmJU+PKqp1fE6BBrgMdkmv+HeeutIuso8VF3zitU0hU0 6hQZNR1jTapFY4o4GjSaf3ULIVJGntnGWXLr4Ps6MrDcP0xYc+yZ4V1MveFK/qdkJvlM Tk68Mk3PJk7sCe/u427UDS92uhsI9+0J+o/R6gEdQJo8L8vKHt2ov1h4t6EsoL3KBHgK 80tg== X-Forwarded-Encrypted: i=1; AJvYcCWqkWfPxb8eR/+jYYe9m2VU2L8Eb7ojA4IomV1TBmnxp2jK7xHkyptkIAnj/rg8IUB/Z74=@dpdk.org X-Gm-Message-State: AOJu0YydCBFSOz/ly1aUvVr9cytQNfMim1mDA05fkzSjJ0FDpXdF5ayG G+J+k1wc95EiT2k5I5IrfX+knyJg0f7whjaBtmP+pBBFrUrS+EYrWrX3EFoDgdugOqrtNYH1ZH4 zqAMNNBTMqoMiCT4QdxdiX0mstkd8E3vFsLHGJKaggwPRXSyE+nrfMDvU4Tv80riLmp7O/ImQ12 9Orw/2mZ7+Ekp0P4g= X-Gm-Gg: ASbGncuHdL1fFM/KhBkJqXCvC2P/eo65epd/E4m5uR/EVrC2FCgWgTj9ifCnJqVozJZ XVTpE/hsIpi0eEGbJQmz2pio1aeLj3yxs1A== X-Received: by 2002:a05:6512:3f06:b0:53d:e583:b3a5 with SMTP id 2adb3069b0e04-53de583b444mr1735664e87.37.1732552129037; Mon, 25 Nov 2024 08:28:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IEBjcKHFZ1yN12ZG59M1ENOfC62Q/Ndmk4GFC2d5OedUn6gwAXEQ0bBvgL8Y+UZaBiUMkLootMaETLOSna+b6U= X-Received: by 2002:a05:6512:3f06:b0:53d:e583:b3a5 with SMTP id 2adb3069b0e04-53de583b444mr1735650e87.37.1732552128709; Mon, 25 Nov 2024 08:28:48 -0800 (PST) MIME-Version: 1.0 References: <20241118162424.9038-1-stephen@networkplumber.org> <9ea5b1d6-bafa-44d5-bdb7-1eaeaa62a587@redhat.com> In-Reply-To: <9ea5b1d6-bafa-44d5-bdb7-1eaeaa62a587@redhat.com> From: David Marchand Date: Mon, 25 Nov 2024 17:28:37 +0100 Message-ID: Subject: Re: [PATCH] vhost: fix read vs write lock mismatch To: Maxime Coquelin Cc: Stephen Hemminger , dev@dpdk.org, stable@dpdk.org, Chenbo Xia , Eelco Chaudron X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: qLe0fSgl-UVSc-btLjr5Dnz6JkNomkUvHdHDlKwPUt8_1732552129 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Mon, Nov 25, 2024 at 5:20=E2=80=AFPM Maxime Coquelin wrote: > On 11/25/24 12:14, David Marchand wrote: > > On Mon, Nov 18, 2024 at 5:24=E2=80=AFPM Stephen Hemminger > > wrote: > >> > >> If lock is acquired for write, it must be released for write > >> or a deadlock is likely. > >> > >> Bugzilla ID: 1582 > >> Fixes: 9fc93a1e2320 ("vhost: fix virtqueue access check in datapath") > >> Cc: david.marchand@redhat.com > >> Cc: stable@dpdk.org > >> > >> Signed-off-by: Stephen Hemminger > >> --- > >> lib/vhost/virtio_net.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c > >> index 298a5dae74..d764d4bc6a 100644 > >> --- a/lib/vhost/virtio_net.c > >> +++ b/lib/vhost/virtio_net.c > >> @@ -2538,7 +2538,7 @@ virtio_dev_rx_async_submit(struct virtio_net *de= v, struct vhost_virtqueue *vq, > >> > >> if (unlikely(!vq->access_ok)) { > >> vhost_user_iotlb_rd_unlock(vq); > >> - rte_rwlock_read_unlock(&vq->access_lock); > >> + rte_rwlock_write_unlock(&vq->access_lock); > > > > A write lock is taken earlier, because virtio_dev_rx_async_submit_* > > need it for access to vq->async (as opposed to the sync code that only > > takes read lock). > > > > Here, no need to release/take again all locks. > > A simpler fix is to directly call vring_translate(dev, vq). > > > > > > Ok, so both solutions are correct. > > David's one is more optimized, but this is a corner case in the async > datapath so not really critical. > > On the other hand, Stephen's solution keeps the same pattern as the > other datapaths. Ok, it avoids having a special case, and I prefer limiting special cases to= o. I'll send a RFC for new lock annotations (for catching such bug) for the next release. > > I'd go with Stephen's solution, but would change the commit title to: > > vhost: fix deadlock in Rx async path > > With this change: > > Reviewed-by: Maxime Coquelin Yes, the title needed some work. --=20 David Marchand