Re: [PATCH 05/14] vfio/mdev: simplify mdev_type handling

Eric Farman
2022-07-20T20:47:32Z

On Sat, 2022-07-09 at 06:54 +0200, Christoph Hellwig wrote:
> Instead of abusing struct attribute_group to control initialization
> of
> struct mdev_type, just define the actual attributes in the
> mdev_driver,
> allocate the mdev_type structures in the caller and pass them to
> mdev_register_parent.
>
> This allows the caller to use container_of to get at the containing
> structure and thus significantly simplify the code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> ---

...snip...

> diff --git a/drivers/vfio/mdev/mdev_sysfs.c
> b/drivers/vfio/mdev/mdev_sysfs.c
> b/drivers/vfio/mdev/mdev_sysfs.c
> index b71ffc5594870..80b2d546a3d98 100644
> --- a/drivers/vfio/mdev/mdev_sysfs.c
> +++ b/drivers/vfio/mdev/mdev_sysfs.c
> @@ -90,35 +90,21 @@ static struct kobj_type mdev_type_ktype = {
> .release = mdev_type_release,
> };
>
> -static struct mdev_type *add_mdev_supported_type(struct mdev_parent
> *parent,
> - unsigned int
> type_group_id)
> +static int mdev_type_add(struct mdev_parent *parent, struct
> mdev_type *type)
> {
> - struct mdev_type *type;
> - struct attribute_group *group =
> - parent->mdev_driver-
> >supported_type_groups[type_group_id];
> int ret;
>
> - if (!group->name) {
> - pr_err("%s: Type name empty!\n", __func__);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - type = kzalloc(sizeof(*type), GFP_KERNEL);
> - if (!type)
> - return ERR_PTR(-ENOMEM);
> -

Since mdev_type is embedded in the parent and the alloc is removed,
shouldn't the kfree(type) in mdev_type_release() also be removed? (This
appears to be at least one of the causes of my system crashes.)

...snip.
..