[PATCH 0/6 V3] fdinfo shared stats

[PATCH 0/6 V3] fdinfo shared stats

[Alex Deucher]

We had a request to add shared buffer stats to fdinfo for amdgpu and
while implementing that, Christian mentioned that just looking at
the GEM handle count doesn't take into account buffers shared with other
subsystems like V4L or RDMA.  Those subsystems don't use GEM, so it
doesn't really matter from a GPU top perspective, but it's more
correct if you actually want to see shared buffers.

After further discussions, add a helper and update all fdinfo
implementations to use that helper for consistency.

Alex Deucher (6):
  Documentation/gpu: Update documentation on drm-shared-*
  drm: add drm_gem_object_is_shared_for_memory_stats() helper
  drm: update drm_show_memory_stats() for dma-bufs
  drm/amdgpu: add shared fdinfo stats
  drm/i915: Update shared stats to use the new gem helper
  drm/xe: Update shared stats to use the new gem helper

 Documentation/gpu/drm-usage-stats.rst      |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 +++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++++++
 drivers/gpu/drm/drm_file.c                 |  2 +-
 drivers/gpu/drm/drm_gem.c                  | 16 ++++++++++++++++
 drivers/gpu/drm/i915/i915_drm_client.c     |  2 +-
 drivers/gpu/drm/xe/xe_drm_client.c         |  2 +-
 include/drm/drm_gem.h                      |  1 +
 9 files changed, 42 insertions(+), 4 deletions(-)

-- 
2.42.0

[PATCH 1/6] Documentation/gpu: Update documentation on drm-shared-*

[Alex Deucher]

Clarify the documentaiton in preparation for updated
helpers which check the handle count as well as whether
a dma-buf has been attached.

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 Documentation/gpu/drm-usage-stats.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
index 7aca5c7a7b1d..6dc299343b48 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -138,7 +138,7 @@ indicating kibi- or mebi-bytes.
 
 - drm-shared-<region>: <uint> [KiB|MiB]
 
-The total size of buffers that are shared with another file (ie. have more
+The total size of buffers that are shared with another file (e.g., have more
 than a single handle).
 
 - drm-total-<region>: <uint> [KiB|MiB]
-- 
2.42.0

[PATCH 2/6] drm: add drm_gem_object_is_shared_for_memory_stats() helper

[Alex Deucher]

Add a helper so that drm drivers can consistently report
shared status via the fdinfo shared memory stats interface.

In addition to handle count, show buffers as shared if they
are shared via dma-buf as well (e.g., shared with v4l or some
other subsystem).

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/drm_gem.c | 16 ++++++++++++++++
 include/drm/drm_gem.h     |  1 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 44a948b80ee1..71b5f628d828 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1506,3 +1506,19 @@ int drm_gem_evict(struct drm_gem_object *obj)
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_evict);
+
+/**
+ * drm_gem_object_is_shared_for_memory_stats - helper for shared memory stats
+ *
+ * This helper should only be used for fdinfo shared memory stats to determine
+ * if a GEM object is shared.
+ *
+ * @obj: obj in question
+ */
+bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj)
+{
+	if ((obj->handle_count > 1) || obj->dma_buf)
+		return true;
+	return false;
+}
+EXPORT_SYMBOL(drm_gem_object_is_shared_for_memory_stats);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 369505447acd..86a9c696f038 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -552,6 +552,7 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
 			       bool (*shrink)(struct drm_gem_object *obj));
 
 int drm_gem_evict(struct drm_gem_object *obj);
+bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj);
 
 #ifdef CONFIG_LOCKDEP
 /**
-- 
2.42.0

[PATCH 3/6] drm: update drm_show_memory_stats() for dma-bufs

[Alex Deucher]

Show buffers as shared if they are shared via dma-buf as well
(e.g., shared with v4l or some other subsystem).

v2: switch to gem helper

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Reviewed-by: Rob Clark <robdclark@gmail.com> (v1)
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/drm_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 8c87287c3e16..638ffa4444f5 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -913,7 +913,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file)
 					DRM_GEM_OBJECT_PURGEABLE;
 		}
 
-		if (obj->handle_count > 1) {
+		if (drm_gem_object_is_shared_for_memory_stats(obj)) {
 			status.shared += obj->size;
 		} else {
 			status.private += obj->size;
-- 
2.42.0

[PATCH 4/6] drm/amdgpu: add shared fdinfo stats

[Alex Deucher]

Add shared stats.  Useful for seeing shared memory.

v2: take dma-buf into account as well
v3: use the new gem helper

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 +++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index 5706b282a0c7..c7df7fa3459f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -97,6 +97,10 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 		   stats.requested_visible_vram/1024UL);
 	drm_printf(p, "amd-requested-gtt:\t%llu KiB\n",
 		   stats.requested_gtt/1024UL);
+	drm_printf(p, "drm-shared-vram:\t%llu KiB\n", stats.vram_shared/1024UL);
+	drm_printf(p, "drm-shared-gtt:\t%llu KiB\n", stats.gtt_shared/1024UL);
+	drm_printf(p, "drm-shared-cpu:\t%llu KiB\n", stats.cpu_shared/1024UL);
+
 	for (hw_ip = 0; hw_ip < AMDGPU_HW_IP_NUM; ++hw_ip) {
 		if (!usage[hw_ip])
 			continue;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 425cebcc5cbf..e6f69fce539b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1276,25 +1276,36 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
 			  struct amdgpu_mem_stats *stats)
 {
 	uint64_t size = amdgpu_bo_size(bo);
+	struct drm_gem_object *obj;
 	unsigned int domain;
+	bool shared;
 
 	/* Abort if the BO doesn't currently have a backing store */
 	if (!bo->tbo.resource)
 		return;
 
