From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=HGuyjTso; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1C1685A0265 for ; Wed, 18 Mar 2026 02:17:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773796669; bh=e7sWEQkd2tTlZPkXal2TNvpU0hjXYWDd6NnbEjoVyvI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HGuyjTsoBLgFlHbhZYYKKU7hW2I8LFktkMjb0WlEXgA0hZ0N4lZdHnLKpT7ewjVsU KHBwdD/m8FE8EptOVd6Nc0X2z/F7Ow6HvgLdS4OVScYa3O4sBzuiPk/5Gv7ObFpo8w pz4D+WRAx6EAEFuGTT7nvaQ6ClLYV4mAWlEhxIRrGPGN+gxsh3jHq6JfEvt0QOvFzC 39mans9OBQr02RTAeoSCf+hFdW3FDnLpnhR6VszhsuZHq0DzdYBFQrKwi2gQhmh2OQ QJBKYFkTsG1gsyvrDldn/T2Puk/fCIRqoUyiI1l4rrHR7mVnY/R39bzjCFoMoidR+v p/42UU6KxixtA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fb9tP0Xwmz4w0L; Wed, 18 Mar 2026 12:17:49 +1100 (AEDT) Date: Wed, 18 Mar 2026 12:16:39 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 3/3] vu_common: Move iovec management into vu_collect() Message-ID: References: <20260313182618.4157365-1-lvivier@redhat.com> <20260313182618.4157365-4-lvivier@redhat.com> <20260317162350.058e10e0@elisabeth> <4705f7ab-9277-4462-ada6-6bee39342627@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="B9MUI94sGkbvPN76" Content-Disposition: inline In-Reply-To: <4705f7ab-9277-4462-ada6-6bee39342627@redhat.com> Message-ID-Hash: DJAX34HVK7M4VLXBBJPVLU7YSJFTUWPF X-Message-ID-Hash: DJAX34HVK7M4VLXBBJPVLU7YSJFTUWPF X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Stefano Brivio , passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --B9MUI94sGkbvPN76 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 17, 2026 at 05:30:32PM +0100, Laurent Vivier wrote: > On 3/17/26 16:23, Stefano Brivio wrote: > > On Tue, 17 Mar 2026 08:25:49 +0100 > > Laurent Vivier wrote: > >=20 > > > On 3/17/26 03:40, David Gibson wrote: > > > > On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote: > > > > > Previously, callers had to pre-initialize virtqueue elements with= iovec > > > > > entries using vu_set_element() or vu_init_elem() before calling > > > > > vu_collect(). This meant each element owned a fixed, pre-assigne= d iovec > > > > > slot. > > > > >=20 > > > > > Move the iovec array into vu_collect() as explicit parameters (in= _sg, > > > > > max_in_sg, and in_num), letting it pass the remaining iovec capac= ity > > > > > directly to vu_queue_pop(). A running current_iov counter tracks > > > > > consumed entries across elements, so multiple elements share a si= ngle > > > > > iovec pool. The optional in_num output parameter reports how man= y iovec > > > > > entries were consumed, allowing callers to track usage across mul= tiple > > > > > vu_collect() calls. > > > > >=20 > > > > > This removes vu_set_element() and vu_init_elem() which are no lon= ger > > > > > needed, and is a prerequisite for multi-buffer support where a si= ngle > > > > > virtqueue element can use more than one iovec entry. For now, ca= llers > > > > > assert the current single-iovec-per-element invariant until they = are > > > > > updated to handle multiple iovecs. > > > > >=20 > > > > > Signed-off-by: Laurent Vivier > > > >=20 > > > > Reviewed-by: David Gibson > > > >=20 > > > > Couple of thoughts on possible polish below. > > > >=20 > > > > [snip] > > > > > /** > > > > > * vu_collect() - collect virtio buffers from a given virtqueue > > > > > * @vdev: vhost-user device > > > > > * @vq: virtqueue to collect from > > > > > - * @elem: Array of virtqueue element > > > > > - * each element must be initialized with one iovec entry > > > > > - * in the in_sg array. > > > > > + * @elem: Array of @max_elem virtqueue elements > > > > > * @max_elem: Number of virtqueue elements in the array > > > > > + * @in_sg: Incoming iovec array for device-writable descriptors > > > > > + * @max_in_sg: Maximum number of entries in @in_sg > > > > > + * @in_num: Number of collected entries from @in_sg (output) > > > > > * @size: Maximum size of the data in the frame > > > > > * @collected: Collected buffer length, up to @size, set on r= eturn > > > > > * > > > > > @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *el= em, struct iovec *iov, int elem_cnt > > > > > */ > > > > > int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, > > > > > struct vu_virtq_element *elem, int max_elem, > > > > > + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, > > > > > size_t size, size_t *collected) > > > > > { > > > > > size_t current_size =3D 0; > > > > > + size_t current_iov =3D 0; > > > > > int elem_cnt =3D 0; > > > > > - while (current_size < size && elem_cnt < max_elem) { > > > > > - struct iovec *iov; > > > > > + while (current_size < size && elem_cnt < max_elem && > > > > > + current_iov < max_in_sg) { > > > > > int ret; > > > > > ret =3D vu_queue_pop(vdev, vq, &elem[elem_cnt], > > > > > - elem[elem_cnt].in_sg, > > > > > - elem[elem_cnt].in_num, > > > > > - elem[elem_cnt].out_sg, > > > > > - elem[elem_cnt].out_num); > > > > > + &in_sg[current_iov], > > > > > + max_in_sg - current_iov, > > > > > + NULL, 0); > > > > > if (ret < 0) > > > > > break; > > > > > @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, st= ruct vu_virtq *vq, > > > > > break; > > > > > } > > > > > - iov =3D &elem[elem_cnt].in_sg[0]; > > > > > - > > > > > - if (iov->iov_len > size - current_size) > > > > > - iov->iov_len =3D size - current_size; > > > > > + elem[elem_cnt].in_num =3D iov_truncate(elem[elem_cnt].in_sg, > > > > > + elem[elem_cnt].in_num, > > > > > + size - current_size); > > > >=20 > > > > Will elem[].in_num always end up with the same value as the @in_num > > > > parameter? If so, do we need the explicit parameter? > > >=20 > > > @in_num parameter of vu_collect()? > > >=20 > > > @in_num is the sum of all elem[].in_num, it can be computed by the ca= ller function from > > > elem, but it is simpler to return it as we need to compute it in the = loop. > >=20 > > I'm not sure I understood the point of David's comment here, and this > > explanation makes sense to me now, but it took me a bit to figure that > > out. > >=20 > > Could it be that @in_num is a bit confusing as it has "in" and "num" in > > it, but it's actually an output representing how many "in" entries we > > used/need? >=20 > For an element, *=ECn_*num is the number of *in_*sg we have read from the= ring for an element. >=20 > It's virtio semantic, so *in_* means sg going *in* the guest. >=20 > For *out_*sg we have *out_*num. >=20 > >=20 > > What if we rename it to @in_used or @in_collected? > >=20 >=20 > The idea was to keep the same name as in the element. But we can change t= his to @in_used. Would "in_total" work better to suggest that it's the sum of all the elements' in_num? --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --B9MUI94sGkbvPN76 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmm5/PYACgkQzQJF27ox 2GfBWxAAhxGIFdK4BKMxfdvX72Hm2Pd/OIhewH+bNZh9qY7pE7BGOFz+PwTKhDOM 7NGqZ57UoBKb3de9ljZpVkMDx83zDtjg2XGoFtJ8swMAh9mxyDYC4JN9tUOneGoq E7FkH7V2LGKMNHM76jEIRhyxK9MNhEGFwWvsmFVvLXlpgZAY/qNslrPD2cVir4Jb NiZo1nu7KmcyBDpnBkCT1N1UTxxLqpj4pT9d373ySGJVcO9dVwP7dPFnN1iQVts2 jiNFRrpUA3SqCYWlywXJlhkS7Apq+Y2iK+LYFY8afCtYtTvfSQKDK9eitfPIuAMi CNj97Ny7cIFqNGP1jey/QJTkV8VK99WNrOJjldqBX5N20tWrkGcb0m+REqhdMDAC 45K4ctYOkvrdLd+m5eCyrpyewc4p1gpenyr1JvZJS4gMbYaO+++0/IGdreTY5QhA GbOrArPd5wH2OSqzN2lnywm6Ur8cyCtERpcVZJu8KrGQZ7mVL7O/nHrMiY96RNIS gJLUSJqB1tvbOPLofGjYor68F5ZkAz/wA2wvXZ6JMKGZN+7cUXt3gVzcC2SVUnNJ mpSa47Yt1OeQdpQrvbsZJs8U4UqWwESTvHOaoWPs4CDD2eOJjc8+ew5yMR7DPHnc pCmvJgNw8HrTMr/hmDyF7192VeuUKbAzZxncUZjKHn/W2CqBaXc= =84Kk -----END PGP SIGNATURE----- --B9MUI94sGkbvPN76--