All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch V2 0/2] Cleanup on IOMMU
@ 2016-03-23 22:25 ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding, alex.williamson; +Cc: joro, iommu, linux-kernel, Wei Yang

This two patches tries to do some cleanup in iommu.

V2:
  * add patch 2

Wei Yang (2):
  iommu: remove the iommu_callback_data
  iommu: remove sysfs_link to device in iommu_group/devices when failed

 drivers/iommu/iommu.c |   22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Patch V2 0/2] Cleanup on IOMMU
@ 2016-03-23 22:25 ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding-DDmLM1+adcrQT0dZR+AlfA, alex.williamson-H+wXaHxf7aLQT0dZR+AlfA
  Cc: Wei Yang, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

This two patches tries to do some cleanup in iommu.

V2:
  * add patch 2

Wei Yang (2):
  iommu: remove the iommu_callback_data
  iommu: remove sysfs_link to device in iommu_group/devices when failed

 drivers/iommu/iommu.c |   22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Patch V2 1/2] iommu: remove the iommu_callback_data
@ 2016-03-23 22:25   ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding, alex.williamson; +Cc: joro, iommu, linux-kernel, Wei Yang

According to the code path, iommu_callback_data is passed in
iommu_bus_init() and  just used in {add/remove}_iommu_group, by when the
bus->iommu_ops is already set up properly.

This patch removes the iommu_callback_data by retrieving iommu_ops from
bus->iommu_ops directly.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 drivers/iommu/iommu.c |   21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..2696a38 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -37,10 +37,6 @@ static struct kset *iommu_group_kset;
 static struct ida iommu_group_ida;
 static struct mutex iommu_group_mutex;
 
