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

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Tue Apr 18 08:59:55 UTC 2023


On 17/04/2023 20:39, Rob Clark wrote:
> On Mon, Apr 17, 2023 at 8:56 AM Tvrtko Ursulin
> <tvrtko.ursulin at linux.intel.com> wrote:
>>
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Add support to dump GEM stats to fdinfo.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at 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.
> 
> this doesn't solve the problem that led me to drm-$CATEGORY-memory... ;-)

Could you explain or remind me with a link to a previous explanation?

What tool barfs on it and how? Spirit of drm-usage-stats.pl is that for 
both drm-engine-<str>: and drm-memory-<str>: generic userspace is 
supposed to take the whole <std> as opaque and just enumerate all it finds.

Gputop manages to do that with engines names it knows _nothing_ about. 
If it worked with memory regions it would just show the list of new 
regions as for example "system^resident", "system^active". Then tools 
can be extended to understand it better and provide a sub-breakdown if 
wanted.

Ugly not, we can change from caret to something nicer, unless I am 
missing something it works, no?

Regards,

Tvrtko

> 
> (also, it is IMHO rather ugly)
> 
> BR,
> -R
> 
>>   - 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]
>> +
>> +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
>> --
>> 2.37.2
>>


More information about the Intel-gfx mailing list