On Mon, Jun 16, 2025 at 07:25:57PM -0700, Nicolin Chen wrote:
On Mon, Jun 16, 2025 at 10:37:19AM -0300, Jason Gunthorpe wrote:
On Sat, Jun 14, 2025 at 12:14:32AM -0700, Nicolin Chen wrote:
Now, access->ops can be NULL, to support an internal use case for the new HW queue object. Since an access object in this case will be allocated by an inernal iommufd object, the refcount on the ictx should be skipped, so as not to deadlock the release of the ictx as it would otherwise wait for the release of the access first during the release of the internal object that could wait for the release of ictx: ictx --releases--> hw_queue --releases--> access ^ | |_________________releases________________v
Add a set of lightweight internal APIs to unlink access and ictx: ictx --releases--> hw_queue --releases--> access
Signed-off-by: Nicolin Chen nicolinc@nvidia.com
drivers/iommu/iommufd/iommufd_private.h | 8 ++++ drivers/iommu/iommufd/device.c | 59 +++++++++++++++++++++---- 2 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 4a375a8c9216..468717d5e5bc 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -484,6 +484,14 @@ void iopt_remove_access(struct io_pagetable *iopt, struct iommufd_access *access, u32 iopt_access_list_id); void iommufd_access_destroy_object(struct iommufd_object *obj); +/* iommufd_access for internal use */ +struct iommufd_access *iommufd_access_create_internal(struct iommufd_ctx *ictx); +#define iommufd_access_destroy_internal(ictx, access) \
- iommufd_object_destroy_user(ictx, &(access)->obj)
Use a static inline please
+int iommufd_access_attach_internal(struct iommufd_access *access,
struct iommufd_ioas *ioas);+#define iommufd_access_detach_internal(access) iommufd_access_detach(access)
struct iommufd_eventq { struct iommufd_object obj; struct iommufd_ctx *ictx; diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 9293722b9cff..ad33f1e41a24 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -1084,7 +1084,39 @@ void iommufd_access_destroy_object(struct iommufd_object *obj) if (access->ioas) WARN_ON(iommufd_access_change_ioas(access, NULL)); mutex_unlock(&access->ioas_lock);
- iommufd_ctx_put(access->ictx);
- if (access->ops)
iommufd_ctx_put(access->ictx);I was hoping we could null the ictx to signal internal? That didn't work out?
access->ictx should be NULL for internal. It should have been:
- if (access->ictx)
iommufd_ctx_put(access->ictx);
Ohh sorry, just saw this. +1, I too believe this is better than relying on access->ops being NULL.
I would at least add a comment here this is filtering internal that doesn't have ictx. Maybe a little inline 'iommufd_access_is_internal' is appropriate. We'll be sad down the road if we need ops for internal.
Yea, an inline will be cleaner. Will add that.
Ack.
Thanks, Praan