Messages in this thread | | | Date | Mon, 9 Nov 2015 10:10:59 +0000 | From | Will Deacon <> | Subject | Re: [RFC V2] iommu: correct group reference count |
| |
On Mon, Nov 09, 2015 at 02:13:28PM +0800, Peng Fan wrote: > The basic flow for iommu_group_for_dev is: > iommu_group_get_for_dev > |-> iommu_group_get : increase reference count by 1. > return group; > |-> ops->device_group : Init/increase reference count to/by 1. > iommu_group_add_device : Increase reference count by 1. > return group; > > We can see that ops->device_group and iommu_group_add_device will together > increase the iommu group reference count by 2. Actually we only need 1, > but not 2. So we need add iommu_group_put after iommu_group_add_device > to make sure iommu_group_get_for_dev only increase reference count by 1. > > Signed-off-by: Peng Fan <van.freenix@gmail.com> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Will Deacon <will.deacon@arm.com> > --- > > V1 thread: https://lkml.org/lkml/2015/11/3/304 > Changes V2: > I did not see the update about device_group when I worked out V1. So > redo the patch and refine commit msg and rebased to latest linus' > linux master tree. > > drivers/iommu/iommu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index abae363..9c1971b 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -852,10 +852,10 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev) > } > > ret = iommu_group_add_device(group, dev); > - if (ret) { > - iommu_group_put(group); > + iommu_group_put(group); > + > + if (ret) > return ERR_PTR(ret); > - }
Hmm, I don't think this is correct either:
(1) It doesn't help the arm-smmu driver (which deals with platform devices too)
(2) It breaks IOMMU drivers that currently have the explicit put already (e.g. amd-iommu.c:init_iommu_group)
The easiest way for you to proceed is to extend your original patch so that it also adds an iommu_group_put to the arm-smmu-v3 driver after a successful iommu_group_get_for_dev.
Will
| |