-struct iommu_callback_data {
-	const struct iommu_ops *ops;
-};
-
 struct iommu_group {
 	struct kobject kobj;
 	struct kobject *devices_kobj;
@@ -867,8 +863,7 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
 
 static int add_iommu_group(struct device *dev, void *data)
 {
-	struct iommu_callback_data *cb = data;
-	const struct iommu_ops *ops = cb->ops;
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret;
 
 	if (!ops->add_device)
@@ -891,8 +886,7 @@ static int add_iommu_group(struct device *dev, void *data)
 
 static int remove_iommu_group(struct device *dev, void *data)
 {
-	struct iommu_callback_data *cb = data;
-	const struct iommu_ops *ops = cb->ops;
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 
 	if (ops->remove_device && dev->iommu_group)
 		ops->remove_device(dev);
@@ -953,13 +947,10 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	return 0;
 }
 
-static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
+static int iommu_bus_init(struct bus_type *bus)
 {
 	int err;
 	struct notifier_block *nb;
-	struct iommu_callback_data cb = {
-		.ops = ops,
-	};
 
 	nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
 	if (!nb)
@@ -971,7 +962,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
 	if (err)
 		goto out_free;
 
-	err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
+	err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
 	if (err)
 		goto out_err;
 
@@ -980,7 +971,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
 
 out_err:
 	/* Clean up */
-	bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
+	bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
 	bus_unregister_notifier(bus, nb);
 
 out_free:
@@ -1012,7 +1003,7 @@ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops)
 	bus->iommu_ops = ops;
 
 	/* Do IOMMU specific setup for this bus-type */
-	err = iommu_bus_init(bus, ops);
+	err = iommu_bus_init(bus);
 	if (err)
 		bus->iommu_ops = NULL;
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Patch V2 1/2] iommu: remove the iommu_callback_data
@ 2016-03-23 22:25   ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding-DDmLM1+adcrQT0dZR+AlfA, alex.williamson-H+wXaHxf7aLQT0dZR+AlfA
  Cc: Wei Yang, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

According to the code path, iommu_callback_data is passed in
iommu_bus_init() and  just used in {add/remove}_iommu_group, by when the
bus->iommu_ops is already set up properly.

This patch removes the iommu_callback_data by retrieving iommu_ops from
bus->iommu_ops directly.

Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/iommu.c |   21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..2696a38 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -37,10 +37,6 @@ static struct kset *iommu_group_kset;
 static struct ida iommu_group_ida;
 static struct mutex iommu_group_mutex;
 
-struct iommu_callback_data {
-	const struct iommu_ops *ops;
-};
-
 struct iommu_group {
 	struct kobject kobj;
 	struct kobject *devices_kobj;
@@ -867,8 +863,7 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
 
 static int add_iommu_group(struct device *dev, void *data)
 {
-	struct iommu_callback_data *cb = data;
-	const struct iommu_ops *ops = cb->ops;
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret;
 
 	if (!ops->add_device)
@@ -891,8 +886,7 @@ static int add_iommu_group(struct device *dev, void *data)
 
 static int remove_iommu_group(struct device *dev, void *data)
 {
-	struct iommu_callback_data *cb = data;
-	const struct iommu_ops *ops = cb->ops;
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 
 	if (ops->remove_device && dev->iommu_group)
 		ops->remove_device(dev);
@@ -953,13 +947,10 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	return 0;
 }
 
-static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
+static int iommu_bus_init(struct bus_type *bus)
 {
 	int err;
 	struct notifier_block *nb;
-	struct iommu_callback_data cb = {
-		.ops = ops,
-	};
 
 	nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
 	if (!nb)
@@ -971,7 +962,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
 	if (err)
 		goto out_free;
 
-	err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
+	err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
 	if (err)
 		goto out_err;
 
@@ -980,7 +971,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
 
 out_err:
 	/* Clean up */
-	bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
+	bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
 	bus_unregister_notifier(bus, nb);
 
 out_free:
@@ -1012,7 +1003,7 @@ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops)
 	bus->iommu_ops = ops;
 
 	/* Do IOMMU specific setup for this bus-type */
-	err = iommu_bus_init(bus, ops);
+	err = iommu_bus_init(bus);
 	if (err)
 		bus->iommu_ops = NULL;
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-23 22:25   ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding, alex.williamson; +Cc: joro, iommu, linux-kernel, Wei Yang

The original code forgets to remove the sysfs_link to a device in
iommu_group/devices directory, when the creation fails or conflicts on the
name.

This patch tries to remove the sysfs_link on the failure.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 drivers/iommu/iommu.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2696a38..8f480ba 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -403,6 +403,7 @@ rename:
 	ret = sysfs_create_link_nowarn(group->devices_kobj,
 				       &dev->kobj, device->name);
 	if (ret) {
+		sysfs_remove_link(group->devices_kobj, device->name);
 		kfree(device->name);
 		if (ret == -EEXIST && i >= 0) {
 			/*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-23 22:25   ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-23 22:25 UTC (permalink / raw)
  To: treding-DDmLM1+adcrQT0dZR+AlfA, alex.williamson-H+wXaHxf7aLQT0dZR+AlfA
  Cc: Wei Yang, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

The original code forgets to remove the sysfs_link to a device in
iommu_group/devices directory, when the creation fails or conflicts on the
name.

This patch tries to remove the sysfs_link on the failure.

Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/iommu.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2696a38..8f480ba 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -403,6 +403,7 @@ rename:
 	ret = sysfs_create_link_nowarn(group->devices_kobj,
 				       &dev->kobj, device->name);
 	if (ret) {
+		sysfs_remove_link(group->devices_kobj, device->name);
 		kfree(device->name);
 		if (ret == -EEXIST && i >= 0) {
 			/*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-23 23:04     ` Alex Williamson
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Williamson @ 2016-03-23 23:04 UTC (permalink / raw)
  To: Wei Yang; +Cc: treding, joro, iommu, linux-kernel

On Wed, 23 Mar 2016 22:25:11 +0000
Wei Yang <richard.weiyang@gmail.com> wrote:

> The original code forgets to remove the sysfs_link to a device in
> iommu_group/devices directory, when the creation fails or conflicts on the
> name.
> 
> This patch tries to remove the sysfs_link on the failure.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  drivers/iommu/iommu.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 2696a38..8f480ba 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -403,6 +403,7 @@ rename:
>  	ret = sysfs_create_link_nowarn(group->devices_kobj,
>  				       &dev->kobj, device->name);
>  	if (ret) {
> +		sysfs_remove_link(group->devices_kobj, device->name);
>  		kfree(device->name);
>  		if (ret == -EEXIST && i >= 0) {
>  			/*

If we failed to create a link, potentially due to a conflicting link
already present, then aren't we arbitrarily removing that conflicting
link with this change?  If sysfs_create_link_nowarn() fails then we
haven't created a link of our own to remove.  This looks wrong.  Thanks,

Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-23 23:04     ` Alex Williamson
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Williamson @ 2016-03-23 23:04 UTC (permalink / raw)
  To: Wei Yang
  Cc: treding-DDmLM1+adcrQT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Wed, 23 Mar 2016 22:25:11 +0000
Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> The original code forgets to remove the sysfs_link to a device in
> iommu_group/devices directory, when the creation fails or conflicts on the
> name.
> 
> This patch tries to remove the sysfs_link on the failure.
> 
> Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iommu/iommu.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 2696a38..8f480ba 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -403,6 +403,7 @@ rename:
>  	ret = sysfs_create_link_nowarn(group->devices_kobj,
>  				       &dev->kobj, device->name);
>  	if (ret) {
> +		sysfs_remove_link(group->devices_kobj, device->name);
>  		kfree(device->name);
>  		if (ret == -EEXIST && i >= 0) {
>  			/*

If we failed to create a link, potentially due to a conflicting link
already present, then aren't we arbitrarily removing that conflicting
link with this change?  If sysfs_create_link_nowarn() fails then we
haven't created a link of our own to remove.  This looks wrong.  Thanks,

Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-24 14:51       ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-24 14:51 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Wei Yang, treding, joro, iommu, linux-kernel

On Wed, Mar 23, 2016 at 05:04:15PM -0600, Alex Williamson wrote:
>On Wed, 23 Mar 2016 22:25:11 +0000
>Wei Yang <richard.weiyang@gmail.com> wrote:
>
>> The original code forgets to remove the sysfs_link to a device in
>> iommu_group/devices directory, when the creation fails or conflicts on the
>> name.
>> 
>> This patch tries to remove the sysfs_link on the failure.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>  drivers/iommu/iommu.c |    1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 2696a38..8f480ba 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -403,6 +403,7 @@ rename:
>>  	ret = sysfs_create_link_nowarn(group->devices_kobj,
>>  				       &dev->kobj, device->name);
>>  	if (ret) {
>> +		sysfs_remove_link(group->devices_kobj, device->name);
>>  		kfree(device->name);
>>  		if (ret == -EEXIST && i >= 0) {
>>  			/*
>
>If we failed to create a link, potentially due to a conflicting link
>already present, then aren't we arbitrarily removing that conflicting
>link with this change?  If sysfs_create_link_nowarn() fails then we
>haven't created a link of our own to remove.  This looks wrong.  Thanks,
>

Hmm... you are right, that's my bad.

Do you have comments on the first patch?

>Alex

-- 
Wei Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed
@ 2016-03-24 14:51       ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-03-24 14:51 UTC (permalink / raw)
  To: Alex Williamson
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Wei Yang

On Wed, Mar 23, 2016 at 05:04:15PM -0600, Alex Williamson wrote:
>On Wed, 23 Mar 2016 22:25:11 +0000
>Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> The original code forgets to remove the sysfs_link to a device in
>> iommu_group/devices directory, when the creation fails or conflicts on the
>> name.
>> 
>> This patch tries to remove the sysfs_link on the failure.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/iommu/iommu.c |    1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 2696a38..8f480ba 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -403,6 +403,7 @@ rename:
>>  	ret = sysfs_create_link_nowarn(group->devices_kobj,
>>  				       &dev->kobj, device->name);
>>  	if (ret) {
>> +		sysfs_remove_link(group->devices_kobj, device->name);
>>  		kfree(device->name);
>>  		if (ret == -EEXIST && i >= 0) {
>>  			/*
>
>If we failed to create a link, potentially due to a conflicting link
>already present, then aren't we arbitrarily removing that conflicting
>link with this change?  If sysfs_create_link_nowarn() fails then we
>haven't created a link of our own to remove.  This looks wrong.  Thanks,
>

Hmm... you are right, that's my bad.

Do you have comments on the first patch?

>Alex

-- 
Wei Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch V2 1/2] iommu: remove the iommu_callback_data
@ 2016-04-14 13:59     ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-04-14 13:59 UTC (permalink / raw)
  To: Wei Yang; +Cc: treding, alex.williamson, joro, iommu, linux-kernel

Hi, Alex

I think this one is correct. It applies on v4.5 and works fine.
Or I missed something else?

On Wed, Mar 23, 2016 at 10:25:10PM +0000, Wei Yang wrote:
>According to the code path, iommu_callback_data is passed in
>iommu_bus_init() and  just used in {add/remove}_iommu_group, by when the
>bus->iommu_ops is already set up properly.
>
>This patch removes the iommu_callback_data by retrieving iommu_ops from
>bus->iommu_ops directly.
>
>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>---
> drivers/iommu/iommu.c |   21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>index 0e3b009..2696a38 100644
>--- a/drivers/iommu/iommu.c
>+++ b/drivers/iommu/iommu.c
>@@ -37,10 +37,6 @@ static struct kset *iommu_group_kset;
> static struct ida iommu_group_ida;
> static struct mutex iommu_group_mutex;
> 
>-struct iommu_callback_data {
>-	const struct iommu_ops *ops;
>-};
>-
> struct iommu_group {
> 	struct kobject kobj;
> 	struct kobject *devices_kobj;
>@@ -867,8 +863,7 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
> 
> static int add_iommu_group(struct device *dev, void *data)
> {
>-	struct iommu_callback_data *cb = data;
>-	const struct iommu_ops *ops = cb->ops;
>+	const struct iommu_ops *ops = dev->bus->iommu_ops;
> 	int ret;
> 
> 	if (!ops->add_device)
>@@ -891,8 +886,7 @@ static int add_iommu_group(struct device *dev, void *data)
> 
> static int remove_iommu_group(struct device *dev, void *data)
> {
>-	struct iommu_callback_data *cb = data;
>-	const struct iommu_ops *ops = cb->ops;
>+	const struct iommu_ops *ops = dev->bus->iommu_ops;
> 
> 	if (ops->remove_device && dev->iommu_group)
> 		ops->remove_device(dev);
>@@ -953,13 +947,10 @@ static int iommu_bus_notifier(struct notifier_block *nb,
> 	return 0;
> }
> 
>-static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
>+static int iommu_bus_init(struct bus_type *bus)
> {
> 	int err;
> 	struct notifier_block *nb;
>-	struct iommu_callback_data cb = {
>-		.ops = ops,
>-	};
> 
> 	nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
> 	if (!nb)
>@@ -971,7 +962,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
> 	if (err)
> 		goto out_free;
> 
>-	err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
>+	err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
> 	if (err)
> 		goto out_err;
> 
>@@ -980,7 +971,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
> 
> out_err:
> 	/* Clean up */
>-	bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
>+	bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
> 	bus_unregister_notifier(bus, nb);
> 
> out_free:
>@@ -1012,7 +1003,7 @@ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops)
> 	bus->iommu_ops = ops;
> 
> 	/* Do IOMMU specific setup for this bus-type */
>-	err = iommu_bus_init(bus, ops);
>+	err = iommu_bus_init(bus);
> 	if (err)
> 		bus->iommu_ops = NULL;
> 
>-- 
>1.7.9.5

-- 
Wei Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch V2 1/2] iommu: remove the iommu_callback_data
@ 2016-04-14 13:59     ` Wei Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2016-04-14 13:59 UTC (permalink / raw)
  To: Wei Yang
  Cc: treding-DDmLM1+adcrQT0dZR+AlfA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi, Alex

I think this one is correct. It applies on v4.5 and works fine.
Or I missed something else?

On Wed, Mar 23, 2016 at 10:25:10PM +0000, Wei Yang wrote:
>According to the code path, iommu_callback_data is passed in
>iommu_bus_init() and  just used in {add/remove}_iommu_group, by when the
>bus->iommu_ops is already set up properly.
>
>This patch removes the iommu_callback_data by retrieving iommu_ops from
>bus->iommu_ops directly.
>
>Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>---
> drivers/iommu/iommu.c |   21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>index 0e3b009..2696a38 100644
>--- a/drivers/iommu/iommu.c
>+++ b/drivers/iommu/iommu.c
>@@ -37,10 +37,6 @@ static struct kset *iommu_group_kset;
> static struct ida iommu_group_ida;
> static struct mutex iommu_group_mutex;
> 
>-struct iommu_callback_data {
>-	const struct iommu_ops *ops;
>-};
>-
> struct iommu_group {
> 	struct kobject kobj;
> 	struct kobject *devices_kobj;
>@@ -867,8 +863,7 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
> 
> static int add_iommu_group(struct device *dev, void *data)
> {
>-	struct iommu_callback_data *cb = data;
>-	const struct iommu_ops *ops = cb->ops;
>+	const struct iommu_ops *ops = dev->bus->iommu_ops;
> 	int ret;
> 
> 	if (!ops->add_device)
>@@ -891,8 +886,7 @@ static int add_iommu_group(struct device *dev, void *data)
> 
> static int remove_iommu_group(struct device *dev, void *data)
> {
>-	struct iommu_callback_data *cb = data;
>-	const struct iommu_ops *ops = cb->ops;
>+	const struct iommu_ops *ops = dev->bus->iommu_ops;
> 
> 	if (ops->remove_device && dev->iommu_group)
> 		ops->remove_device(dev);
>@@ -953,13 +947,10 @@ static int iommu_bus_notifier(struct notifier_block *nb,
> 	return 0;
> }
> 
>-static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
>+static int iommu_bus_init(struct bus_type *bus)
> {
> 	int err;
> 	struct notifier_block *nb;
>-	struct iommu_callback_data cb = {
>-		.ops = ops,
>-	};
> 
> 	nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
> 	if (!nb)
>@@ -971,7 +962,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
> 	if (err)
> 		goto out_free;
> 
>-	err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
>+	err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
> 	if (err)
> 		goto out_err;
> 
>@@ -980,7 +971,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
> 
> out_err:
> 	/* Clean up */
>-	bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
>+	bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
> 	bus_unregister_notifier(bus, nb);
> 
> out_free:
>@@ -1012,7 +1003,7 @@ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops)
> 	bus->iommu_ops = ops;
> 
> 	/* Do IOMMU specific setup for this bus-type */
>-	err = iommu_bus_init(bus, ops);
>+	err = iommu_bus_init(bus);
> 	if (err)
> 		bus->iommu_ops = NULL;
> 
>-- 
>1.7.9.5

-- 
Wei Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-04-14 14:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-23 22:25 [Patch V2 0/2] Cleanup on IOMMU Wei Yang
2016-03-23 22:25 ` Wei Yang
2016-03-23 22:25 ` [Patch V2 1/2] iommu: remove the iommu_callback_data Wei Yang
2016-03-23 22:25   ` Wei Yang
2016-04-14 13:59   ` Wei Yang
2016-04-14 13:59     ` Wei Yang
2016-03-23 22:25 ` [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed Wei Yang
2016-03-23 22:25   ` Wei Yang
2016-03-23 23:04   ` Alex Williamson
2016-03-23 23:04     ` Alex Williamson
2016-03-24 14:51     ` Wei Yang
2016-03-24 14:51       ` Wei Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.