[PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects

Xu Yilun posted 7 patches 3 months ago
There is a newer version of this series
[PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Xu Yilun 3 months ago
Add a pre_destroy() op which gives objects a chance to clear their
short term users references before destruction. This op is intended for
external driver created objects (e.g. idev) which does deterministic
destruction.

In order to manage the lifecycle of interrelated objects as well as the
deterministic destruction (e.g. vdev can't outlive idev, and idev
destruction can't fail), short term users references are allowed to
live out of an ioctl execution. An immediate use case is, vdev holds
idev's short term user reference until vdev destruction completes, idev
leverages existing wait_shortterm mechanism to ensure it is destroyed
after vdev.

This extended usage makes the referenced object unable to just wait for
its reference gone. It needs to actively trigger the reference removal,
as well as prevent new references before wait. Should implement these
work in pre_destroy().

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 drivers/iommu/iommufd/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index 25ab2f41d650..e91a36cc02d0 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -23,6 +23,7 @@
 #include "iommufd_test.h"
 
 struct iommufd_object_ops {
+	void (*pre_destroy)(struct iommufd_object *obj);
 	void (*destroy)(struct iommufd_object *obj);
 	void (*abort)(struct iommufd_object *obj);
 };
@@ -151,6 +152,9 @@ static int iommufd_object_dec_wait_shortterm(struct iommufd_ctx *ictx,
 	if (refcount_dec_and_test(&to_destroy->shortterm_users))
 		return 0;
 
+	if (iommufd_object_ops[to_destroy->type].pre_destroy)
+		iommufd_object_ops[to_destroy->type].pre_destroy(to_destroy);
+
 	if (wait_event_timeout(ictx->destroy_wait,
 			       refcount_read(&to_destroy->shortterm_users) == 0,
 			       msecs_to_jiffies(60000)))
-- 
2.25.1
Re: [PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Nicolin Chen 2 months, 3 weeks ago
On Wed, Jul 09, 2025 at 12:02:30PM +0800, Xu Yilun wrote:
> Add a pre_destroy() op which gives objects a chance to clear their
> short term users references before destruction. This op is intended for
> external driver created objects (e.g. idev) which does deterministic
> destruction.
> 
> In order to manage the lifecycle of interrelated objects as well as the
> deterministic destruction (e.g. vdev can't outlive idev, and idev
> destruction can't fail), short term users references are allowed to
> live out of an ioctl execution. An immediate use case is, vdev holds
> idev's short term user reference until vdev destruction completes, idev
> leverages existing wait_shortterm mechanism to ensure it is destroyed
> after vdev.
> 
> This extended usage makes the referenced object unable to just wait for
> its reference gone. It needs to actively trigger the reference removal,
> as well as prevent new references before wait. Should implement these
> work in pre_destroy().
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
RE: [PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Tian, Kevin 2 months, 4 weeks ago
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, July 9, 2025 12:03 PM
> 
> Add a pre_destroy() op which gives objects a chance to clear their
> short term users references before destruction. This op is intended for
> external driver created objects (e.g. idev) which does deterministic
> destruction.
> 
> In order to manage the lifecycle of interrelated objects as well as the
> deterministic destruction (e.g. vdev can't outlive idev, and idev
> destruction can't fail), short term users references are allowed to
> live out of an ioctl execution. An immediate use case is, vdev holds
> idev's short term user reference until vdev destruction completes, idev
> leverages existing wait_shortterm mechanism to ensure it is destroyed
> after vdev.
> 
> This extended usage makes the referenced object unable to just wait for
> its reference gone. It needs to actively trigger the reference removal,
> as well as prevent new references before wait. Should implement these
> work in pre_destroy().
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

btw is it clearer to rename 'shortterm_users' as 'wait_cnt', as the
meaning now is beyond shortterm and is really about the need of
waiting?
Re: [PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Jason Gunthorpe 2 months, 4 weeks ago
On Thu, Jul 10, 2025 at 07:40:58AM +0000, Tian, Kevin wrote:
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Wednesday, July 9, 2025 12:03 PM
> > 
> > Add a pre_destroy() op which gives objects a chance to clear their
> > short term users references before destruction. This op is intended for
> > external driver created objects (e.g. idev) which does deterministic
> > destruction.
> > 
> > In order to manage the lifecycle of interrelated objects as well as the
> > deterministic destruction (e.g. vdev can't outlive idev, and idev
> > destruction can't fail), short term users references are allowed to
> > live out of an ioctl execution. An immediate use case is, vdev holds
> > idev's short term user reference until vdev destruction completes, idev
> > leverages existing wait_shortterm mechanism to ensure it is destroyed
> > after vdev.
> > 
> > This extended usage makes the referenced object unable to just wait for
> > its reference gone. It needs to actively trigger the reference removal,
> > as well as prevent new references before wait. Should implement these
> > work in pre_destroy().
> > 
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> btw is it clearer to rename 'shortterm_users' as 'wait_cnt', as the
> meaning now is beyond shortterm and is really about the need of
> waiting?

Probably so, as a followup change would be fine if we don't need a v5

/*
 * Destroy will sleep and wait for wait_cnt to go to zero. This
 * allows concurrent users of the ID to reliably avoid causing a
 * spurious destroy failure. Incrementing this count should either be
 * short lived or be revoked and blocked during pre_destroy
 */
refcount_t wait_cnt;

?

Jason
RE: [PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Tian, Kevin 2 months, 4 weeks ago
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, July 11, 2025 1:15 AM
> 
> On Thu, Jul 10, 2025 at 07:40:58AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Wednesday, July 9, 2025 12:03 PM
> > >
> > > Add a pre_destroy() op which gives objects a chance to clear their
> > > short term users references before destruction. This op is intended for
> > > external driver created objects (e.g. idev) which does deterministic
> > > destruction.
> > >
> > > In order to manage the lifecycle of interrelated objects as well as the
> > > deterministic destruction (e.g. vdev can't outlive idev, and idev
> > > destruction can't fail), short term users references are allowed to
> > > live out of an ioctl execution. An immediate use case is, vdev holds
> > > idev's short term user reference until vdev destruction completes, idev
> > > leverages existing wait_shortterm mechanism to ensure it is destroyed
> > > after vdev.
> > >
> > > This extended usage makes the referenced object unable to just wait for
> > > its reference gone. It needs to actively trigger the reference removal,
> > > as well as prevent new references before wait. Should implement these
> > > work in pre_destroy().
> > >
> > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> >
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> >
> > btw is it clearer to rename 'shortterm_users' as 'wait_cnt', as the
> > meaning now is beyond shortterm and is really about the need of
> > waiting?
> 
> Probably so, as a followup change would be fine if we don't need a v5
> 
> /*
>  * Destroy will sleep and wait for wait_cnt to go to zero. This
>  * allows concurrent users of the ID to reliably avoid causing a
>  * spurious destroy failure. Incrementing this count should either be
>  * short lived or be revoked and blocked during pre_destroy
>  */
> refcount_t wait_cnt;
> 
> ?

Looks good
Re: [PATCH v4 3/7] iommufd: Add a pre_destroy() op for objects
Posted by Xu Yilun 2 months, 4 weeks ago
On Fri, Jul 11, 2025 at 03:16:38AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Friday, July 11, 2025 1:15 AM
> > 
> > On Thu, Jul 10, 2025 at 07:40:58AM +0000, Tian, Kevin wrote:
> > > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > > Sent: Wednesday, July 9, 2025 12:03 PM
> > > >
> > > > Add a pre_destroy() op which gives objects a chance to clear their
> > > > short term users references before destruction. This op is intended for
> > > > external driver created objects (e.g. idev) which does deterministic
> > > > destruction.
> > > >
> > > > In order to manage the lifecycle of interrelated objects as well as the
> > > > deterministic destruction (e.g. vdev can't outlive idev, and idev
> > > > destruction can't fail), short term users references are allowed to
> > > > live out of an ioctl execution. An immediate use case is, vdev holds
> > > > idev's short term user reference until vdev destruction completes, idev
> > > > leverages existing wait_shortterm mechanism to ensure it is destroyed
> > > > after vdev.
> > > >
> > > > This extended usage makes the referenced object unable to just wait for
> > > > its reference gone. It needs to actively trigger the reference removal,
> > > > as well as prevent new references before wait. Should implement these
> > > > work in pre_destroy().
> > > >
> > > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > > > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > >
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > btw is it clearer to rename 'shortterm_users' as 'wait_cnt', as the
> > > meaning now is beyond shortterm and is really about the need of
> > > waiting?
> > 
> > Probably so, as a followup change would be fine if we don't need a v5

We have some comments to quick fix. Let me make a v5 and include this
change.

> > 
> > /*
> >  * Destroy will sleep and wait for wait_cnt to go to zero. This
> >  * allows concurrent users of the ID to reliably avoid causing a
> >  * spurious destroy failure. Incrementing this count should either be
> >  * short lived or be revoked and blocked during pre_destroy
> >  */
> > refcount_t wait_cnt;
> > 
> > ?
> 
> Looks good