RE: [RFC v2 03/11] vfio: Set device->group in helper function

From: "Tian, Kevin" <kevin.tian@intel.com>
To: "Liu, Yi L" <yi.l.liu@intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"jgg@nvidia.com" <jgg@nvidia.com>
Cc: "eric.auger@redhat.com" <eric.auger@redhat.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
	"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
	"mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [RFC v2 03/11] vfio: Set device->group in helper function
Date: Mon, 28 Nov 2022 08:08:32 +0000	[thread overview]
Message-ID: <BN9PR11MB52768F967E34C3BE70AA0FCD8C139@BN9PR11MB5276.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20221124122702.26507-4-yi.l.liu@intel.com>

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Thursday, November 24, 2022 8:27 PM
> 
> This avoids referencing device->group in __vfio_register_dev()

this is not true. There is still reference to device->group when adding
the device to the group->device_list.

this remark will become true if you put it after patch04.

> +static int vfio_device_set_group(struct vfio_device *device,
> +				 enum vfio_group_type type)
>  {
> -	int ret;
> +	struct vfio_group *group;
> +
> +	if (type == VFIO_IOMMU)
> +		group = vfio_group_find_or_alloc(device->dev);
> +	else
> +		group = vfio_noiommu_group_alloc(device->dev, type);

Do we need a WARN_ON(type == VFIO_NO_IOMMU)? 

> @@ -638,6 +651,7 @@ void vfio_unregister_group_dev(struct vfio_device
> *device)
>  	/* Balances device_add in register path */
>  	device_del(&device->device);
> 
> +	/* Balances vfio_device_set_group in register path */

"vfio_device_set_group()"

>  	vfio_device_remove_group(device);
>  }
>  EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
> --
> 2.34.1