On Mon, Mar 30, 2026, at 5:24 PM, Rosen Penev wrote: > On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <[email protected]> wrote: >> >> [Cc +Leon] >> >> On Wed, 25 Mar 2026 19:37:47 -0700 >> Rosen Penev <[email protected]> wrote: >> >> > Simplify allocation by using a flexible array member and kzalloc_flex. >> > Less memory management needed. >> > >> > Use __counted_by for extra runtime analysis. Move assignment to after >> > allocation as required by __counted_by. >> >> I don't understand this statement, nr_ranges was previously set after >> the allocation of phys_vec. The only reordering was relative to >> setting vdev, but that appears arbitrary. > Yes that one. My understanding is __counted_by mandates immediate > assignment after allocation. Otherwise UBSAN complains. >> >> In fact, we don't need to explicitly set the __counted_by variable at >> all, kzalloc_flex() handles that. So if anything, it's now redundant. > Redundant with GCC`15 and above.
Sorry, this seems like a -2 on Rusty's manifesto of API design[1]. Why are we doing this? We could do the math to do a single allocation and add the annotation for UBSAN without introducing this helper that's not obvious how to use correctly and at best introduces a redundant counter initialization. Thanks, Alex [1] https://gist.github.com/mjball/9cd028ac793ae8b351df1379f1e721f9 >> >> Leon, any other comments? This should have a v2 removing the >> redundancy and fixing the commit log. >> >> NB. This will be a bit messy to merge since kref and completion exist in >> linux-next via drm, but maybe Linus will consolidate the hole in the >> structure when he resolves it. Thanks, >> >> Alex >> >> > >> > Signed-off-by: Rosen Penev <[email protected]> >> > --- >> > drivers/vfio/pci/vfio_pci_dmabuf.c | 18 +++++------------- >> > 1 file changed, 5 insertions(+), 13 deletions(-) >> > >> > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c >> > b/drivers/vfio/pci/vfio_pci_dmabuf.c >> > index 3a803923141b..40e7e035a720 100644 >> > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c >> > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c >> > @@ -14,12 +14,12 @@ struct vfio_pci_dma_buf { >> > struct vfio_pci_core_device *vdev; >> > struct list_head dmabufs_elm; >> > size_t size; >> > - struct phys_vec *phys_vec; >> > struct p2pdma_provider *provider; >> > u32 nr_ranges; >> > struct kref kref; >> > struct completion comp; >> > u8 revoked : 1; >> > + struct phys_vec phys_vec[] __counted_by(nr_ranges); >> > }; >> > >> > static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, >> > @@ -95,7 +95,6 @@ static void vfio_pci_dma_buf_release(struct dma_buf >> > *dmabuf) >> > up_write(&priv->vdev->memory_lock); >> > vfio_device_put_registration(&priv->vdev->vdev); >> > } >> > - kfree(priv->phys_vec); >> > kfree(priv); >> > } >> > >> > @@ -258,33 +257,28 @@ int vfio_pci_core_feature_dma_buf(struct >> > vfio_pci_core_device *vdev, u32 flags, >> > if (ret) >> > goto err_free_ranges; >> > >> > - priv = kzalloc_obj(*priv); >> > + priv = kzalloc_flex(*priv, phys_vec, get_dma_buf.nr_ranges); >> > if (!priv) { >> > ret = -ENOMEM; >> > goto err_free_ranges; >> > } >> > - priv->phys_vec = kzalloc_objs(*priv->phys_vec, >> > get_dma_buf.nr_ranges); >> > - if (!priv->phys_vec) { >> > - ret = -ENOMEM; >> > - goto err_free_priv; >> > - } >> > >> > - priv->vdev = vdev; >> > priv->nr_ranges = get_dma_buf.nr_ranges; >> > + priv->vdev = vdev; >> > priv->size = length; >> > ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider, >> > get_dma_buf.region_index, >> > priv->phys_vec, dma_ranges, >> > priv->nr_ranges); >> > if (ret) >> > - goto err_free_phys; >> > + goto err_free_priv; >> > >> > kfree(dma_ranges); >> > dma_ranges = NULL; >> > >> > if (!vfio_device_try_get_registration(&vdev->vdev)) { >> > ret = -ENODEV; >> > - goto err_free_phys; >> > + goto err_free_priv; >> > } >> > >> > exp_info.ops = &vfio_pci_dmabuf_ops; >> > @@ -323,8 +317,6 @@ int vfio_pci_core_feature_dma_buf(struct >> > vfio_pci_core_device *vdev, u32 flags, >> > dma_buf_put(priv->dmabuf); >> > err_dev_put: >> > vfio_device_put_registration(&vdev->vdev); >> > -err_free_phys: >> > - kfree(priv->phys_vec); >> > err_free_priv: >> > kfree(priv); >> > err_free_ranges: >>
