Subject: Re: [Patch v7 2/5] RDMA/mana_ib: Register Mana IB device with
Management SW

On Mon, Oct 16, 2023 at 03:11:59PM -0700,
sharmaajay@linuxonhyperv.com wrote:

quoted

diff --git a/drivers/infiniband/hw/mana/device.c

b/drivers/infiniband/hw/mana/device.c
index 083f27246ba8..ea4c8c8fc10d 100644

--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c

@@ -78,22 +78,34 @@ static int mana_ib_probe(struct auxiliary_device

*adev,

quoted

 	mib_dev->ib_dev.num_comp_vectors = 1;
 	mib_dev->ib_dev.dev.parent = mdev->gdma_context->dev;

-	ret = ib_register_device(&mib_dev->ib_dev, "mana_%d",
-				 mdev->gdma_context->dev);
+	ret = mana_gd_register_device(&mib_dev->gc->mana_ib);
 	if (ret) {
-		ib_dealloc_device(&mib_dev->ib_dev);
-		return ret;
+		ibdev_err(&mib_dev->ib_dev, "Failed to register device,

ret %d",

quoted

+			  ret);
+		goto free_ib_device;
 	}

+	ret = ib_register_device(&mib_dev->ib_dev, "mana_%d",
+				 mdev->gdma_context->dev);
+	if (ret)
+		goto deregister_device;
+
 	dev_set_drvdata(&adev->dev, mib_dev);

 	return 0;
+
+deregister_device:
+	mana_gd_deregister_device(&mib_dev->gc->mana_ib);
+free_ib_device:
+	ib_dealloc_device(&mib_dev->ib_dev);
+	return ret;
 }

 static void mana_ib_remove(struct auxiliary_device *adev)  {
 	struct mana_ib_dev *mib_dev = dev_get_drvdata(&adev->dev);

+	mana_gd_deregister_device(&mib_dev->gc->mana_ib);
 	ib_unregister_device(&mib_dev->ib_dev);
 	ib_dealloc_device(&mib_dev->ib_dev);
 }

That's definitely in the wrong order

Are you shure these things should just be part of
ops->enable_driver/dealloc_driver?

I think we want to register with the management interface before calling ib_register_device(). Because we need to communicate with PF to respond to query_device().

But the order in mana_ib_remove() is wrong, the call to mana_gd_deregister_device() should be moved after calling ib_unregister_device().

Thanks,

Long