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

Tony Krowiak
2021-09-21T00:42:24Z

On 9/20/21 7:19 PM, Jason Gunthorpe wrote:
> On Mon, Sep 20, 2021 at 05:26:25PM -0400, Tony Krowiak wrote:
>>
>> On 9/16/21 2:51 PM, Alex Williamson wrote:
>>> On Fri, 10 Sep 2021 20:06:30 -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()")
>>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>>>> drivers/s390/crypto/vfio_ap_ops.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>> Hi Tony, Halil, Jason (H),
>>>
>>> Any acks for this one? Thanks,
>>>
>>> Alex
>> I installed this on a test system running the latest linux
>> code from our library and ran our test suite. I got the
>> following running a simple test case that assigns some
>> adapters and domains to a mediated device then
>> starts a guest using the mdev.
> Oh, neat. There is no reason for this stuff to be in the
> matrix_dev->lock, it should be symmetrical with the error unwind in
> probe.

I moved the vfio_uninit_group_dev outside of the matrix_dev->lock
and it fixed the problem:

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);
atomic_inc(&matrix_dev->available_instances);
mutex_unlock(&matrix_dev->lock);

vfio_uninit_group_dev(&matrix_mdev->vdev);
kfree(matrix_mdev);
}

>
> I'll resend it.
>
> Thanks,
> Jason