diff mbox series

[v3,2/2] drm/panthor: Avoid sleep locking in the internal BO size path

Message ID 20250227231628.4048738-2-adrian.larumbe@collabora.com (mailing list archive)
State New
Headers show
Series [v3,1/2] drm/panthor: Replace sleep locks with spinlocks in fdinfo path | expand

Commit Message

Adrián Larumbe Feb. 27, 2025, 11:16 p.m. UTC
Commit 434e5ca5b5d7 ("drm/panthor: Expose size of driver internal BO's over
fdinfo") locks the VMS xarray, to avoid UAF errors when the same VM is
being concurrently destroyed by another thread. However, that puts the
current thread in atomic context, which means taking the VMS' heap locks
will trigger a warning as the thread is no longer allowed to sleep.

Because in this case replacing the heap mutex with a spinlock isn't
feasible, the fdinfo handler no longer traverses the list of heaps for
every single VM associated with an open DRM file. Instead, when a new heap
chunk is allocated, its size is accumulated into a pool-wide tally, which
also makes the atomic context code path somewhat faster.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
Fixes: 3e2c8c718567 ("drm/panthor: Expose size of driver internal BO's over fdinfo")
---
 drivers/gpu/drm/panthor/panthor_heap.c | 59 +++++++++++++-------------
 drivers/gpu/drm/panthor/panthor_mmu.c  |  8 +---
 2 files changed, 31 insertions(+), 36 deletions(-)

Comments

Boris Brezillon Feb. 28, 2025, 8:18 a.m. UTC | #1
On Thu, 27 Feb 2025 23:16:26 +0000
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> -static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
> +static int panthor_alloc_heap_chunk(struct panthor_heap_pool *pool,
>  				    struct panthor_vm *vm,
>  				    struct panthor_heap *heap,
>  				    bool initial_chunk)

The pool has a ptdev and a vm, so we can get rid of the vm field and
make the panthor_alloc_heap_chunk() and panthor_fre_heap_chunk() helpers
consistent.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
index db0285ce5812..7cd05c7ee342 100644
--- a/drivers/gpu/drm/panthor/panthor_heap.c
+++ b/drivers/gpu/drm/panthor/panthor_heap.c
@@ -97,6 +97,9 @@  struct panthor_heap_pool {
 
 	/** @gpu_contexts: Buffer object containing the GPU heap contexts. */
 	struct panthor_kernel_bo *gpu_contexts;
+
+	/** @size: Size of all chunks across all heaps in the pool. */
+	atomic_t size;
 };
 
 static int panthor_heap_ctx_stride(struct panthor_device *ptdev)
@@ -118,7 +121,7 @@  static void *panthor_get_heap_ctx(struct panthor_heap_pool *pool, int id)
 	       panthor_get_heap_ctx_offset(pool, id);
 }
 
