[Intel-gfx] [PATCH 02/13] vfio: Introduce a vfio_uninit_group_dev() API call

[Cornelia Huck]

On Mon, Jul 19 2021, Jason Gunthorpe <jgg at nvidia.com> wrote:

> On Mon, Jul 19, 2021 at 02:11:38PM +0200, Cornelia Huck wrote:
>> On Wed, Jul 14 2021, Jason Gunthorpe <jgg at nvidia.com> wrote:
>> 
>> > From: Max Gurtovoy <mgurtovoy at nvidia.com>
>> >
>> > This pairs with vfio_init_group_dev() and allows undoing any state that is
>> > stored in the vfio_device unrelated to registration. Add appropriately
>> > placed calls to all the drivers.
>> >
>> > The following patch will use this to add pre-registration state for the
>> > device set.
>> >
>> > Signed-off-by: Max Gurtovoy <mgurtovoy at nvidia.com>
>> > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
>> >  Documentation/driver-api/vfio.rst            |  4 ++-
>> >  drivers/vfio/fsl-mc/vfio_fsl_mc.c            |  6 +++--
>> >  drivers/vfio/mdev/vfio_mdev.c                | 13 +++++++---
>> >  drivers/vfio/pci/vfio_pci.c                  |  6 +++--
>> >  drivers/vfio/platform/vfio_platform_common.c |  7 +++--
>> >  drivers/vfio/vfio.c                          |  5 ++++
>> >  include/linux/vfio.h                         |  1 +
>> >  samples/vfio-mdev/mbochs.c                   |  2 ++
>> >  samples/vfio-mdev/mdpy.c                     | 25 ++++++++++--------
>> >  samples/vfio-mdev/mtty.c                     | 27 ++++++++++++--------
>> >  10 files changed, 64 insertions(+), 32 deletions(-)
>> 
>> (...)
>> 
>> > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
>> > index e81b875b4d87b4..cf264d0bf11053 100644
>> > +++ b/samples/vfio-mdev/mbochs.c
>> > @@ -558,6 +558,7 @@ static int mbochs_probe(struct mdev_device *mdev)
>> >  	return 0;
>> >  
>> >  err_mem:
>> > +	vfio_uninit_group_dev(&mdev_state->vdev);
>> >  	kfree(mdev_state->vconfig);
>> >  	kfree(mdev_state);
>> >  	return ret;
>
> Doesn't this leak pages? Sigh.

I think it also fails to decrease mbochs_used_mbytes; looks like there
need to be two cleanup labels.

>
>> > @@ -571,6 +572,7 @@ static void mbochs_remove(struct mdev_device *mdev)
>> >  	vfio_unregister_group_dev(&mdev_state->vdev);
>> >  	kfree(mdev_state->pages);
>> >  	kfree(mdev_state->vconfig);
>> > +	vfio_uninit_group_dev(&mdev_state->vdev);
>> 
>> Does the location of the uninit vs the kfree matter? Even if not, it
>> might be good to keep it consistent.
>
> It does not, but I will reorder it anyhow
>
> Jason