Re: [PATCH] drm/amdgpu: add shared fdinfo stats

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 17.11.23 um 20:56 schrieb Alex Deucher:
> Add shared stats.  Useful for seeing shared memory.
>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
>   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 d79b4ca1ecfc..c24f7b2c04c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1287,25 +1287,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 = obj->handle_count > 1;

Interesting approach but I don't think that this is correct.

The handle_count is basically how many GEM handles are there for BO, so 
for example it doesn't catch sharing things with V4L.

What we should probably rather do is to take a look if 
bo->tbo.base.dma_buf is NULL or not.

Regards,
Christian.


> +
>   	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 d28e21baef16..0503af75dc26 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 */