Re: [PATCH 14/14] vfio/mdev: add mdev available instance checking to the core

[Eric Farman <farman@linux.ibm.com>]

On Sat, 2022-07-09 at 06:54 +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> Many of the mdev drivers use a simple counter for keeping track of
> the
> available instances. Move this code to the core code and store the
> counter
> in the mdev_parent. Implement it using correct locking, fixing mdpy.
> 
> Drivers just provide the value in the mdev_driver at registration
> time
> and the core code takes care of maintaining it and exposing the value
> in
> sysfs.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> [hch: count instances per-parent instead of per-type, use an atomic_t
>  to avoid taking mdev_list_lock in the show method]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> ---
>  drivers/s390/cio/vfio_ccw_drv.c       |  1 -
>  drivers/s390/cio/vfio_ccw_ops.c       | 14 +-------------
>  drivers/s390/cio/vfio_ccw_private.h   |  2 --
>  drivers/s390/crypto/vfio_ap_ops.c     | 21 +++------------------
>  drivers/s390/crypto/vfio_ap_private.h |  2 --
>  drivers/vfio/mdev/mdev_core.c         | 17 ++++++++++++++---
>  drivers/vfio/mdev/mdev_sysfs.c        |  5 ++++-
>  include/linux/mdev.h                  |  3 +++
>  samples/vfio-mdev/mdpy.c              | 23 ++++-------------------
>  9 files changed, 29 insertions(+), 59 deletions(-)

...snip...

> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 93f8caf2e5f77..2d0302995d7b7 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -70,6 +70,7 @@ int mdev_register_parent(struct mdev_parent
> *parent, struct device *dev,
>  	parent->mdev_driver = mdev_driver;
>  	parent->types = types;
>  	parent->nr_types = nr_types;
> +	atomic_set(&parent->available_instances, mdev_driver-
> >max_instances);
>  
>  	if (!mdev_bus_compat_class) {
>  		mdev_bus_compat_class =
> class_compat_register("mdev_bus");
> @@ -115,14 +116,17 @@ EXPORT_SYMBOL(mdev_unregister_parent);
>  static void mdev_device_release(struct device *dev)
>  {
>  	struct mdev_device *mdev = to_mdev_device(dev);
> -
> -	/* Pairs with the get in mdev_device_create() */
> -	kobject_put(&mdev->type->kobj);
> +	struct mdev_parent *parent = mdev->type->parent;
>  
>  	mutex_lock(&mdev_list_lock);
>  	list_del(&mdev->next);
> +	if (!parent->mdev_driver->get_available)
> +		atomic_inc(&parent->available_instances);
>  	mutex_unlock(&mdev_list_lock);
>  
> +	/* Pairs with the get in mdev_device_create() */
> +	kobject_put(&mdev->type->kobj);
> +
>  	dev_dbg(&mdev->dev, "MDEV: destroying\n");
>  	kfree(mdev);
>  }
> @@ -144,6 +148,13 @@ int mdev_device_create(struct mdev_type *type,
> const guid_t *uuid)
>  		}
>  	}
>  
> +	if (!drv->get_available) {
> +		if (atomic_dec_and_test(&parent->available_instances))
> {

Ah, subtle change between v5 and v6 to use atomics. As vfio-ccw only
has 1 available instance per mdev, this breaks us. Did you mean
atomic_dec_if_positive() ?

> +			mutex_unlock(&mdev_list_lock);
> +			return -EUSERS;
> +		}
> +	}
> +
>  	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
>  	if (!mdev) {
>  		mutex_unlock(&mdev_list_lock);
>