[PATCH 0/6 V4] fdinfo shared stats

[PATCH 0/6 V4] 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.

v4: switch drm_gem_object_is_shared_for_memory_stats() to an inline function

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/i915/i915_drm_client.c     |  2 +-
 drivers/gpu/drm/xe/xe_drm_client.c         |  2 +-
 include/drm/drm_gem.h                      | 13 +++++++++++++
 8 files changed, 38 insertions(+), 4 deletions(-)

-- 
2.42.0

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

[Alex Deucher]

Clarify the documentation 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/
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.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).

v2: switch to inline function

Link: https://lore.kernel.org/all/20231207180225.439482-1-alexander.deucher@amd.com/
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v1)
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/drm/drm_gem.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 369505447acd..2ebec3984cd4 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -553,6 +553,19 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
 
 int drm_gem_evict(struct drm_gem_object *obj);
 
+/**
+ * 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
+ */
+static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj)
+{
+	return (obj->handle_count > 1) || obj->dma_buf;
+}
+
 #ifdef CONFIG_LOCKDEP
 /**
  * drm_gem_gpuva_set_lock() - Set the lock protecting accesses to the gpuva list.
-- 
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)
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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/
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.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 0/6 V4] fdinfo shared stats

[Christian König]

Am 12.02.24 um 22:04 schrieb 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.
>
> v4: switch drm_gem_object_is_shared_for_memory_stats() to an inline function

I'm still not sure if looking at the actual handle count is the right 
approach, but it's certainly better than before.

So Reviewed-by: Christian König <christian.koenig@amd.com> for the 
entire series.

Should I take this through drm-misc-next?

Regards,
Christian.

>
> 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/i915/i915_drm_client.c     |  2 +-
>   drivers/gpu/drm/xe/xe_drm_client.c         |  2 +-
>   include/drm/drm_gem.h                      | 13 +++++++++++++
>   8 files changed, 38 insertions(+), 4 deletions(-)
>

Re: [PATCH 0/6 V4] fdinfo shared stats

[Alex Deucher]

On Thu, Feb 15, 2024 at 9:18 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 12.02.24 um 22:04 schrieb 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.
> >
> > v4: switch drm_gem_object_is_shared_for_memory_stats() to an inline function
>
> I'm still not sure if looking at the actual handle count is the right
> approach, but it's certainly better than before.

Well, it's consistent across drivers.

>
> So Reviewed-by: Christian König <christian.koenig@amd.com> for the
> entire series.
>
> Should I take this through drm-misc-next?

Yes, please.

Thanks,

Alex

>
> Regards,
> Christian.
>
> >
> > 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/i915/i915_drm_client.c     |  2 +-
> >   drivers/gpu/drm/xe/xe_drm_client.c         |  2 +-
> >   include/drm/drm_gem.h                      | 13 +++++++++++++
> >   8 files changed, 38 insertions(+), 4 deletions(-)
> >
>

Re: [PATCH 0/6 V4] fdinfo shared stats

[Christian König]

Am 15.02.24 um 15:20 schrieb Alex Deucher:
> On Thu, Feb 15, 2024 at 9:18 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 12.02.24 um 22:04 schrieb 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.
>>>
>>> v4: switch drm_gem_object_is_shared_for_memory_stats() to an inline function
>> I'm still not sure if looking at the actual handle count is the right
>> approach, but it's certainly better than before.
> Well, it's consistent across drivers.

Yeah, which makes it easy to change if we find something better.

>
>> So Reviewed-by: Christian König <christian.koenig@amd.com> for the
>> entire series.
>>
>> Should I take this through drm-misc-next?
> Yes, please.

Done.

Regards,
Christian.

>
> Thanks,
>
> Alex
>
>> Regards,
>> Christian.
>>
>>> 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/i915/i915_drm_client.c     |  2 +-
>>>    drivers/gpu/drm/xe/xe_drm_client.c         |  2 +-
>>>    include/drm/drm_gem.h                      | 13 +++++++++++++
>>>    8 files changed, 38 insertions(+), 4 deletions(-)
>>>