[PATCH] iommu: move the adding group info to proper place

[PATCH] iommu: move the adding group info to proper place

[Li RongQing]

After commit fa0828036488 ("iommu: Split iommu_group_add_device()"), the
adding group information is left in iommu_group_alloc_device, but which
only allocate group device, not attach the device to iommu group;

so move the print to iommu probing and adding functions

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/iommu/iommu.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 599030e..03f95f2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -586,6 +586,9 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 
 	mutex_unlock(&group->mutex);
 
+	trace_add_device_to_group(group->id, dev);
+	dev_info(dev, "Adding to iommu group %d\n", group->id);
+
 	return 0;
 
 err_remove_gdev:
@@ -594,6 +597,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 err_put_group:
 	iommu_deinit_device(dev);
 	mutex_unlock(&group->mutex);
+	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
 	iommu_group_put(group);
 
 	return ret;
@@ -1192,10 +1196,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
 		goto err_free_name;
 	}
 
-	trace_add_device_to_group(group->id, dev);
-
-	dev_info(dev, "Adding to iommu group %d\n", group->id);
-
 	return device;
 
 err_free_name:
@@ -1204,7 +1204,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
 	sysfs_remove_link(&dev->kobj, "iommu_group");
 err_free_device:
 	kfree(device);
-	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
 	return ERR_PTR(ret);
 }
 
@@ -1230,6 +1229,10 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
 	mutex_lock(&group->mutex);
 	list_add_tail(&gdev->list, &group->devices);
 	mutex_unlock(&group->mutex);
+
+	trace_add_device_to_group(group->id, dev);
+	dev_info(dev, "Adding to iommu group %d\n", group->id);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_group_add_device);
-- 
2.9.4

Re: [PATCH] iommu: move the adding group info to proper place

[Baolu Lu]

On 1/3/25 13:28, Li RongQing wrote:
> After commit fa0828036488 ("iommu: Split iommu_group_add_device()"), the
> adding group information is left in iommu_group_alloc_device, but which
> only allocate group device, not attach the device to iommu group;
> 
> so move the print to iommu probing and adding functions
> 
> Signed-off-by: Li RongQing<lirongqing@baidu.com>
> ---
>   drivers/iommu/iommu.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Re: [PATCH] iommu: move the adding group info to proper place

[Vasant Hegde]

On 1/3/2025 10:58 AM, Li RongQing wrote:
> After commit fa0828036488 ("iommu: Split iommu_group_add_device()"), the
> adding group information is left in iommu_group_alloc_device, but which
> only allocate group device, not attach the device to iommu group;
> 
> so move the print to iommu probing and adding functions
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  drivers/iommu/iommu.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 599030e..03f95f2 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -586,6 +586,9 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  
>  	mutex_unlock(&group->mutex);
>  
> +	trace_add_device_to_group(group->id, dev);
> +	dev_info(dev, "Adding to iommu group %d\n", group->id);
> +
>  	return 0;
>  
>  err_remove_gdev:
> @@ -594,6 +597,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  err_put_group:
>  	iommu_deinit_device(dev);
>  	mutex_unlock(&group->mutex);
> +	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
>  	iommu_group_put(group);
>  
>  	return ret;
> @@ -1192,10 +1196,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
>  		goto err_free_name;
>  	}
>  
> -	trace_add_device_to_group(group->id, dev);
> -
> -	dev_info(dev, "Adding to iommu group %d\n", group->id);
> -
>  	return device;
>  
>  err_free_name:
> @@ -1204,7 +1204,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
>  	sysfs_remove_link(&dev->kobj, "iommu_group");
>  err_free_device:
>  	kfree(device);
> -	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -1230,6 +1229,10 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)

Add error message if iommu_group_alloc_device() fails here?

-Vasant

答复: [外部邮件] Re: [PATCH] iommu: move the adding group info to proper place

[Li,Rongqing]

> 
> On 1/3/2025 10:58 AM, Li RongQing wrote:
> > After commit fa0828036488 ("iommu: Split iommu_group_add_device()"),
> > the adding group information is left in iommu_group_alloc_device, but
> >
> > @@ -1230,6 +1229,10 @@ int iommu_group_add_device(struct
> iommu_group
> > *group, struct device *dev)
> 
> Add error message if iommu_group_alloc_device() fails here?
> 

Ok, I will add a error message here

Thanks

-Li

> -Vasant