[PATCH] vfio/ap_ops: Add missed vfio_uninit_group_dev()

[PATCH] vfio/ap_ops: Add missed vfio_uninit_group_dev()

[Jason Gunthorpe]

Without this call an xarray entry is leaked when the vfio_ap device is
unprobed. It was missed when the below patch was rebased across the
dev_set patch.

Fixes: eb0feefd4c02 ("vfio/ap_ops: Convert to use vfio_register_group_dev()")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/crypto/vfio_ap_ops.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 2347808fa3e427..54bb0c22e8020e 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -360,6 +360,7 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev)
 	mutex_lock(&matrix_dev->lock);
 	list_del(&matrix_mdev->node);
 	mutex_unlock(&matrix_dev->lock);
+	vfio_uninit_group_dev(&matrix_mdev->vdev);
 	kfree(matrix_mdev);
 err_dec_available:
 	atomic_inc(&matrix_dev->available_instances);
@@ -375,8 +376,8 @@ static void vfio_ap_mdev_remove(struct mdev_device *mdev)
 	mutex_lock(&matrix_dev->lock);
 	vfio_ap_mdev_reset_queues(matrix_mdev);
 	list_del(&matrix_mdev->node);
+	vfio_uninit_group_dev(&matrix_mdev->vdev);
 	kfree(matrix_mdev);
 	atomic_inc(&matrix_dev->available_instances);
 	mutex_unlock(&matrix_dev->lock);

-- 
2.33.0

Re: [PATCH] vfio/ap_ops: Add missed vfio_uninit_group_dev()

[Christoph Hellwig]

On Thu, Sep 09, 2021 at 02:24:00PM -0300, Jason Gunthorpe wrote:
> Without this call an xarray entry is leaked when the vfio_ap device is
> unprobed. It was missed when the below patch was rebased across the
> dev_set patch.
> 
> Fixes: eb0feefd4c02 ("vfio/ap_ops: Convert to use vfio_register_group_dev()")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] vfio/ap_ops: Add missed vfio_uninit_group_dev()

[Alex Williamson]

On Thu,  9 Sep 2021 14:24:00 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> Without this call an xarray entry is leaked when the vfio_ap device is
> unprobed. It was missed when the below patch was rebased across the
> dev_set patch.
> 
> Fixes: eb0feefd4c02 ("vfio/ap_ops: Convert to use vfio_register_group_dev()")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/s390/crypto/vfio_ap_ops.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 2347808fa3e427..54bb0c22e8020e 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -360,6 +360,7 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev)
>  	mutex_lock(&matrix_dev->lock);
>  	list_del(&matrix_mdev->node);
>  	mutex_unlock(&matrix_dev->lock);
> +	vfio_uninit_group_dev(&matrix_mdev->vdev);
>  	kfree(matrix_mdev);
>  err_dec_available:
>  	atomic_inc(&matrix_dev->available_instances);
> @@ -375,8 +376,8 @@ static void vfio_ap_mdev_remove(struct mdev_device *mdev)


Not sure if you're editing patches by hand, but your line counts above
don't match the chunk below, should be ,6..,7 as the previous chunk.
It's malformed as is.  Thanks,

Alex

>  	mutex_lock(&matrix_dev->lock);
>  	vfio_ap_mdev_reset_queues(matrix_mdev);
>  	list_del(&matrix_mdev->node);
> +	vfio_uninit_group_dev(&matrix_mdev->vdev);
>  	kfree(matrix_mdev);
>  	atomic_inc(&matrix_dev->available_instances);
>  	mutex_unlock(&matrix_dev->lock);
> 

Re: [PATCH] vfio/ap_ops: Add missed vfio_uninit_group_dev()

[Jason Gunthorpe]

On Fri, Sep 10, 2021 at 02:25:31PM -0600, Alex Williamson wrote:
> On Thu,  9 Sep 2021 14:24:00 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > Without this call an xarray entry is leaked when the vfio_ap device is
> > unprobed. It was missed when the below patch was rebased across the
> > dev_set patch.
> > 
> > Fixes: eb0feefd4c02 ("vfio/ap_ops: Convert to use vfio_register_group_dev()")
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> >  drivers/s390/crypto/vfio_ap_ops.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> > index 2347808fa3e427..54bb0c22e8020e 100644
> > +++ b/drivers/s390/crypto/vfio_ap_ops.c
> > @@ -360,6 +360,7 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev)
> >  	mutex_lock(&matrix_dev->lock);
> >  	list_del(&matrix_mdev->node);
> >  	mutex_unlock(&matrix_dev->lock);
> > +	vfio_uninit_group_dev(&matrix_mdev->vdev);
> >  	kfree(matrix_mdev);
> >  err_dec_available:
> >  	atomic_inc(&matrix_dev->available_instances);
> > @@ -375,8 +376,8 @@ static void vfio_ap_mdev_remove(struct mdev_device *mdev)
> 
> 
> Not sure if you're editing patches by hand, but your line counts above
> don't match the chunk below, should be ,6..,7 as the previous chunk.

No, never... 

It took awhile to figure out but it turns out emacs did it?!

I've been trying out the 'base-commit' trailer feature and removed it
by hand from this patch before sending because it was just some bogus
local merge. When I did this emacs diff mode wrongly thought the diff
was being edited and corrupted the @@ line.

Wow, I had no idea it could edit patches :\

Jason