On Wed, Mar 15, 2023 at 06:16:37AM +0000, Tian, Kevin wrote:
> External email: Use caution opening links or attachments
>
>
> > From: Nicolin Chen
> > Sent: Wednesday, March 15, 2023 2:51 AM
> >
> > On Fri, Mar 10, 2023 at 02:08:15AM +0000, Tian, Kevin wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > > Sent: Wednesday, March 8, 2023 9:14 PM
> > > >
> > > > @@ -449,33 +450,18 @@ iommufd_access_create(struct iommufd_ctx
> > *ictx,
> > > > u32 ioas_id,
> > > > access->data = data;
> > > > access->ops = ops;
> > > >
> > > > - obj = iommufd_get_object(ictx, ioas_id, IOMMUFD_OBJ_IOAS);
> > > > - if (IS_ERR(obj)) {
> > > > - rc = PTR_ERR(obj);
> > > > - goto out_abort;
> > > > - }
> > > > - access->ioas = container_of(obj, struct iommufd_ioas, obj);
> > > > - iommufd_ref_to_users(obj);
> > > > -
> > > > if (ops->needs_pin_pages)
> > > > access->iova_alignment = PAGE_SIZE;
> > > > else
> > > > access->iova_alignment = 1;
> > > > - rc = iopt_add_access(&access->ioas->iopt, access);
> > > > - if (rc)
> > > > - goto out_put_ioas;
> > > >
> > > > /* The calling driver is a user until iommufd_access_destroy() */
> > > > refcount_inc(&access->obj.users);
> > > > + mutex_init(&access->ioas_lock);
> > > > access->ictx = ictx;
> > > > iommufd_ctx_get(ictx);
> > >
> > > this refcnt get should be moved to the start given next patch
> > > removes the reference in the caller side.
> >
> > I don't feel quite convincing to justify for moving it in this
> > patch. Perhaps we should do that in the following patch, where
> > it needs this? Or another individual patch moving this alone?
> >
>
> Next patch. I just tried to point out the required change caused
> by next patch. 😊
OK. I made a small individual patch. Posting here so Yi can just
squash it into the next patch:
>From dbfe7457d35ea9a4da9c8e6daa838b101bc8f621 Mon Sep 17 00:00:00 2001
From: Nicolin Chen <nicolinc@xxxxxxxxxx>
Date: Tue, 14 Mar 2023 12:51:07 -0700
Subject: [PATCH] iommufd/device: Do iommufd_ctx_get() at the top of
iommufd_access_create()
The following patch will remove the iommufd_ctx_get() call prior to the
iommufd_access_create() call in vfio_iommufd_emulated_bind(), expecting
iommufd_access_create() call covers the iommufd_ctx_get(). However, the
iommufd_access_create() only does iommufd_ctx_get() at the end. Thus,
move the iommufd_ctx_get() call to the top of iommufd_access_create().
Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
---
drivers/iommu/iommufd/device.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 0132803449be..dc1015b02a53 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -649,13 +649,16 @@ iommufd_access_create(struct iommufd_ctx *ictx,
{
struct iommufd_access *access;
+ iommufd_ctx_get(ictx);
/*
* There is no uAPI for the access object, but to keep things symmetric
* use the object infrastructure anyhow.
*/
access = iommufd_object_alloc(ictx, access, IOMMUFD_OBJ_ACCESS);
- if (IS_ERR(access))
+ if (IS_ERR(access)) {
+ iommufd_ctx_put(ictx);
return access;
+ }
access->data = data;
access->ops = ops;
@@ -668,7 +671,6 @@ iommufd_access_create(struct iommufd_ctx *ictx,
/* The calling driver is a user until iommufd_access_destroy() */
refcount_inc(&access->obj.users);
access->ictx = ictx;
- iommufd_ctx_get(ictx);
iommufd_object_finalize(ictx, &access->obj);
return access;
}
--
2.39.2