-static void panthor_free_heap_chunk(struct panthor_vm *vm,
+static void panthor_free_heap_chunk(struct panthor_heap_pool *pool,
 				    struct panthor_heap *heap,
 				    struct panthor_heap_chunk *chunk)
 {
@@ -127,11 +130,13 @@  static void panthor_free_heap_chunk(struct panthor_vm *vm,
 	heap->chunk_count--;
 	mutex_unlock(&heap->lock);
 
+	atomic_sub(heap->chunk_size, &pool->size);
+
 	panthor_kernel_bo_destroy(chunk->bo);
 	kfree(chunk);
 }
 
-static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
+static int panthor_alloc_heap_chunk(struct panthor_heap_pool *pool,
 				    struct panthor_vm *vm,
 				    struct panthor_heap *heap,
 				    bool initial_chunk)
@@ -144,7 +149,7 @@  static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
 	if (!chunk)
 		return -ENOMEM;
 
-	chunk->bo = panthor_kernel_bo_create(ptdev, vm, heap->chunk_size,
+	chunk->bo = panthor_kernel_bo_create(pool->ptdev, vm, heap->chunk_size,
 					     DRM_PANTHOR_BO_NO_MMAP,
 					     DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
 					     PANTHOR_VM_KERNEL_AUTO_VA);
@@ -180,6 +185,8 @@  static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
 	heap->chunk_count++;
 	mutex_unlock(&heap->lock);
 
+	atomic_add(heap->chunk_size, &pool->size);
+
 	return 0;
 
 err_destroy_bo:
@@ -191,16 +198,16 @@  static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
 	return ret;
 }
 
-static void panthor_free_heap_chunks(struct panthor_vm *vm,
+static void panthor_free_heap_chunks(struct panthor_heap_pool *pool,
 				     struct panthor_heap *heap)
 {
 	struct panthor_heap_chunk *chunk, *tmp;
 
 	list_for_each_entry_safe(chunk, tmp, &heap->chunks, node)
-		panthor_free_heap_chunk(vm, heap, chunk);
+		panthor_free_heap_chunk(pool, heap, chunk);
 }
 
-static int panthor_alloc_heap_chunks(struct panthor_device *ptdev,
+static int panthor_alloc_heap_chunks(struct panthor_heap_pool *pool,
 				     struct panthor_vm *vm,
 				     struct panthor_heap *heap,
 				     u32 chunk_count)
@@ -209,7 +216,7 @@  static int panthor_alloc_heap_chunks(struct panthor_device *ptdev,
 	u32 i;
 
 	for (i = 0; i < chunk_count; i++) {
-		ret = panthor_alloc_heap_chunk(ptdev, vm, heap, true);
+		ret = panthor_alloc_heap_chunk(pool, vm, heap, true);
 		if (ret)
 			return ret;
 	}
@@ -226,7 +233,7 @@  panthor_heap_destroy_locked(struct panthor_heap_pool *pool, u32 handle)
 	if (!heap)
 		return -EINVAL;
 
-	panthor_free_heap_chunks(pool->vm, heap);
+	panthor_free_heap_chunks(pool, heap);
 	mutex_destroy(&heap->lock);
 	kfree(heap);
 	return 0;
@@ -308,7 +315,7 @@  int panthor_heap_create(struct panthor_heap_pool *pool,
 	heap->max_chunks = max_chunks;
 	heap->target_in_flight = target_in_flight;
 
-	ret = panthor_alloc_heap_chunks(pool->ptdev, vm, heap,
+	ret = panthor_alloc_heap_chunks(pool, vm, heap,
 					initial_chunk_count);
 	if (ret)
 		goto err_free_heap;
@@ -342,7 +349,7 @@  int panthor_heap_create(struct panthor_heap_pool *pool,
 	return id;
 
 err_free_heap:
-	panthor_free_heap_chunks(pool->vm, heap);
+	panthor_free_heap_chunks(pool, heap);
 	mutex_destroy(&heap->lock);
 	kfree(heap);
 
@@ -389,6 +396,7 @@  int panthor_heap_return_chunk(struct panthor_heap_pool *pool,
 			removed = chunk;
 			list_del(&chunk->node);
 			heap->chunk_count--;
+			atomic_sub(heap->chunk_size, &pool->size);
 			break;
 		}
 	}
@@ -466,7 +474,7 @@  int panthor_heap_grow(struct panthor_heap_pool *pool,
 	 * further jobs in this queue fail immediately instead of having to
 	 * wait for the job timeout.
 	 */
-	ret = panthor_alloc_heap_chunk(pool->ptdev, pool->vm, heap, false);
+	ret = panthor_alloc_heap_chunk(pool, pool->vm, heap, false);
 	if (ret)
 		goto out_unlock;
 
@@ -560,6 +568,8 @@  panthor_heap_pool_create(struct panthor_device *ptdev, struct panthor_vm *vm)
 	if (ret)
 		goto err_destroy_pool;
 
+	atomic_add(pool->gpu_contexts->obj->size, &pool->size);
+
 	return pool;
 
 err_destroy_pool:
@@ -594,8 +604,10 @@  void panthor_heap_pool_destroy(struct panthor_heap_pool *pool)
 	xa_for_each(&pool->xa, i, heap)
 		drm_WARN_ON(&pool->ptdev->base, panthor_heap_destroy_locked(pool, i));
 
-	if (!IS_ERR_OR_NULL(pool->gpu_contexts))
+	if (!IS_ERR_OR_NULL(pool->gpu_contexts)) {
+		atomic_sub(pool->gpu_contexts->obj->size, &pool->size);
 		panthor_kernel_bo_destroy(pool->gpu_contexts);
+	}
 
 	/* Reflects the fact the pool has been destroyed. */
 	pool->vm = NULL;
@@ -605,27 +617,16 @@  void panthor_heap_pool_destroy(struct panthor_heap_pool *pool)
 }
 
 /**
- * panthor_heap_pool_size() - Calculate size of all chunks across all heaps in a pool
- * @pool: Pool whose total chunk size to calculate.
+ * panthor_heap_pool_size() - Get a heap pool's total size
+ * @pool: Pool whose total chunks size to return
  *
- * This function adds the size of all heap chunks across all heaps in the
- * argument pool. It also adds the size of the gpu contexts kernel bo.
- * It is meant to be used by fdinfo for displaying the size of internal
- * driver BO's that aren't exposed to userspace through a GEM handle.
+ * Returns the aggregated size of all chunks for all heaps in the pool
  *
  */
 size_t panthor_heap_pool_size(struct panthor_heap_pool *pool)
 {
-	struct panthor_heap *heap;
-	unsigned long i;
-	size_t size = 0;
-
-	down_read(&pool->lock);
-	xa_for_each(&pool->xa, i, heap)
-		size += heap->chunk_size * heap->chunk_count;
-	up_read(&pool->lock);
-
-	size += pool->gpu_contexts->obj->size;
+	if (!pool)
+		return 0;
 
-	return size;
+	return atomic_read(&pool->size);
 }
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 8c6fc587ddc3..12a02e28f50f 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1963,13 +1963,7 @@  void panthor_vm_heaps_sizes(struct panthor_file *pfile, struct drm_memory_stats
 
 	xa_lock(&pfile->vms->xa);
 	xa_for_each(&pfile->vms->xa, i, vm) {
-		size_t size = 0;
-
-		mutex_lock(&vm->heaps.lock);
-		if (vm->heaps.pool)
-			size = panthor_heap_pool_size(vm->heaps.pool);
-		mutex_unlock(&vm->heaps.lock);
-
+		size_t size = panthor_heap_pool_size(vm->heaps.pool);
 		stats->resident += size;
 		if (vm->as.id >= 0)
 			stats->active += size;