Re: [PATCH V2 02/11] iommufd: Move igroup allocation to a function

Samiullah Khawaja
2026-03-23T16:46:38Z

On Thu, Mar 12, 2026 at 08:56:28AM -0700, Jacob Pan wrote:
>From: Jason Gunthorpe <jgg@nvidia.com>
>
>So it can be reused in the next patch which allows binding to noiommu
>device.
>
>Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
>---
> drivers/iommu/iommufd/device.c | 48 +++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
>index 344d620cdecc..54d73016468f 100644
>--- a/drivers/iommu/iommufd/device.c
>+++ b/drivers/iommu/iommufd/device.c
>@@ -30,8 +30,9 @@ static void iommufd_group_release(struct kref *kref)
>
> WARN_ON(!xa_empty(&igroup->pasid_attach));
>
>- xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
>- NULL, GFP_KERNEL);
>+ if (igroup->group)
>+ xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group),
>+ igroup, NULL, GFP_KERNEL);
> iommu_group_put(igroup->group);
> mutex_destroy(&igroup->lock);
> kfree(igroup);
>@@ -56,6 +57,30 @@ static bool iommufd_group_try_get(struct iommufd_group *igroup,
> return kref_get_unless_zero(&igroup->ref);
> }
>
>+static struct iommufd_group *iommufd_alloc_group(struct iommufd_ctx *ictx,
>+ struct iommu_group *group)
>+{
>+ struct iommufd_group *new_igroup;
>+
>+ new_igroup = kzalloc(sizeof(*new_igroup), GFP_KERNEL);
>+ if (!new_igroup)
>+ return ERR_PTR(-ENOMEM);
>+
>+ kref_init(&new_igroup->ref);
>+ mutex_init(&new_igroup->lock);
>+ xa_init(&new_igroup->pasid_attach);
>+ new_igroup->sw_msi_start = PHYS_ADDR_MAX;
>+ /* group reference moves into new_igroup */
>+ new_igroup->group = group;
>+
>+ /*
>+ * The ictx is not additionally refcounted here becase all objects using
>+ * an igroup must put it before their destroy completes.
>+ */
>+ new_igroup->ictx = ictx;
>+ return new_igroup;
>+}
>+
> /*
> * iommufd needs to store some more data for each iommu_group, we keep a
> * parallel xarray indexed by iommu_group id to hold this instead of putting it
>@@ -87,25 +112,12 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
> }
> xa_unlock(&ictx->groups);
>
>- new_igroup = kzalloc_obj(*new_igroup);
>- if (!new_igroup) {
>+ new_igroup = iommufd_alloc_group(ictx, group);
>+ if (IS_ERR(new_igroup)) {
> iommu_group_put(group);
>- return ERR_PTR(-ENOMEM);
>+ return new_igroup;
> }
>
>- kref_init(&new_igroup->ref);
>- mutex_init(&new_igroup->lock);
>- xa_init(&new_igroup->pasid_attach);
>- new_igroup->sw_msi_start = PHYS_ADDR_MAX;
>- /* group reference moves into new_igroup */
>- new_igroup->group = group;
>-
>- /*
>- * The ictx is not additionally refcounted here becase all objects using
>- * an igroup must put it before their destroy completes.
>- */
>- new_igroup->ictx = ictx;
>-
> /*
> * We dropped the lock so igroup is invalid. NULL is a safe and likely
> * value to assume for the xa_cmpxchg algorithm.
>--
>2.34.1
>

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>