accel/habanalabs: postpone mem_mgr IDR destruction to hpriv_release()

The memory manager IDR is currently destroyed when user releases the
file descriptor.
However, at this point the user context might be still held, and memory
buffers might be still in use.
Later on, calls to release those buffers will fail due to not finding
their handles in the IDR, leading to a memory leak.
To avoid this leak, split the IDR destruction from the memory manager
fini, and postpone it to hpriv_release() when there is no user context
and no buffers are used.

Signed-off-by: Tomer Tayar <ttayar@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index 8db00cb3b71ddcdaf13f6e83f6282a86507475f9..713005998cbcf01cb337a5fc9e15560f14671158 100644 (file)
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -413,6 +413,9 @@ static void hpriv_release(struct kref *ref)
        mutex_destroy(&hpriv->ctx_lock);
        mutex_destroy(&hpriv->restore_phase_mutex);
 
+       /* There should be no memory buffers at this point and handles IDR can be destroyed */
+       hl_mem_mgr_idr_destroy(&hpriv->mem_mgr);
+
        /* Device should be reset if reset-upon-device-release is enabled, or if there is a pending
         * reset that waits for device release.
         */
@@ -534,6 +537,10 @@ static int hl_device_release(struct inode *inode, struct file *filp)
        }
 
        hl_ctx_mgr_fini(hdev, &hpriv->ctx_mgr);
+
+       /* Memory buffers might be still in use at this point and thus the handles IDR destruction
+        * is postponed to hpriv_release().
+        */
        hl_mem_mgr_fini(&hpriv->mem_mgr);
 
        hdev->compute_ctx_in_release = 1;
@@ -910,6 +917,7 @@ static int device_early_init(struct hl_device *hdev)
 
 free_cb_mgr:
        hl_mem_mgr_fini(&hdev->kernel_mem_mgr);
+       hl_mem_mgr_idr_destroy(&hdev->kernel_mem_mgr);
 free_chip_info:
        kfree(hdev->hl_chip_info);
 free_prefetch_wq:
@@ -953,6 +961,7 @@ static void device_early_fini(struct hl_device *hdev)
        mutex_destroy(&hdev->clk_throttling.lock);
 
        hl_mem_mgr_fini(&hdev->kernel_mem_mgr);
+       hl_mem_mgr_idr_destroy(&hdev->kernel_mem_mgr);
 
        kfree(hdev->hl_chip_info);
 

diff --git a/drivers/accel/habanalabs/common/habanalabs.h b/drivers/accel/habanalabs/common/habanalabs.h
index af5a51f8c173b234c616003aac9261dc8760e585..98e6d98cf868cdff419d50cc5aa8e505e4da9646 100644 (file)
--- a/drivers/accel/habanalabs/common/habanalabs.h
+++ b/drivers/accel/habanalabs/common/habanalabs.h
@@ -3927,6 +3927,7 @@ const char *hl_sync_engine_to_string(enum hl_sync_engine_type engine_type);
 
 void hl_mem_mgr_init(struct device *dev, struct hl_mem_mgr *mmg);
 void hl_mem_mgr_fini(struct hl_mem_mgr *mmg);
+void hl_mem_mgr_idr_destroy(struct hl_mem_mgr *mmg);
 int hl_mem_mgr_mmap(struct hl_mem_mgr *mmg, struct vm_area_struct *vma,
                    void *args);
 struct hl_mmap_mem_buf *hl_mmap_mem_buf_get(struct hl_mem_mgr *mmg,

diff --git a/drivers/accel/habanalabs/common/habanalabs_drv.c b/drivers/accel/habanalabs/common/habanalabs_drv.c
index 0cb6e52a11926ac43a082f4d368d2262213ba9ce..8c65607c83b08fc9893567b4033bb176ac32dcb8 100644 (file)
--- a/drivers/accel/habanalabs/common/habanalabs_drv.c
+++ b/drivers/accel/habanalabs/common/habanalabs_drv.c
@@ -234,6 +234,7 @@ int hl_device_open(struct inode *inode, struct file *filp)
 out_err:
        mutex_unlock(&hdev->fpriv_list_lock);
        hl_mem_mgr_fini(&hpriv->mem_mgr);
+       hl_mem_mgr_idr_destroy(&hpriv->mem_mgr);
        hl_ctx_mgr_fini(hpriv->hdev, &hpriv->ctx_mgr);
        filp->private_data = NULL;
        mutex_destroy(&hpriv->ctx_lock);

diff --git a/drivers/accel/habanalabs/common/memory_mgr.c b/drivers/accel/habanalabs/common/memory_mgr.c
index 9f57bcef3be3c894b13b3a86979b486be6b31288..30f8059f28c27fe14f2deae27b42efd88ee4b1f4 100644 (file)
--- a/drivers/accel/habanalabs/common/memory_mgr.c
+++ b/drivers/accel/habanalabs/common/memory_mgr.c
@@ -341,8 +341,19 @@ void hl_mem_mgr_fini(struct hl_mem_mgr *mmg)
                                "%s: Buff handle %u for CTX is still alive\n",
                                topic, id);
        }
+}
 
-       /* TODO: can it happen that some buffer is still in use at this point? */
+/**
+ * hl_mem_mgr_idr_destroy() - destroy memory manager IDR.
+ * @mmg: parent unified memory manager
+ *
+ * Destroy the memory manager IDR.
+ * Shall be called when IDR is empty and no memory buffers are in use.
+ */
+void hl_mem_mgr_idr_destroy(struct hl_mem_mgr *mmg)
+{
+       if (!idr_is_empty(&mmg->handles))
+               dev_crit(mmg->dev, "memory manager IDR is destroyed while it is not empty!\n");
 
        idr_destroy(&mmg->handles);
 }