Hi Alex, On Mon, Nov 09, 2015 at 08:28:09AM -0700, Alex Williamson wrote: >On Mon, 2015-11-09 at 14:13 +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. >
Thanks for comments. >The premise seems incorrect to me. In the first case where >iommu_group_get() provides a group, the reference is increased by 1, but >the device is already a member of the group and therefore already >increases the reference count of the group by 1. The minimum group >reference at that point is therefore 2. Yeah. The minimum group reference at that point is 2. >In the second case, the group >is created, which gives us our first reference, and the device is added, >giving us our second reference. Therefore the minimum reference count >is also 2. Yeah. >If we decrement it, allowing the caller to get the group >with a single reference and they release that reference, the group will >be destroyed even though it has a device member. Thanks. Then Caller of iommu_group_get_for_dev should decrease the count by 1. Thanks, Peng. >One reference to the >group is for the caller, the other reference is for the device contained >in the group. Thanks, > >Alex > >> Signed-off-by: Peng Fan <van.free...@gmail.com> >> Cc: Joerg Roedel <j...@8bytes.org> >> Cc: Will Deacon <will.dea...@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); >> - } >> >> return group; >> } > > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
