On Mon, Jul 19 2021, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > On Mon, Jul 19, 2021 at 02:11:38PM +0200, Cornelia Huck wrote: >> On Wed, Jul 14 2021, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: >> >> > From: Max Gurtovoy <mgurtovoy@xxxxxxxxxx> >> > >> > 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@xxxxxxxxxx> >> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> >> > 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