Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751993AbbKIP2O (ORCPT ); Mon, 9 Nov 2015 10:28:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56733 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751670AbbKIP2J (ORCPT ); Mon, 9 Nov 2015 10:28:09 -0500 Message-ID: <1447082889.4925.5.camel@redhat.com> Subject: Re: [RFC V2] iommu: correct group reference count From: Alex Williamson To: Peng Fan Cc: joro@8bytes.org, will.deacon@arm.com, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Date: Mon, 09 Nov 2015 08:28:09 -0700 In-Reply-To: <1447049608-6123-1-git-send-email-van.freenix@gmail.com> References: <1447049608-6123-1-git-send-email-van.freenix@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2640 Lines: 69 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. 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. 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. 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. 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 > Cc: Joerg Roedel > Cc: Will Deacon > --- > > 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; > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/