Re: [PATCH v1 11/12] iommufd: Introduce iommufd_object_alloc_ucmd helper

[Nicolin Chen <nicolinc@nvidia.com>]

On Fri, Jun 13, 2025 at 10:58:17AM -0300, Jason Gunthorpe wrote:
> On Mon, Jun 09, 2025 at 10:13:34AM -0700, Nicolin Chen wrote:
> 
> > +#define iommufd_object_alloc_ucmd(ucmd, ptr, type) \
> > +	__iommufd_object_alloc_ucmd(ucmd, ptr, type, obj)
> 
> Lets add a comment here and on the normal iommufd_object_alloc
> explaining that this function automatically calls finalize and abort,
> the non ucmd version requires the caller to do so.

Yes.

@@ -231,6 +231,11 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx,
        iommufd_object_remove(ictx, obj, obj->id, 0);
 }

+/*
+ * Callers of these normal object allocators must call iommufd_object_finalize()
+ * to finalize the object, or call iommufd_object_abort_and_destroy() to revert
+ * the allocation.
+ */
 struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
                                             size_t size,
                                             enum iommufd_object_type type);
@@ -247,6 +252,10 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
 #define iommufd_object_alloc(ictx, ptr, type) \
        __iommufd_object_alloc(ictx, ptr, type, obj)

+/*
+ * Callers of these _ucmd allocators should not call iommufd_object_finalize()
+ * or iommufd_object_abort_and_destroy(), as the core automatically does that.
+ */
 struct iommufd_object *
 _iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd, size_t size,
                           enum iommufd_object_type type);

> > +struct iommufd_object *_iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd,
> > +						  size_t size,
> > +						  enum iommufd_object_type type)
> > +{
> > +	struct iommufd_object *new_obj;
> > +
> > +	if (ucmd->new_obj)
> 
> WARN_ON? Something is coded wrong if we hit this right?

Yes.

@@ -67,7 +67,8 @@ struct iommufd_object *_iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd,
 {
        struct iommufd_object *new_obj;

-       if (ucmd->new_obj)
+       /* Something is coded wrong if this is hit */
+       if (WARN_ON(ucmd->new_obj))
                return ERR_PTR(-EBUSY);

        new_obj = _iommufd_object_alloc(ucmd->ictx, size, type);

Thanks
Nicolin