Re: [Intel-gfx] [RFC 3/6] drm: Add fdinfo memory stats

["Christian König" <christian.koenig@amd.com>]

Am 17.04.23 um 17:56 schrieb Tvrtko Ursulin:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Add support to dump GEM stats to fdinfo.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   Documentation/gpu/drm-usage-stats.rst | 12 +++++++
>   drivers/gpu/drm/drm_file.c            | 52 +++++++++++++++++++++++++++
>   include/drm/drm_drv.h                 |  7 ++++
>   include/drm/drm_file.h                |  8 +++++
>   4 files changed, 79 insertions(+)
>
> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
> index 2ab32c40e93c..8273a41b2fb0 100644
> --- a/Documentation/gpu/drm-usage-stats.rst
> +++ b/Documentation/gpu/drm-usage-stats.rst
> @@ -21,6 +21,7 @@ File format specification
>   
>   - File shall contain one key value pair per one line of text.
>   - Colon character (`:`) must be used to delimit keys and values.
> +- Caret (`^`) is also a reserved character.
>   - All keys shall be prefixed with `drm-`.
>   - Whitespace between the delimiter and first non-whitespace character shall be
>     ignored when parsing.
> @@ -105,6 +106,17 @@ object belong to this client, in the respective memory region.
>   Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
>   indicating kibi- or mebi-bytes.
>   
> +- drm-memory-<str>^size:      <uint> [KiB|MiB]
> +- drm-memory-<str>^shared:    <uint> [KiB|MiB]
> +- drm-memory-<str>^resident:  <uint> [KiB|MiB]
> +- drm-memory-<str>^purgeable: <uint> [KiB|MiB]
> +- drm-memory-<str>^active:    <uint> [KiB|MiB]

What exactly does size/shared/active mean here?

If it means what I think it does I don't see how TTM based drivers 
should track that in the first place.

Christian.

> +
> +Resident category is identical to the drm-memory-<str> key and two should be
> +mutually exclusive.
> +
> +TODO more description text...
> +
>   - drm-cycles-<str> <uint>
>   
>   Engine identifier string must be the same as the one specified in the
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 37b4f76a5191..e202f79e816d 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -42,6 +42,7 @@
>   #include <drm/drm_client.h>
>   #include <drm/drm_drv.h>
>   #include <drm/drm_file.h>
> +#include <drm/drm_gem.h>
>   #include <drm/drm_print.h>
>   
>   #include "drm_crtc_internal.h"
> @@ -871,6 +872,54 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
>   }
>   EXPORT_SYMBOL(drm_send_event);
>   
> +static void
> +print_stat(struct drm_printer *p, const char *stat, const char *region, u64 sz)
> +{
> +	const char *units[] = {"", " KiB", " MiB"};
> +	unsigned int u;
> +
> +	if (sz == ~0ull) /* Not supported by the driver. */
> +		return;
> +
> +	for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> +		if (sz < SZ_1K)
> +			break;
> +		sz = div_u64(sz, SZ_1K);
> +	}
> +
> +	drm_printf(p, "drm-memory-%s^%s:\t%llu%s\n",
> +		   region, stat, sz, units[u]);
> +}
> +
> +static void print_memory_stats(struct drm_printer *p, struct drm_file *file)
> +{
> +	struct drm_device *dev = file->minor->dev;
> +	struct drm_fdinfo_memory_stat *stats;
> +	unsigned int num, i;
> +	char **regions;
> +
> +	regions = dev->driver->query_fdinfo_memory_regions(dev, &num);
> +
> +	stats = kcalloc(num, sizeof(*stats), GFP_KERNEL);
> +	if (!stats)
> +		return;
> +
> +	dev->driver->query_fdinfo_memory_stats(file, stats);
> +
> +	for (i = 0; i < num; i++) {
> +		if (!regions[i]) /* Allow sparse name arrays. */
> +			continue;
> +
> +		print_stat(p, "size", regions[i], stats[i].size);
> +		print_stat(p, "shared", regions[i], stats[i].shared);
> +		print_stat(p, "resident", regions[i], stats[i].resident);
> +		print_stat(p, "purgeable", regions[i], stats[i].purgeable);
> +		print_stat(p, "active", regions[i], stats[i].active);
> +	}
> +
> +	kfree(stats);
> +}
> +
>   /**
>    * drm_show_fdinfo - helper for drm file fops
>    * @seq_file: output stream
> @@ -900,6 +949,9 @@ void drm_show_fdinfo(struct seq_file *m, struct file *f)
>   
>   	if (dev->driver->show_fdinfo)
>   		dev->driver->show_fdinfo(&p, file);
> +
> +	if (dev->driver->query_fdinfo_memory_regions)
> +		print_memory_stats(&p, file);
>   }
>   EXPORT_SYMBOL(drm_show_fdinfo);
>   
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 89e2706cac56..ccc1cd98d2aa 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -35,6 +35,7 @@
>   #include <drm/drm_device.h>
>   
>   struct drm_file;
> +struct drm_fdinfo_memory_stat;
>   struct drm_gem_object;
>   struct drm_master;
>   struct drm_minor;
> @@ -408,6 +409,12 @@ struct drm_driver {
>   	 */
>   	void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
>   
> +	char ** (*query_fdinfo_memory_regions)(struct drm_device *dev,
> +					       unsigned int *num);
> +
> +	void (*query_fdinfo_memory_stats)(struct drm_file *f,
> +					  struct drm_fdinfo_memory_stat *stat);
> +
>   	/** @major: driver major number */
>   	int major;
>   	/** @minor: driver minor number */
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 7d9b3c65cbc1..00d48beeac5c 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -375,6 +375,14 @@ struct drm_file {
>   #endif
>   };
>   
> +struct drm_fdinfo_memory_stat {
> +	u64 size;
> +	u64 shared;
> +	u64 resident;
> +	u64 purgeable;
> +	u64 active;
> +};
> +
>   /**
>    * drm_is_primary_client - is this an open file of the primary node
>    * @file_priv: DRM file