Re: [PATCH V3 2/4] RDMA/core: Introduce shared CQ pool API

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

 




On 5/25/2020 7:47 PM, Leon Romanovsky wrote:
On Mon, May 25, 2020 at 01:42:15PM -0300, Jason Gunthorpe wrote:
On Tue, May 19, 2020 at 03:43:34PM +0300, Yamin Friedman wrote:

+void ib_cq_pool_init(struct ib_device *dev)
+{
+	int i;
I generally rather see unsigned types used for unsigned values

+
+	spin_lock_init(&dev->cq_pools_lock);
+	for (i = 0; i < ARRAY_SIZE(dev->cq_pools); i++)
+		INIT_LIST_HEAD(&dev->cq_pools[i]);
+}
+
+void ib_cq_pool_destroy(struct ib_device *dev)
+{
+	struct ib_cq *cq, *n;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dev->cq_pools); i++) {
+		list_for_each_entry_safe(cq, n, &dev->cq_pools[i],
+					 pool_entry) {
+			cq->shared = false;
+			ib_free_cq_user(cq, NULL);
WARN_ON cqe_used == 0?
An opposite is better - WARN_ON(cqe_used).

<...>

Is this check really necessary as we are closing the device?


@@ -1418,6 +1418,7 @@ int ib_register_device(struct ib_device *device, const char *name)
  		device->ops.dealloc_driver = dealloc_fn;
  		return ret;
  	}
+	ib_cq_pool_init(device);
  	ib_device_put(device);
This look like wrong placement, it should be done before enable_device
as enable_device triggers ULPs t start using the device and they might
start allocating using this API.

  	return 0;
@@ -1446,6 +1447,7 @@ static void __ib_unregister_device(struct ib_device *ib_dev)
  	if (!refcount_read(&ib_dev->refcount))
  		goto out;

+	ib_cq_pool_destroy(ib_dev);
  	disable_device(ib_dev);
similar issue, should be after disable_device as ULPs are still
running here
Sorry, this were my mistakes. I suggested to Yamin to put it here.

Thanks

I will move them to the suggest location.

Thanks




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux