Re: [PATCH v6 4/5] drm: add drm_memory_stats_is_zero

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 25/10/2024 18:41, Yunxiang Li wrote:
> Add a helper to check if the memory stats is zero, this will be used to
> check for memory accounting errors.
> 
> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> ---
>   drivers/gpu/drm/drm_file.c | 9 +++++++++
>   include/drm/drm_file.h     | 1 +
>   2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 714e42b051080..75ed701d80f74 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -859,6 +859,15 @@ static void print_size(struct drm_printer *p, const char *stat,
>   	drm_printf(p, "drm-%s-%s:\t%llu%s\n", stat, region, sz, units[u]);
>   }
>   
> +int drm_memory_stats_is_zero(const struct drm_memory_stats *stats) {
> +	return (stats->shared == 0 &&
> +		stats->private == 0 &&
> +		stats->resident == 0 &&
> +		stats->purgeable == 0 &&
> +		stats->active == 0);
> +}

Could use mem_is_zero() for some value of source/binary compactness.

> +EXPORT_SYMBOL(drm_memory_stats_is_zero);
> +

I am not a huge fan of adding this as an interface as the only caller 
appears to be a sanity check in amdgpu_vm_fini():

	if (!amdgpu_vm_stats_is_zero(vm))
		dev_err(adev->dev, "VM memory stats is non-zero when fini\n");

But I guess there is some value in sanity checking since amdgpu does not 
have a notion of debug only code (compiled at production and exercised 
via a test suite).

I do suggest to demote the dev_err to notice log level would suffice and 
be more accurate.

Regards,

Tvrtko

>   /**
>    * drm_print_memory_stats - A helper to print memory stats
>    * @p: The printer to print output to
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index ab230d3af138d..7f91e35d027d9 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -477,6 +477,7 @@ struct drm_memory_stats {
>   
>   enum drm_gem_object_status;
>   
> +int drm_memory_stats_is_zero(const struct drm_memory_stats *stats);
>   void drm_print_memory_stats(struct drm_printer *p,
>   			    const struct drm_memory_stats *stats,
>   			    enum drm_gem_object_status supported_status,