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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/11/24 21:17, Jason Gunthorpe wrote:
On Thu, Nov 24, 2022 at 04:26:54AM -0800, Yi Liu wrote:
This avoids referencing device->group in __vfio_register_dev()

Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
  drivers/vfio/vfio_main.c | 52 +++++++++++++++++++++++++---------------
  1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 7a78256a650e..4980b8acf5d3 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -503,10 +503,15 @@ static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
  	return group;
  }
-static int __vfio_register_dev(struct vfio_device *device,
-		struct vfio_group *group)
+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);
/*
  	 * In all cases group is the output of one of the group allocation

This comment should be deleted

ok.

@@ -515,6 +520,16 @@ static int __vfio_register_dev(struct vfio_device *device,
  	if (IS_ERR(group))
  		return PTR_ERR(group);
+ /* Our reference on group is moved to the device */
+	device->group = group;
+	return 0;
+}
+
+static int __vfio_register_dev(struct vfio_device *device,
+			       enum vfio_group_type type)
+{
+	int ret;
+
  	if (WARN_ON(device->ops->bind_iommufd &&
  		    (!device->ops->unbind_iommufd ||
  		     !device->ops->attach_ioas)))
@@ -527,34 +542,33 @@ static int __vfio_register_dev(struct vfio_device *device,
  	if (!device->dev_set)
  		vfio_assign_device_set(device, device);
- /* Our reference on group is moved to the device */
-	device->group = group;
-
  	ret = dev_set_name(&device->device, "vfio%d", device->index);
  	if (ret)
-		goto err_out;
+		return ret;
- ret = device_add(&device->device);
+	ret = vfio_device_set_group(device, type);
  	if (ret)
-		goto err_out;
+		return ret;
+
+	ret = device_add(&device->device);
+	if (ret) {
+		vfio_device_remove_group(device);
+		return ret;

You could probably keep the goto

after the change, only this branch will use the err_out; may be not necessary to use goto. but keep goto may have less changes in patch
file. e.g. I'm ok to keep goto.

@@ -527,12 +542,13 @@ static int __vfio_register_dev(struct vfio_device *device,
        if (!device->dev_set)
                vfio_assign_device_set(device, device);

-       /* Our reference on group is moved to the device */
-       device->group = group;
-
        ret = dev_set_name(&device->device, "vfio%d", device->index);
        if (ret)
-               goto err_out;
+               return ret;
+
+       ret = vfio_device_set_group(device, type);
+       if (ret)
+               return ret;

        ret = device_add(&device->device);
        if (ret)


--
Regards,
Yi Liu



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux