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

Eric Farman
2022-07-19T02:00:55Z

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);
>