Re: [PATCH] vfio: pci: use kzalloc_flex

[Leon Romanovsky]

On Mon, Mar 30, 2026 at 06:23:05PM -0600, Alex Williamson wrote:
> On Mon, Mar 30, 2026, at 5:24 PM, Rosen Penev wrote:
> > On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <alex@xxxxxxxxxxx> wrote:
> >>
> >> [Cc +Leon]
> >>
> >> On Wed, 25 Mar 2026 19:37:47 -0700
> >> Rosen Penev <rosenp@xxxxxxxxx> 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,

My comment is that we can simply drop this patch. It adds no value and only
creates unnecessary churn.

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 <rosenp@xxxxxxxxx>
> >> > ---
> >> >  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:
> >>