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 9BBA943215; Fri, 27 Oct 2023 12:11:33 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 42A7040263; Fri, 27 Oct 2023 12:11:33 +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 EAB3B4025E for ; Fri, 27 Oct 2023 12:11:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1698401491; 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=1Mwrn0QG//YCt6thlzdJped7+Ez6J1X8x0p50qVQydw=; b=JzvPR0Bmhk0BjsSuzkdYnc2h7yknEvKE8fnEIqx05ArZPR64rqm+9WgpQ84rbdvfud+JOO NxQvMeshIk9aZ/x3L3x8FNaKNK3qBQwbBqsBNOSg3swZIcsxuA6AfGojnjCHkB9NVzgMBa 543DzrNxpkNJVKKRAtTtm7VgE2dVlxA= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-643-oEJjJ25YMwe_w0jBJPy7LA-1; Fri, 27 Oct 2023 06:11:30 -0400 X-MC-Unique: oEJjJ25YMwe_w0jBJPy7LA-1 Received: by mail-ed1-f70.google.com with SMTP id 4fb4d7f45d1cf-53e305a5a1bso1456184a12.1 for ; Fri, 27 Oct 2023 03:11:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698401489; x=1699006289; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ck26CZHqvpJk2h9iyrsIbJJJZauDrEQIvJ4W5asHcm0=; b=h0Oa9Qo4ovNKPmrdfJjj+OY0Og58EHAq5JorGoh6w8AA/A1BSsRAIxsV5Uh/i9d9C3 rUyTNU5FR3Tk3F7rTRlpFkG3DvTE7jNN6tM08E42Q9R2+iTC4nvskfyaIqPSP7pjZIgi QF/7Wkwb/ejljOV0FzobzKM35Slo4MuR5SlB7CeQfajF3SUgJaw+bfVd+qU+ypq0AhwC wuKZxAUT9ofRIgSNHWDn+ZPD799ycGT9Wu45oYHfEg5/EQH9PQsHWd+Nb8hZyUoi1jA0 DJhCdlw2q60PLNvsfrex6Dg9eYVczf9aSrbsV3Wt/1x+uYn+R6SeV4R1YWTz0XZ+JgU3 S2zg== X-Gm-Message-State: AOJu0Ywz1rjR+zUJK6bi00Wul68wpywK6sM3ddaihbSbIG1CJA4xil5O nT1OIrueMMXp/nDyPprHq9+nqBgF2kZuUTQR77B6LLeAQvdX3O16RxLRfmeCa/pqyKsFDvJvsWV +kYw= X-Received: by 2002:a17:906:7309:b0:9bf:1aff:6e03 with SMTP id di9-20020a170906730900b009bf1aff6e03mr1836039ejc.65.1698401489191; Fri, 27 Oct 2023 03:11:29 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFyqv+qqRNphgBOO97rAD78/6d8dIHkZdY7e4+eqgSftqcrRXZB/TTFZq4VjxxLn1R4pL0qMQ== X-Received: by 2002:a17:906:7309:b0:9bf:1aff:6e03 with SMTP id di9-20020a170906730900b009bf1aff6e03mr1836018ejc.65.1698401488794; Fri, 27 Oct 2023 03:11:28 -0700 (PDT) Received: from [172.16.1.27] (5920ab7b.static.cust.trined.nl. [89.32.171.123]) by smtp.gmail.com with ESMTPSA id r23-20020a170906365700b009a1a653770bsm946064ejb.87.2023.10.27.03.11.27 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Oct 2023 03:11:27 -0700 (PDT) From: Eelco Chaudron To: David Marchand Cc: dev@dpdk.org, stable@dpdk.org, Maxime Coquelin , Chenbo Xia Subject: Re: [PATCH 2/3] vhost: fix virtqueue access lock in datapath Date: Fri, 27 Oct 2023 12:11:27 +0200 X-Mailer: MailMate (1.14r5964) Message-ID: <2569D0EC-C478-4AEA-9F86-3CD8BA99C42A@redhat.com> In-Reply-To: References: <20231023095520.2864868-1-david.marchand@redhat.com> <20231023095520.2864868-2-david.marchand@redhat.com> <76029D4A-DA8B-4CE9-ACFC-0B7D5BFF8512@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 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 27 Oct 2023, at 11:22, David Marchand wrote: > On Fri, Oct 27, 2023 at 11:05=E2=80=AFAM Eelco Chaudron wrote: >>> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c >>> index 759a78e3e3..4116f79d4f 100644 >>> --- a/lib/vhost/virtio_net.c >>> +++ b/lib/vhost/virtio_net.c >>> @@ -1694,6 +1694,17 @@ virtio_dev_rx_packed(struct virtio_net *dev, >>> return pkt_idx; >>> } >>> >>> +static void >>> +virtio_dev_vring_translate(struct virtio_net *dev, struct vhost_virtqu= eue *vq) >>> +{ >> >> Would it be an idea to annotate this function that it needs to be called= with the =E2=80=9Cread locks=E2=80=9D (and that it will free them) to avoi= d the duplicate: >> >> + vhost_user_iotlb_rd_unlock(vq); >> + rte_rwlock_read_unlock(&vq->access_lock); > > The "unlock" annotations do not express read/write concerns for locks. > So that would make the code less readable and potentially hide some issue= s. > > I prefer to keep as is, with clear calls to rd_lock / rd_unlock in > those functions. ACK, keeping this as is fine by me. Acked-by: Eelco Chaudron >>> + rte_rwlock_write_lock(&vq->access_lock); >>> + vhost_user_iotlb_rd_lock(vq); >>> + if (!vq->access_ok) >>> + vring_translate(dev, vq); >>> + vhost_user_iotlb_rd_unlock(vq); >>> + rte_rwlock_write_unlock(&vq->access_lock); >>> +} >>> + > > --=20 > David Marchand