From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jike Song Subject: Re: [PATCH kernel v2 10/11] vfio: Check for unregistered notifiers when group is actually released Date: Mon, 19 Dec 2016 18:41:05 +0800 Message-ID: <5857B941.2060805@intel.com> References: <20161218012900.18142-1-aik@ozlabs.ru> <20161218012900.18142-11-aik@ozlabs.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: linuxppc-dev@lists.ozlabs.org, Alex Williamson , David Gibson , Paul Mackerras , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org To: Alexey Kardashevskiy Return-path: Received: from mga14.intel.com ([192.55.52.115]:8017 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933442AbcLSKmA (ORCPT ); Mon, 19 Dec 2016 05:42:00 -0500 In-Reply-To: <20161218012900.18142-11-aik@ozlabs.ru> Sender: kvm-owner@vger.kernel.org List-ID: On 12/18/2016 09:28 AM, Alexey Kardashevskiy wrote: > This moves a check for unregistered notifiers from fops release > callback to the place where the group will actually be released. > > Signed-off-by: Alexey Kardashevskiy > --- > > This is going to be used in the following patch in cleanup > path. Since the next patch is RFC, this one might not be needed. Hi Alexey, I didn't find any use in the following patch 11/11, did you mean something else? BTW the warning in vfio_group_release seems too late, the user should actually unregister everything by close()? -- Thanks, Jike > --- > drivers/vfio/vfio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index 6b9a98508939..083b581e87c0 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -403,6 +403,8 @@ static void vfio_group_release(struct kref *kref) > struct iommu_group *iommu_group = group->iommu_group; > > WARN_ON(!list_empty(&group->device_list)); > + /* Any user didn't unregister? */ > + WARN_ON(group->notifier.head); > > list_for_each_entry_safe(unbound, tmp, > &group->unbound_list, unbound_next) { > @@ -1584,9 +1586,6 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep) > > filep->private_data = NULL; > > - /* Any user didn't unregister? */ > - WARN_ON(group->notifier.head); > - > vfio_group_try_dissolve_container(group); > > atomic_dec(&group->opened); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jike Song Date: Mon, 19 Dec 2016 10:41:05 +0000 Subject: Re: [PATCH kernel v2 10/11] vfio: Check for unregistered notifiers when group is actually released Message-Id: <5857B941.2060805@intel.com> List-Id: References: <20161218012900.18142-1-aik@ozlabs.ru> <20161218012900.18142-11-aik@ozlabs.ru> In-Reply-To: <20161218012900.18142-11-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alexey Kardashevskiy Cc: linuxppc-dev@lists.ozlabs.org, Alex Williamson , David Gibson , Paul Mackerras , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org On 12/18/2016 09:28 AM, Alexey Kardashevskiy wrote: > This moves a check for unregistered notifiers from fops release > callback to the place where the group will actually be released. > > Signed-off-by: Alexey Kardashevskiy > --- > > This is going to be used in the following patch in cleanup > path. Since the next patch is RFC, this one might not be needed. Hi Alexey, I didn't find any use in the following patch 11/11, did you mean something else? BTW the warning in vfio_group_release seems too late, the user should actually unregister everything by close()? -- Thanks, Jike > --- > drivers/vfio/vfio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index 6b9a98508939..083b581e87c0 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -403,6 +403,8 @@ static void vfio_group_release(struct kref *kref) > struct iommu_group *iommu_group = group->iommu_group; > > WARN_ON(!list_empty(&group->device_list)); > + /* Any user didn't unregister? */ > + WARN_ON(group->notifier.head); > > list_for_each_entry_safe(unbound, tmp, > &group->unbound_list, unbound_next) { > @@ -1584,9 +1586,6 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep) > > filep->private_data = NULL; > > - /* Any user didn't unregister? */ > - WARN_ON(group->notifier.head); > - > vfio_group_try_dissolve_container(group); > > atomic_dec(&group->opened); >