* [PATCH] vfio: pci: use kzalloc_flex @ 2026-03-26 2:37 Rosen Penev 2026-03-30 22:19 ` Alex Williamson 0 siblings, 1 reply; 8+ messages in thread From: Rosen Penev @ 2026-03-26 2:37 UTC (permalink / raw) To: kvm Cc: Alex Williamson, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b 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. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- 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: -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-26 2:37 [PATCH] vfio: pci: use kzalloc_flex Rosen Penev @ 2026-03-30 22:19 ` Alex Williamson 2026-03-30 23:24 ` Rosen Penev 0 siblings, 1 reply; 8+ messages in thread From: Alex Williamson @ 2026-03-30 22:19 UTC (permalink / raw) To: Rosen Penev Cc: kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, alex, Leon Romanovsky [Cc +Leon] On Wed, 25 Mar 2026 19:37:47 -0700 Rosen Penev <rosenp@gmail.com> 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. 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. 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@gmail.com> > --- > 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: ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-30 22:19 ` Alex Williamson @ 2026-03-30 23:24 ` Rosen Penev 2026-03-30 23:46 ` Gustavo A. R. Silva 2026-03-31 0:23 ` Alex Williamson 0 siblings, 2 replies; 8+ messages in thread From: Rosen Penev @ 2026-03-30 23:24 UTC (permalink / raw) To: Alex Williamson Cc: kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, Leon Romanovsky On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <alex@shazbot.org> wrote: > > [Cc +Leon] > > On Wed, 25 Mar 2026 19:37:47 -0700 > Rosen Penev <rosenp@gmail.com> 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. > > 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@gmail.com> > > --- > > 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: > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-30 23:24 ` Rosen Penev @ 2026-03-30 23:46 ` Gustavo A. R. Silva 2026-03-31 0:18 ` Rosen Penev 2026-03-31 0:23 ` Alex Williamson 1 sibling, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2026-03-30 23:46 UTC (permalink / raw) To: Rosen Penev, Alex Williamson Cc: kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, Leon Romanovsky On 3/30/26 17:24, Rosen Penev wrote: > On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <alex@shazbot.org> wrote: >> >> [Cc +Leon] >> >> On Wed, 25 Mar 2026 19:37:47 -0700 >> Rosen Penev <rosenp@gmail.com> 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. Not true. However, it's best practice. The requirement is that the _counter_ must be initialized before the first reference to the flexible-array member. -Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-30 23:46 ` Gustavo A. R. Silva @ 2026-03-31 0:18 ` Rosen Penev 2026-03-31 0:51 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Rosen Penev @ 2026-03-31 0:18 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alex Williamson, kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, Leon Romanovsky On Mon, Mar 30, 2026 at 4:47 PM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > > On 3/30/26 17:24, Rosen Penev wrote: > > On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <alex@shazbot.org> wrote: > >> > >> [Cc +Leon] > >> > >> On Wed, 25 Mar 2026 19:37:47 -0700 > >> Rosen Penev <rosenp@gmail.com> 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. > > Not true. However, it's best practice. > > The requirement is that the _counter_ must be initialized before > the first reference to the flexible-array member. OTOH kzalloc_flex automatically sets it for GCC15 and above. Useful to keep it right after for an eventual coccinelle script... > > -Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-31 0:18 ` Rosen Penev @ 2026-03-31 0:51 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2026-03-31 0:51 UTC (permalink / raw) To: Rosen Penev Cc: Alex Williamson, kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, Leon Romanovsky >>>>> 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. >> >> Not true. However, it's best practice. >> >> The requirement is that the _counter_ must be initialized before >> the first reference to the flexible-array member. > OTOH kzalloc_flex automatically sets it for GCC15 and above. Useful to That's what the "it's best practice." comment above alludes to. -Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-30 23:24 ` Rosen Penev 2026-03-30 23:46 ` Gustavo A. R. Silva @ 2026-03-31 0:23 ` Alex Williamson 2026-03-31 5:11 ` Leon Romanovsky 1 sibling, 1 reply; 8+ messages in thread From: Alex Williamson @ 2026-03-31 0:23 UTC (permalink / raw) To: Rosen Penev Cc: kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b, Leon Romanovsky On Mon, Mar 30, 2026, at 5:24 PM, Rosen Penev wrote: > On Mon, Mar 30, 2026 at 4:16 PM Alex Williamson <alex@shazbot.org> wrote: >> >> [Cc +Leon] >> >> On Wed, 25 Mar 2026 19:37:47 -0700 >> Rosen Penev <rosenp@gmail.com> 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 <rosenp@gmail.com> >> > --- >> > 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: >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio: pci: use kzalloc_flex 2026-03-31 0:23 ` Alex Williamson @ 2026-03-31 5:11 ` Leon Romanovsky 0 siblings, 0 replies; 8+ messages in thread From: Leon Romanovsky @ 2026-03-31 5:11 UTC (permalink / raw) To: Alex Williamson Cc: Rosen Penev, kvm, Kees Cook, Gustavo A. R. Silva, open list, open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b 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@shazbot.org> wrote: > >> > >> [Cc +Leon] > >> > >> On Wed, 25 Mar 2026 19:37:47 -0700 > >> Rosen Penev <rosenp@gmail.com> 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@gmail.com> > >> > --- > >> > 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: > >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-03-31 5:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-26 2:37 [PATCH] vfio: pci: use kzalloc_flex Rosen Penev 2026-03-30 22:19 ` Alex Williamson 2026-03-30 23:24 ` Rosen Penev 2026-03-30 23:46 ` Gustavo A. R. Silva 2026-03-31 0:18 ` Rosen Penev 2026-03-31 0:51 ` Gustavo A. R. Silva 2026-03-31 0:23 ` Alex Williamson 2026-03-31 5:11 ` Leon Romanovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox