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 4AC6745DA5 for ; Mon, 25 Nov 2024 17:28:55 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 42E5840B8F; Mon, 25 Nov 2024 17:28:55 +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 80D4340265 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-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-569-bnyeqiNQNPiIHO0fzpRoYw-1; Mon, 25 Nov 2024 11:28:50 -0500 X-MC-Unique: bnyeqiNQNPiIHO0fzpRoYw-1 X-Mimecast-MFC-AGG-ID: bnyeqiNQNPiIHO0fzpRoYw Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-53ddfdbeeffso1387647e87.2 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=Gaaszrovynl+MFm1aNNcjnfY7CMcigLqPxF9YaQaqxdnJ8bh5xAHHURtiyjf1rvNAL pfNJfyVu90ef09dv8hNVuE9vk+6TbYGT3tLsyzzTAc9AVAzWvv7gl3eFou+CmwBGk3qA NFg4TEfPWJ3VqiubOZvq0L6AhTb2C7QUx0G2j5jSBJ6v2YF8FMVr8Q1ZHnb0vVMiDhaR 6cIDX8KiyC5Veu0z4dpULyDHsN4bHxWqBp1BlOXlOKe3RV9jNaF9E6maPCivWnBucRLh bUlGU0VbNy0iPxuaJ8GamJMdujXmXQicQbkG2kdTeP0CTCRb9hzJa+34okMVbVW0mlWl Xzpg== X-Forwarded-Encrypted: i=1; AJvYcCVBk5hnwKRZobjjZUQ0Hx5LvkLabr0dfWwA8rzMqWRpT688C1dC/Y9paJktPRg3C6Or4SnbF1E=@dpdk.org X-Gm-Message-State: AOJu0Yz8/fCUz0o9MFTv86I8L/qTtALenK+GexCWrMCRGsL25HgLJSER /D+Zi4+/v7A17DZmNXejpos3XAF2tPx45s53pA6zb5bWvmgVxkTr5eFqU4nDOAEoBwaiktjIEgc F1tZxvM6OZdRBxHg6/B6tDOeEbof8SR6cWjuaKFmZNfosWKMyIoiZShw91TZv0vbecUWeC6nqiJ S2GGpxtSfesvCMOvl3zbauU0gcManwcA== X-Gm-Gg: ASbGnctkHWSWg8QHlx+HmIWTMlKvSoHO5F56FHPC6O5BzPSb/+nli2u92Cxv+3W8TK6 FDEh2TPOoj/4PdPXXYpOFCuKBUvtH9u8SCA== X-Received: by 2002:a05:6512:3f06:b0:53d:e583:b3a5 with SMTP id 2adb3069b0e04-53de583b444mr1735665e87.37.1732552129040; 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: p5ffeyqtjofUel9ixtgqUr0hghvA-hY2U-m3BRiYTPQ_1732552129 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-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