Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751860AbbD3C3i (ORCPT ); Wed, 29 Apr 2015 22:29:38 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:35149 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751022AbbD3C3h (ORCPT ); Wed, 29 Apr 2015 22:29:37 -0400 Message-ID: <5541938A.6050803@ozlabs.ru> Date: Thu, 30 Apr 2015 12:29:30 +1000 From: Alexey Kardashevskiy User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: David Gibson CC: linuxppc-dev@lists.ozlabs.org, Benjamin Herrenschmidt , Paul Mackerras , Alex Williamson , Gavin Shan , linux-kernel@vger.kernel.org Subject: Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching References: <1429964096-11524-1-git-send-email-aik@ozlabs.ru> <1429964096-11524-10-git-send-email-aik@ozlabs.ru> <20150429021618.GH32589@voom.redhat.com> In-Reply-To: <20150429021618.GH32589@voom.redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4630 Lines: 131 On 04/29/2015 12:16 PM, David Gibson wrote: > On Sat, Apr 25, 2015 at 10:14:33PM +1000, Alexey Kardashevskiy wrote: >> This is to make extended ownership and multiple groups support patches >> simpler for review. >> >> This should cause no behavioural change. > > Um.. this doesn't appear to be true. Previously removing a group from > an enabled container would fail with EBUSY, now it forces a disable. This is the original tce_iommu_detach_group() where I cannot find EBUSY you are referring to; it did and does enforce disable. What do I miss here? static void tce_iommu_detach_group(void *iommu_data, struct iommu_group *iommu_group) { struct tce_container *container = iommu_data; struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group); BUG_ON(!tbl); mutex_lock(&container->lock); if (tbl != container->tbl) { pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n", iommu_group_id(iommu_group), iommu_group_id(tbl->it_group)); } else { if (container->enabled) { pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n", iommu_group_id(tbl->it_group)); tce_iommu_disable(container); } /* pr_debug("tce_vfio: detaching group #%u from iommu %p\n", iommu_group_id(iommu_group), iommu_group); */ container->tbl = NULL; tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size); iommu_release_ownership(tbl); } mutex_unlock(&container->lock); } > >> >> Signed-off-by: Alexey Kardashevskiy >> [aw: for the vfio related changes] >> Acked-by: Alex Williamson >> Reviewed-by: David Gibson >> --- >> drivers/vfio/vfio_iommu_spapr_tce.c | 40 ++++++++++++++++++++++--------------- >> 1 file changed, 24 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c >> index 115d5e6..0fbe03e 100644 >> --- a/drivers/vfio/vfio_iommu_spapr_tce.c >> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c >> @@ -460,16 +460,21 @@ static int tce_iommu_attach_group(void *iommu_data, >> iommu_group_id(container->tbl->it_group), >> iommu_group_id(iommu_group)); >> ret = -EBUSY; >> - } else if (container->enabled) { >> + goto unlock_exit; >> + } >> + >> + if (container->enabled) { >> pr_err("tce_vfio: attaching group #%u to enabled container\n", >> iommu_group_id(iommu_group)); >> ret = -EBUSY; >> - } else { >> - ret = iommu_take_ownership(tbl); >> - if (!ret) >> - container->tbl = tbl; >> + goto unlock_exit; >> } >> >> + ret = iommu_take_ownership(tbl); >> + if (!ret) >> + container->tbl = tbl; >> + >> +unlock_exit: >> mutex_unlock(&container->lock); >> >> return ret; >> @@ -487,19 +492,22 @@ static void tce_iommu_detach_group(void *iommu_data, >> pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n", >> iommu_group_id(iommu_group), >> iommu_group_id(tbl->it_group)); >> - } else { >> - if (container->enabled) { >> - pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n", >> - iommu_group_id(tbl->it_group)); >> - tce_iommu_disable(container); >> - } >> + goto unlock_exit; >> + } >> >> - /* pr_debug("tce_vfio: detaching group #%u from iommu %p\n", >> - iommu_group_id(iommu_group), iommu_group); */ >> - container->tbl = NULL; >> - tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size); >> - iommu_release_ownership(tbl); >> + if (container->enabled) { >> + pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n", >> + iommu_group_id(tbl->it_group)); >> + tce_iommu_disable(container); >> } >> + >> + /* pr_debug("tce_vfio: detaching group #%u from iommu %p\n", >> + iommu_group_id(iommu_group), iommu_group); */ >> + container->tbl = NULL; >> + tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size); >> + iommu_release_ownership(tbl); >> + >> +unlock_exit: >> mutex_unlock(&container->lock); >> } >> > -- Alexey -- 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/