On 29/06/16 08:07, Alex Williamson wrote: > On Tue, 28 Jun 2016 23:37:06 +0200 > Heiner Kallweit <hkallwe...@gmail.com> wrote: > >> Am 28.06.2016 um 22:09 schrieb Alex Williamson: >>> On Tue, 28 Jun 2016 20:40:37 +0200 >>> Heiner Kallweit <hkallwe...@gmail.com> wrote: >>> >>>> The init function ensures that iommu_group_kset can't be NULL. >>>> Therefore this check isn't needed. >>> >>> Have you seen your previous patch? Wasn't this claim that the init >>> function ensures iommu_group_kset more true when iommu_init() called >>> BUG_ON(!iommu_group_kset) than it is when we can get by with just >>> returning -ENOMEM? Thanks, >>> >> Uh, you're right. I was in "module mode" where initcall retvals are >> assessed. Here the initcall retval is ignored. >> If we leave the BUG_ON in however then the check is actually unneeded >> as the system is halted after the BUG_ON. >> >> By the way: >> This check is in iommu_group_get_by_id() and this function doesn't seem >> to have any user in the kernel. It was added about three years ago. >> Is the original intention of this function obsolete meanwhile? > > > Alexey, this is a question for you. Did the user of this function > never materialize:
Not yet and probably never will. I am (still) working on KVM+VFIO+SPAPR acceleration and one of the approaches uses this helper, another one (proposed by David a couple of months ago) does not. This week I'll post a version and see if it goes well, then we will ditch iommu_group_get_by_id(). > > commit aa16bea929aed6ea854b55d2be8306a9fb40e694 > Author: Alexey Kardashevskiy <a...@ozlabs.ru> > Date: Mon Mar 25 10:23:49 2013 +1100 > > iommu: Add a function to find an iommu group by id > > As IOMMU groups are exposed to the user space by their numbers, > the user space can use them in various kernel APIs so the kernel > might need an API to find a group by its ID. > > As an example, QEMU VFIO on PPC64 platform needs it to associate > a logical bus number (LIOBN) with a specific IOMMU group in order > to support in-kernel handling of DMA map/unmap requests. > > The patch adds the iommu_group_get_by_id(id) function which performs > such search. > > If not, let's remove it. Dead code is broken code. Thanks, > > Alex > >>>> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com> >>>> --- >>>> drivers/iommu/iommu.c | 3 --- >>>> 1 file changed, 3 deletions(-) >>>> >>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>>> index b36ec9c..b36e24d 100644 >>>> --- a/drivers/iommu/iommu.c >>>> +++ b/drivers/iommu/iommu.c >>>> @@ -235,9 +235,6 @@ struct iommu_group *iommu_group_get_by_id(int id) >>>> struct iommu_group *group; >>>> const char *name; >>>> >>>> - if (!iommu_group_kset) >>>> - return NULL; >>>> - >>>> name = kasprintf(GFP_KERNEL, "%d", id); >>>> if (!name) >>>> return NULL; >>> >>> >> > -- Alexey _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
