On Thu, Jul 27, 2023 at 05:25:33AM +0000, Tian, Kevin wrote:
It has the downside that if userspace races destroy with other operations it will get an EBUSY instead of waiting, but this is kind of racing is already dangerous.
it's not a new downside. Even old code also returns -EBUSY if iommufd_object_destroy_user() returns false.
It sort of is, previously you could race ioctls and destroy would wait for the ioctl to finish, now it returns -EBUSY
/*
- The caller holds a users refcount and wants to destroy the object. Returns
s/users/user's/
'users' is the name of the variable
- /*
* If there is a bug and we couldn't destroy the object then we did put* back the callers refcount and will eventually try to free it agains/callers/caller's/
Yep
Reviewed-by: Kevin Tian kevin.tian@intel.com
btw,
- iommufd_ref_to_users(obj);
- /* See iommufd_ref_to_users() */
- if (!iommufd_object_destroy_user(ucmd->ictx, obj))
return -EBUSY;I wonder whether there is any other reason to keep iommufd_ref_to_users(). Now the only invocation is in iommufd_access_attach(), but it could be simply replaced by "get_object(); refcount_inc(); put_object()" as all other places are doing.
Hmm, yes, the replace series could probably be tweaked to comfortably do this.
Jason