Commit fbd0e2b0 authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Alex Williamson

vfio/mdev: Reorganize mdev_device_create()

Once the memory for the struct mdev_device is allocated it should
immediately be device_initialize()'d and filled in so that put_device()
can always be used to undo the allocation.

Place the mdev_get/put_parent() so that they are clearly protecting the
mdev->parent pointer. Move the final put to the release function so that
the lifetime rules are trivial to understand. Update the goto labels to
follow the normal convention.

Remove mdev_device_free() as the release function via device_put() is now
usable in all cases.
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Message-Id: <8-v2-d36939638fc6+d54-vfio2_jgg@nvidia.com>
Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
parent 9a302449
...@@ -71,7 +71,6 @@ static void mdev_device_remove_common(struct mdev_device *mdev) ...@@ -71,7 +71,6 @@ static void mdev_device_remove_common(struct mdev_device *mdev)
/* Balances with device_initialize() */ /* Balances with device_initialize() */
put_device(&mdev->dev); put_device(&mdev->dev);
mdev_put_parent(parent);
} }
static int mdev_device_remove_cb(struct device *dev, void *data) static int mdev_device_remove_cb(struct device *dev, void *data)
...@@ -208,8 +207,13 @@ void mdev_unregister_device(struct device *dev) ...@@ -208,8 +207,13 @@ void mdev_unregister_device(struct device *dev)
} }
EXPORT_SYMBOL(mdev_unregister_device); EXPORT_SYMBOL(mdev_unregister_device);
static void mdev_device_free(struct mdev_device *mdev) static void mdev_device_release(struct device *dev)
{ {
struct mdev_device *mdev = to_mdev_device(dev);
/* Pairs with the get in mdev_device_create() */
mdev_put_parent(mdev->parent);
mutex_lock(&mdev_list_lock); mutex_lock(&mdev_list_lock);
list_del(&mdev->next); list_del(&mdev->next);
mutex_unlock(&mdev_list_lock); mutex_unlock(&mdev_list_lock);
...@@ -218,70 +222,61 @@ static void mdev_device_free(struct mdev_device *mdev) ...@@ -218,70 +222,61 @@ static void mdev_device_free(struct mdev_device *mdev)
kfree(mdev); kfree(mdev);
} }
static void mdev_device_release(struct device *dev)
{
struct mdev_device *mdev = to_mdev_device(dev);
mdev_device_free(mdev);
}
int mdev_device_create(struct mdev_type *type, const guid_t *uuid) int mdev_device_create(struct mdev_type *type, const guid_t *uuid)
{ {
int ret; int ret;
struct mdev_device *mdev, *tmp; struct mdev_device *mdev, *tmp;
struct mdev_parent *parent = type->parent; struct mdev_parent *parent = type->parent;
mdev_get_parent(parent);
mutex_lock(&mdev_list_lock); mutex_lock(&mdev_list_lock);
/* Check for duplicate */ /* Check for duplicate */
list_for_each_entry(tmp, &mdev_list, next) { list_for_each_entry(tmp, &mdev_list, next) {
if (guid_equal(&tmp->uuid, uuid)) { if (guid_equal(&tmp->uuid, uuid)) {
mutex_unlock(&mdev_list_lock); mutex_unlock(&mdev_list_lock);
ret = -EEXIST; return -EEXIST;
goto mdev_fail;
} }
} }
mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
if (!mdev) { if (!mdev) {
mutex_unlock(&mdev_list_lock); mutex_unlock(&mdev_list_lock);
ret = -ENOMEM; return -ENOMEM;
goto mdev_fail;
} }
device_initialize(&mdev->dev);
mdev->dev.parent = parent->dev;
mdev->dev.bus = &mdev_bus_type;
mdev->dev.release = mdev_device_release;
mdev->dev.groups = parent->ops->mdev_attr_groups;
mdev->type = type;
mdev->parent = parent;
/* Pairs with the put in mdev_device_release() */
mdev_get_parent(parent);
guid_copy(&mdev->uuid, uuid); guid_copy(&mdev->uuid, uuid);
list_add(&mdev->next, &mdev_list); list_add(&mdev->next, &mdev_list);
mutex_unlock(&mdev_list_lock); mutex_unlock(&mdev_list_lock);
mdev->parent = parent; dev_set_name(&mdev->dev, "%pUl", uuid);
/* Check if parent unregistration has started */ /* Check if parent unregistration has started */
if (!down_read_trylock(&parent->unreg_sem)) { if (!down_read_trylock(&parent->unreg_sem)) {
mdev_device_free(mdev);
ret = -ENODEV; ret = -ENODEV;
goto mdev_fail; goto out_put_device;
} }
device_initialize(&mdev->dev);
mdev->dev.parent = parent->dev;
mdev->dev.bus = &mdev_bus_type;
mdev->dev.release = mdev_device_release;
dev_set_name(&mdev->dev, "%pUl", uuid);
mdev->dev.groups = parent->ops->mdev_attr_groups;
mdev->type = type;
ret = parent->ops->create(&type->kobj, mdev); ret = parent->ops->create(&type->kobj, mdev);
if (ret) if (ret)
goto ops_create_fail; goto out_unlock;
ret = device_add(&mdev->dev); ret = device_add(&mdev->dev);
if (ret) if (ret)
goto add_fail; goto out_remove;
ret = mdev_create_sysfs_files(mdev); ret = mdev_create_sysfs_files(mdev);
if (ret) if (ret)
goto sysfs_fail; goto out_del;
mdev->active = true; mdev->active = true;
dev_dbg(&mdev->dev, "MDEV: created\n"); dev_dbg(&mdev->dev, "MDEV: created\n");
...@@ -289,15 +284,14 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid) ...@@ -289,15 +284,14 @@ int mdev_device_create(struct mdev_type *type, const guid_t *uuid)
return 0; return 0;
sysfs_fail: out_del:
device_del(&mdev->dev); device_del(&mdev->dev);
add_fail: out_remove:
parent->ops->remove(mdev); parent->ops->remove(mdev);
ops_create_fail: out_unlock:
up_read(&parent->unreg_sem); up_read(&parent->unreg_sem);
out_put_device:
put_device(&mdev->dev); put_device(&mdev->dev);
mdev_fail:
mdev_put_parent(parent);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment