On Tue, 20 Jul 2021 14:42:48 -0300 Jason Gunthorpe jgg@nvidia.com wrote:
Compared to mbochs_remove() two cases are missing from the vfio_register_group_dev() unwind. Add them in.
Fixes: 681c1615f891 ("vfio/mbochs: Convert to use vfio_register_group_dev()") Reported-by: Cornelia Huck cohuck@redhat.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com
samples/vfio-mdev/mbochs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index e81b875b4d87b4..501845b08c0974 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev)
ret = vfio_register_group_dev(&mdev_state->vdev); if (ret)
goto err_mem;
dev_set_drvdata(&mdev->dev, mdev_state); return 0;goto err_bytes;
+err_bytes:
- mbochs_used_mbytes -= mdev_state->type->mbytes;
err_mem:
- kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state); return ret;
@@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev) { struct mdev_state *mdev_state = dev_get_drvdata(&mdev->dev);
- mbochs_used_mbytes -= mdev_state->type->mbytes; vfio_unregister_group_dev(&mdev_state->vdev);
- mbochs_used_mbytes -= mdev_state->type->mbytes; kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state);
Hmm, doesn't this suggest we need another atomic conversion? (untested)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index e81b875b4d87..842819e29c6b 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -129,7 +129,7 @@ static dev_t mbochs_devt; static struct class *mbochs_class; static struct cdev mbochs_cdev; static struct device mbochs_dev; -static int mbochs_used_mbytes; +static atomic_t mbochs_avail_mbytes; static const struct vfio_device_ops mbochs_dev_ops;
struct vfio_region_info_ext { @@ -511,14 +511,19 @@ static int mbochs_probe(struct mdev_device *mdev) &mbochs_types[mdev_get_type_group_id(mdev)]; struct device *dev = mdev_dev(mdev); struct mdev_state *mdev_state; + int avail_mbytes = atomic_read(&mbochs_avail_mbytes); int ret = -ENOMEM;
- if (type->mbytes + mbochs_used_mbytes > max_mbytes) - return -ENOMEM; + do { + if (avail_mbytes < type->mbytes) + return ret; + } while (!atomic_try_cmpxchg(&mbochs_avail_mbytes, &avail_mbytes, + avail_mbytes - type->mbytes));
mdev_state = kzalloc(sizeof(struct mdev_state), GFP_KERNEL); if (mdev_state == NULL) - return -ENOMEM; + goto err_resv; + vfio_init_group_dev(&mdev_state->vdev, &mdev->dev, &mbochs_dev_ops);
mdev_state->vconfig = kzalloc(MBOCHS_CONFIG_SPACE_SIZE, GFP_KERNEL); @@ -549,8 +554,6 @@ static int mbochs_probe(struct mdev_device *mdev) mbochs_create_config_space(mdev_state); mbochs_reset(mdev_state);
- mbochs_used_mbytes += type->mbytes; - ret = vfio_register_group_dev(&mdev_state->vdev); if (ret) goto err_mem; @@ -558,8 +561,11 @@ static int mbochs_probe(struct mdev_device *mdev) return 0;
err_mem: + kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state); +err_resv: + atomic_add(mdev_state->type->mbytes, &mbochs_avail_mbytes); return ret; }
@@ -567,11 +573,11 @@ static void mbochs_remove(struct mdev_device *mdev) { struct mdev_state *mdev_state = dev_get_drvdata(&mdev->dev);
- mbochs_used_mbytes -= mdev_state->type->mbytes; vfio_unregister_group_dev(&mdev_state->vdev); kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state); + atomic_add(mdev_state->type->mbytes, &mbochs_avail_mbytes); }
static ssize_t mbochs_read(struct vfio_device *vdev, char __user *buf, @@ -1351,7 +1357,7 @@ static ssize_t available_instances_show(struct mdev_type *mtype, { const struct mbochs_type *type = &mbochs_types[mtype_get_type_group_id(mtype)]; - int count = (max_mbytes - mbochs_used_mbytes) / type->mbytes; + int count = atomic_read(&mbochs_avail_mbytes) / type->mbytes;
return sprintf(buf, "%d\n", count); } @@ -1460,6 +1466,8 @@ static int __init mbochs_dev_init(void) if (ret) goto err_class;
+ atomic_set(&mbochs_avail_mbytes, max_mbytes); + ret = mdev_register_device(&mbochs_dev, &mdev_fops); if (ret) goto err_device;