* [Patch V2 0/2] Cleanup on IOMMU @ 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 ` [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed Wei Yang 0 siblings, 2 replies; 6+ 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] 6+ messages in thread
* [Patch V2 1/2] iommu: remove the iommu_callback_data 2016-03-23 22:25 [Patch V2 0/2] Cleanup on IOMMU Wei Yang @ 2016-03-23 22:25 ` 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 1 sibling, 1 reply; 6+ 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 [flat|nested] 6+ messages in thread
* Re: [Patch V2 1/2] iommu: remove the iommu_callback_data 2016-03-23 22:25 ` [Patch V2 1/2] iommu: remove the iommu_callback_data Wei Yang @ 2016-04-14 13:59 ` Wei Yang 0 siblings, 0 replies; 6+ 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] 6+ messages in thread
* [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed 2016-03-23 22:25 [Patch V2 0/2] Cleanup on IOMMU 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-03-23 23:04 ` Alex Williamson 1 sibling, 1 reply; 6+ 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 [flat|nested] 6+ messages in thread
* Re: [Patch V2 2/2] iommu: remove sysfs_link to device in iommu_group/devices when failed 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 23:04 ` Alex Williamson 2016-03-24 14:51 ` Wei Yang 0 siblings, 1 reply; 6+ 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] 6+ 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 @ 2016-03-24 14:51 ` Wei Yang 0 siblings, 0 replies; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2016-04-14 14:00 UTC | newest] Thread overview: 6+ 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 ` [Patch V2 1/2] iommu: remove the iommu_callback_data 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 23:04 ` Alex Williamson 2016-03-24 14:51 ` Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®