Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753144AbbKIKLA (ORCPT ); Mon, 9 Nov 2015 05:11:00 -0500 Received: from foss.arm.com ([217.140.101.70]:49788 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753094AbbKIKK5 (ORCPT ); Mon, 9 Nov 2015 05:10:57 -0500 Date: Mon, 9 Nov 2015 10:10:59 +0000 From: Will Deacon To: Peng Fan Cc: joro@8bytes.org, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [RFC V2] iommu: correct group reference count Message-ID: <20151109101058.GB24936@arm.com> References: <1447049608-6123-1-git-send-email-van.freenix@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1447049608-6123-1-git-send-email-van.freenix@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2258 Lines: 62 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 > 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); > - } 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 -- 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/