+	obj = &bo->tbo.base;
+	shared = drm_gem_object_is_shared_for_memory_stats(obj);
+
 	domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
 	switch (domain) {
 	case AMDGPU_GEM_DOMAIN_VRAM:
 		stats->vram += size;
 		if (amdgpu_bo_in_cpu_visible_vram(bo))
 			stats->visible_vram += size;
+		if (shared)
+			stats->vram_shared += size;
 		break;
 	case AMDGPU_GEM_DOMAIN_GTT:
 		stats->gtt += size;
+		if (shared)
+			stats->gtt_shared += size;
 		break;
 	case AMDGPU_GEM_DOMAIN_CPU:
 	default:
 		stats->cpu += size;
+		if (shared)
+			stats->cpu_shared += size;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a3ea8a82db23..be679c42b0b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -138,12 +138,18 @@ struct amdgpu_bo_vm {
 struct amdgpu_mem_stats {
 	/* current VRAM usage, includes visible VRAM */
 	uint64_t vram;
+	/* current shared VRAM usage, includes visible VRAM */
+	uint64_t vram_shared;
 	/* current visible VRAM usage */
 	uint64_t visible_vram;
 	/* current GTT usage */
 	uint64_t gtt;
+	/* current shared GTT usage */
+	uint64_t gtt_shared;
 	/* current system memory usage */
 	uint64_t cpu;
+	/* current shared system memory usage */
+	uint64_t cpu_shared;
 	/* sum of evicted buffers, includes visible VRAM */
 	uint64_t evicted_vram;
 	/* sum of evicted buffers due to CPU access */
-- 
2.42.0

[PATCH 5/6] drm/i915: Update shared stats to use the new gem helper

[Alex Deucher]

Switch to using the new gem shared memory stats helper
rather than hand rolling it.

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/i915/i915_drm_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index fa6852713bee..f58682505491 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -53,7 +53,7 @@ obj_meminfo(struct drm_i915_gem_object *obj,
 					obj->mm.region->id : INTEL_REGION_SMEM;
 	const u64 sz = obj->base.size;
 
-	if (obj->base.handle_count > 1)
+	if (drm_gem_object_is_shared_for_memory_stats(&obj->base))
 		stats[id].shared += sz;
 	else
 		stats[id].private += sz;
-- 
2.42.0

[PATCH 6/6] drm/xe: Update shared stats to use the new gem helper

[Alex Deucher]

Switch to using the new gem shared memory stats helper
rather than hand rolling it.

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/xe/xe_drm_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
index 82d1305e831f..ecf2eb67d310 100644
--- a/drivers/gpu/drm/xe/xe_drm_client.c
+++ b/drivers/gpu/drm/xe/xe_drm_client.c
@@ -113,7 +113,7 @@ static void bo_meminfo(struct xe_bo *bo,
 	else
 		mem_type = XE_PL_TT;
 
-	if (bo->ttm.base.handle_count > 1)
+	if (drm_gem_object_is_shared_for_memory_stats(&bo->ttm.base))
 		stats[mem_type].shared += sz;
 	else
 		stats[mem_type].private += sz;
-- 
2.42.0

Re: [PATCH 1/6] Documentation/gpu: Update documentation on drm-shared-*

[Tvrtko Ursulin]

On 30/01/2024 16:12, Alex Deucher wrote:
> Clarify the documentaiton in preparation for updated
> helpers which check the handle count as well as whether
> a dma-buf has been attached.
> 
> Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   Documentation/gpu/drm-usage-stats.rst | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
> index 7aca5c7a7b1d..6dc299343b48 100644
> --- a/Documentation/gpu/drm-usage-stats.rst
> +++ b/Documentation/gpu/drm-usage-stats.rst
> @@ -138,7 +138,7 @@ indicating kibi- or mebi-bytes.
>   
>   - drm-shared-<region>: <uint> [KiB|MiB]
>   
> -The total size of buffers that are shared with another file (ie. have more
> +The total size of buffers that are shared with another file (e.g., have more
>   than a single handle).
>   
>   - drm-total-<region>: <uint> [KiB|MiB]

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

Re: [PATCH 2/6] drm: add drm_gem_object_is_shared_for_memory_stats() helper

[Tvrtko Ursulin]

On 30/01/2024 16:12, Alex Deucher wrote:
> Add a helper so that drm drivers can consistently report
> shared status via the fdinfo shared memory stats interface.
> 
> In addition to handle count, show buffers as shared if they
> are shared via dma-buf as well (e.g., shared with v4l or some
> other subsystem).
> 
> Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/drm_gem.c | 16 ++++++++++++++++
>   include/drm/drm_gem.h     |  1 +
>   2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 44a948b80ee1..71b5f628d828 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1506,3 +1506,19 @@ int drm_gem_evict(struct drm_gem_object *obj)
>   	return 0;
>   }
>   EXPORT_SYMBOL(drm_gem_evict);
> +
> +/**
> + * drm_gem_object_is_shared_for_memory_stats - helper for shared memory stats
> + *
> + * This helper should only be used for fdinfo shared memory stats to determine
> + * if a GEM object is shared.
> + *
> + * @obj: obj in question
> + */
> +bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj)
> +{
> +	if ((obj->handle_count > 1) || obj->dma_buf)
> +		return true;
> +	return false;
> +}
> +EXPORT_SYMBOL(drm_gem_object_is_shared_for_memory_stats);
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 369505447acd..86a9c696f038 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -552,6 +552,7 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
>   			       bool (*shrink)(struct drm_gem_object *obj));
>   
>   int drm_gem_evict(struct drm_gem_object *obj);
> +bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj);
>   
>   #ifdef CONFIG_LOCKDEP
>   /**

Not sure what the local view on static inlines, but fine nevertheless.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

Re: [PATCH 3/6] drm: update drm_show_memory_stats() for dma-bufs

[Tvrtko Ursulin]

On 30/01/2024 16:12, Alex Deucher wrote:
> Show buffers as shared if they are shared via dma-buf as well
> (e.g., shared with v4l or some other subsystem).
> 
> v2: switch to gem helper
> 
> Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
> Reviewed-by: Rob Clark <robdclark@gmail.com> (v1)
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Rob Clark <robdclark@gmail.com>
> ---
>   drivers/gpu/drm/drm_file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 8c87287c3e16..638ffa4444f5 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -913,7 +913,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file)
>   					DRM_GEM_OBJECT_PURGEABLE;
>   		}
>   
> -		if (obj->handle_count > 1) {
> +		if (drm_gem_object_is_shared_for_memory_stats(obj)) {
>   			status.shared += obj->size;
>   		} else {
>   			status.private += obj->size;

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

Re: [PATCH 5/6] drm/i915: Update shared stats to use the new gem helper

[Tvrtko Ursulin]

On 30/01/2024 16:12, Alex Deucher wrote:
> Switch to using the new gem shared memory stats helper
> rather than hand rolling it.
> 
> Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/i915/i915_drm_client.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> index fa6852713bee..f58682505491 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -53,7 +53,7 @@ obj_meminfo(struct drm_i915_gem_object *obj,
>   					obj->mm.region->id : INTEL_REGION_SMEM;
>   	const u64 sz = obj->base.size;
>   
> -	if (obj->base.handle_count > 1)
> +	if (drm_gem_object_is_shared_for_memory_stats(&obj->base))
>   		stats[id].shared += sz;
>   	else
>   		stats[id].private += sz;

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Good that you remembered this story, I completely forgot!

Regards,

Tvrtko

Re: [PATCH 2/6] drm: add drm_gem_object_is_shared_for_memory_stats() helper

[Hamza Mahfooz]

On 1/30/24 11:12, Alex Deucher wrote:
> Add a helper so that drm drivers can consistently report
> shared status via the fdinfo shared memory stats interface.
> 
> In addition to handle count, show buffers as shared if they
> are shared via dma-buf as well (e.g., shared with v4l or some
> other subsystem).
> 
> Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/drm_gem.c | 16 ++++++++++++++++
>   include/drm/drm_gem.h     |  1 +
>   2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 44a948b80ee1..71b5f628d828 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1506,3 +1506,19 @@ int drm_gem_evict(struct drm_gem_object *obj)
>   	return 0;
>   }
>   EXPORT_SYMBOL(drm_gem_evict);
> +
> +/**
> + * drm_gem_object_is_shared_for_memory_stats - helper for shared memory stats
> + *
> + * This helper should only be used for fdinfo shared memory stats to determine
> + * if a GEM object is shared.
> + *
> + * @obj: obj in question
> + */
> +bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj)
> +{
> +	if ((obj->handle_count > 1) || obj->dma_buf)
> +		return true;
> +	return false;

nit: you can simplify this to:
	return (obj->handle_count > 1) || obj->dma_buf;

(It maybe worth just inlining this to drm_gem.h).

> +}
> +EXPORT_SYMBOL(drm_gem_object_is_shared_for_memory_stats);
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 369505447acd..86a9c696f038 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -552,6 +552,7 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
>   			       bool (*shrink)(struct drm_gem_object *obj));
>   
>   int drm_gem_evict(struct drm_gem_object *obj);
> +bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj);
>   
>   #ifdef CONFIG_LOCKDEP
>   /**
-- 
Hamza