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