From df15d76dcacac2126a4d20ba06c9d7e4b18bad8e Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 5 Jun 2023 21:59:42 -0300 Subject: iommu: Simplify the __iommu_group_remove_device() flow Instead of returning the struct group_device and then later freeing it, do the entire free under the group->mutex and defer only putting the iommu_group. It is safe to remove the sysfs_links and free memory while holding that mutex. Move the sanity assert of the group status into __iommu_group_free_device(). The next patch will improve upon this and consolidate the group put and the mutex into __iommu_group_remove_device(). __iommu_group_free_device() is close to being the paired undo of iommu_group_add_device(), following patches will improve on that. Reviewed-by: Kevin Tian Reviewed-by: Lu Baolu Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/4-v3-328044aa278c+45e49-iommu_probe_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 83 ++++++++++++++++++++++++--------------------------- 1 file changed, 39 insertions(+), 44 deletions(-) (limited to 'drivers/iommu') diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index bd56cf02bc99..6cf4da060e94 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -470,32 +470,8 @@ err_out: } -/* - * Remove a device from a group's device list and return the group device - * if successful. - */ -static struct group_device * -__iommu_group_remove_device(struct iommu_group *group, struct device *dev) -{ - struct group_device *device; - - lockdep_assert_held(&group->mutex); - for_each_group_device(group, device) { - if (device->dev == dev) { - list_del(&device->list); - return device; - } - } - - return NULL; -} - -/* - * Release a device from its group and decrements the iommu group reference - * count. - */ -static void __iommu_group_release_device(struct iommu_group *group, - struct group_device *grp_dev) +static void __iommu_group_free_device(struct iommu_group *group, + struct group_device *grp_dev) { struct device *dev = grp_dev->dev; @@ -504,16 +480,45 @@ static void __iommu_group_release_device(struct iommu_group *group, trace_remove_device_from_group(group->id, dev); + /* + * If the group has become empty then ownership must have been + * released, and the current domain must be set back to NULL or + * the default domain. + */ + if (list_empty(&group->devices)) + WARN_ON(group->owner_cnt || + group->domain != group->default_domain); + kfree(grp_dev->name); kfree(grp_dev); dev->iommu_group = NULL; - iommu_group_put(group); } -static void iommu_release_device(struct device *dev) +/* + * Remove the iommu_group from the struct device. The attached group must be put + * by the caller after releaseing the group->mutex. + */ +static void __iommu_group_remove_device(struct device *dev) { struct iommu_group *group = dev->iommu_group; struct group_device *device; + + lockdep_assert_held(&group->mutex); + for_each_group_device(group, device) { + if (device->dev != dev) + continue; + + list_del(&device->list); + __iommu_group_free_device(group, device); + /* Caller must put iommu_group */ + return; + } + WARN(true, "Corrupted iommu_group device_list"); +} + +static void iommu_release_device(struct device *dev) +{ + struct iommu_group *group = dev->iommu_group; const struct iommu_ops *ops; if (!dev->iommu || !group) @@ -522,16 +527,7 @@ static void iommu_release_device(struct device *dev) iommu_device_unlink(dev->iommu->iommu_dev, dev); mutex_lock(&group->mutex); - device = __iommu_group_remove_device(group, dev); - - /* - * If the group has become empty then ownership must have been released, - * and the current domain must be set back to NULL or the default - * domain. - */ - if (list_empty(&group->devices)) - WARN_ON(group->owner_cnt || - group->domain != group->default_domain); + __iommu_group_remove_device(dev); /* * release_device() must stop using any attached domain on the device. @@ -547,8 +543,8 @@ static void iommu_release_device(struct device *dev) ops->release_device(dev); mutex_unlock(&group->mutex); - if (device) - __iommu_group_release_device(group, device); + /* Pairs with the get in iommu_group_add_device() */ + iommu_group_put(group); module_put(ops->owner); dev_iommu_free(dev); @@ -1107,7 +1103,6 @@ EXPORT_SYMBOL_GPL(iommu_group_add_device); void iommu_group_remove_device(struct device *dev) { struct iommu_group *group = dev->iommu_group; - struct group_device *device; if (!group) return; @@ -1115,11 +1110,11 @@ void iommu_group_remove_device(struct device *dev) dev_info(dev, "Removing from iommu group %d\n", group->id); mutex_lock(&group->mutex); - device = __iommu_group_remove_device(group, dev); + __iommu_group_remove_device(dev); mutex_unlock(&group->mutex); - if (device) - __iommu_group_release_device(group, device); + /* Pairs with the get in iommu_group_add_device() */ + iommu_group_put(group); } EXPORT_SYMBOL_GPL(iommu_group_remove_device); -- cgit v1.2.3