[igt-dev] [PATCH igt 0/2] gputop memory information

[igt-dev] [PATCH igt 0/2] gputop memory information

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

This applies on top of Tvrtko's series to add a vendor agnostic
gputop[1].  And uses the utilizes the memory related fdinfo props
added by [2].

If anyone has a better idea how to display the memory usage, I'm
all ears.. but the current thing is a tight squeeze to keep the
text columns under half or 80 col.  (It ended up at 41 col.)

[1] https://patchwork.freedesktop.org/series/102175/
[2] https://patchwork.freedesktop.org/series/116217/

Rob Clark (2):
  lib/igt_drm_fdinfo: Parse memory usage
  gputop: Add memory information

 lib/igt_drm_clients.c |  1 +
 lib/igt_drm_clients.h |  3 +++
 lib/igt_drm_fdinfo.c  | 39 +++++++++++++++++++++++++++++++++++++++
 lib/igt_drm_fdinfo.h  |  9 +++++++++
 tools/gputop.c        | 24 ++++++++++++++++++++++--
 5 files changed, 74 insertions(+), 2 deletions(-)

-- 
2.39.2

[igt-dev] [PATCH igt 1/2] lib/igt_drm_fdinfo: Parse memory usage

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Add parsing for the memory usage related fdinfo stats.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 lib/igt_drm_fdinfo.c | 39 +++++++++++++++++++++++++++++++++++++++
 lib/igt_drm_fdinfo.h |  9 +++++++++
 2 files changed, 48 insertions(+)

diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
index b850d221..6269e166 100644
--- a/lib/igt_drm_fdinfo.c
+++ b/lib/igt_drm_fdinfo.c
@@ -124,6 +124,34 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen)
 	return *p ? p : NULL;
 }
 
+static size_t find_mem_kv(const char *buf, const char *key)
+{
+	const char *val = find_kv(buf, key, strlen(key));
+	char *p, *unit = NULL;
+	size_t sz;
+
+	if (!val)
+		return 0;
+
+	sz = atoi(val);
+
+	p = index(val, ' ');
+	if (!p)
+		return sz;
+
+	unit = ++p;
+
+	if (!strcmp(unit, "KiB")) {
+		sz *= 1024;
+	} else if (!strcmp(unit, "MiB")) {
+		sz *= 1024 * 1024;
+	} else if (!strcmp(unit, "GiB")) {
+		sz *= 1024 * 1024 * 1024;
+	}
+
+	return sz;
+}
+
 unsigned int
 __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
 		       const char **name_map, unsigned int map_entries)
@@ -140,6 +168,7 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
 	while ((l = strtok_r(_buf, "\n", &ctx))) {
 		uint64_t val = 0;
 		const char *v;
+		size_t sz;
 		int idx;
 
 		_buf = NULL;
@@ -173,6 +202,16 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
 				info->capacity[idx] = val;
 				num_capacity++;
 			}
+		} else if ((sz = find_mem_kv(l, "drm-shared-memory"))) {
+			info->mem.shared = sz;
+		} else if ((sz = find_mem_kv(l, "drm-private-memory"))) {
+			info->mem.private = sz;
+		} else if ((sz = find_mem_kv(l, "drm-resident-memory"))) {
+			info->mem.resident = sz;
+		} else if ((sz = find_mem_kv(l, "drm-purgeable-memory"))) {
+			info->mem.purgeable = sz;
+		} else if ((sz = find_mem_kv(l, "drm-active-memory"))) {
+			info->mem.active = sz;
 		}
 	}
 
diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
index 6284e05e..dd4bdd54 100644
--- a/lib/igt_drm_fdinfo.h
+++ b/lib/igt_drm_fdinfo.h
@@ -32,6 +32,14 @@
 
 #define DRM_CLIENT_FDINFO_MAX_ENGINES 16
 
+struct drm_client_meminfo {
+	size_t shared;
+	size_t private;
+	size_t resident;
+	size_t purgeable;
+	size_t active;
+};
+
 struct drm_client_fdinfo {
 	char driver[128];
 	char pdev[128];
@@ -42,6 +50,7 @@ struct drm_client_fdinfo {
 	unsigned int capacity[DRM_CLIENT_FDINFO_MAX_ENGINES];
 	char names[DRM_CLIENT_FDINFO_MAX_ENGINES][256];
 	uint64_t busy[DRM_CLIENT_FDINFO_MAX_ENGINES];
+	struct drm_client_meminfo mem;
 };
 
 /**
-- 
2.39.2

[igt-dev] [PATCH igt 2/2] gputop: Add memory information

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 lib/igt_drm_clients.c |  1 +
 lib/igt_drm_clients.h |  3 +++
 tools/gputop.c        | 24 ++++++++++++++++++++++--
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c
index b3eda39c..9cd5cc5d 100644
--- a/lib/igt_drm_clients.c
+++ b/lib/igt_drm_clients.c
@@ -109,6 +109,7 @@ igt_drm_client_update(struct igt_drm_client *c, unsigned int pid, char *name,
 		c->last[i] = info->busy[i];
 	}
 
+	c->mem = info->mem;
 	c->samples++;
 	c->status = IGT_DRM_CLIENT_ALIVE;
 }
diff --git a/lib/igt_drm_clients.h b/lib/igt_drm_clients.h
index df8022d4..1a631d20 100644
--- a/lib/igt_drm_clients.h
+++ b/lib/igt_drm_clients.h
@@ -8,6 +8,8 @@
 
 #include <stdint.h>
 
+#include "lib/igt_drm_fdinfo.h"
+
 /**
  * SECTION:igt_drm_clients
  * @short_description: Parsing driver exposed fdinfo to track DRM clients
@@ -56,6 +58,7 @@ struct igt_drm_client {
 	unsigned long last_runtime; /* Aggregate busyness on all engines since previous scan. */
 	unsigned long *val; /* Array of engine busyness data, relative to previous scan. */
 	uint64_t *last; /* Array of engine busyness data as parsed from fdinfo. */
+	struct drm_client_meminfo mem;
 };
 
 struct igt_drm_clients {
diff --git a/tools/gputop.c b/tools/gputop.c
index d259cac1..76677e7d 100644
--- a/tools/gputop.c
+++ b/tools/gputop.c
@@ -28,6 +28,7 @@
 
 #include "igt_drm_clients.h"
 #include "igt_drm_fdinfo.h"
+#include "drmtest.h"
 
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
 
@@ -67,7 +68,7 @@ static int
 print_client_header(struct igt_drm_client *c, int lines, int con_w, int con_h,
 		    int *engine_w)
 {
-	const char *pidname = "    PID               NAME ";
+	const char *pidname = "    PID    MEM ACTIV              NAME ";
 	int ret, len = strlen(pidname);
 
 	if (lines++ >= con_h || len >= con_w)
@@ -118,6 +119,21 @@ newheader(const struct igt_drm_client *c, const struct igt_drm_client *pc)
 	return !pc || c->drm_minor != pc->drm_minor;
 }
 
+static void
+print_size(size_t sz)
+{
+	char units[] = {'B', 'K', 'M', 'G'};
+	unsigned u;
+
+	for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
+		if (sz < 1024)
+			break;
+		sz /= 1024;
+	}
+
+	printf("%4zd%c ", sz, units[u]);
+}
+
 static int
 print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
 	     double t, int lines, int con_w, int con_h,
@@ -138,7 +154,11 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
 
 	*prevc = c;
 
-	printf("%8u %17s ", c->pid, c->print_name);
+	printf("%8u ", c->pid);
+	print_size(c->mem.shared + c->mem.private);
+	print_size(c->mem.active);
+	printf("%17s ", c->print_name);
+
 	lines++;
 
 	for (i = 0; c->samples > 1 && i <= c->engines->max_engine_id; i++) {
-- 
2.39.2

[igt-dev] ✗ Fi.CI.BUILD: failure for gputop memory information

[Patchwork]

== Series Details ==

Series: gputop memory information
URL   : https://patchwork.freedesktop.org/series/116236/
State : failure

== Summary ==

Applying: lib/igt_drm_fdinfo: Parse memory usage
Using index info to reconstruct a base tree...
M	lib/igt_drm_fdinfo.c
M	lib/igt_drm_fdinfo.h
Falling back to patching base and 3-way merge...
Auto-merging lib/igt_drm_fdinfo.h
Auto-merging lib/igt_drm_fdinfo.c
Applying: gputop: Add memory information
Using index info to reconstruct a base tree...
A	lib/igt_drm_clients.c
A	lib/igt_drm_clients.h
A	tools/gputop.c
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): tools/gputop.c deleted in HEAD and modified in gputop: Add memory information. Version gputop: Add memory information of tools/gputop.c left in tree.
CONFLICT (modify/delete): lib/igt_drm_clients.h deleted in HEAD and modified in gputop: Add memory information. Version gputop: Add memory information of lib/igt_drm_clients.h left in tree.
CONFLICT (modify/delete): lib/igt_drm_clients.c deleted in HEAD and modified in gputop: Add memory information. Version gputop: Add memory information of lib/igt_drm_clients.c left in tree.
Patch failed at 0002 gputop: Add memory information
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [igt-dev] [PATCH igt 1/2] lib/igt_drm_fdinfo: Parse memory usage

[Tvrtko Ursulin]

On 07/04/2023 22:56, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Add parsing for the memory usage related fdinfo stats.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   lib/igt_drm_fdinfo.c | 39 +++++++++++++++++++++++++++++++++++++++
>   lib/igt_drm_fdinfo.h |  9 +++++++++
>   2 files changed, 48 insertions(+)
> 
> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> index b850d221..6269e166 100644
> --- a/lib/igt_drm_fdinfo.c
> +++ b/lib/igt_drm_fdinfo.c
> @@ -124,6 +124,34 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen)
>   	return *p ? p : NULL;
>   }
>   
> +static size_t find_mem_kv(const char *buf, const char *key)
> +{
> +	const char *val = find_kv(buf, key, strlen(key));

If you were asking yourself why I made strlen an argument to find_kv I 
have to admit I micro-optimized it a bit based on profiling. Sad fact is 
hunting for drm fdinfo in procfs sucks and gputop uses more CPU time 
than top, to even display less data. More processes with more open files 
there are, even when the 99.9% are not DRM, more taxing it is.

So I'd suggest sticking to the micro-optimized pattern. :(

> +	char *p, *unit = NULL;
> +	size_t sz;
> +
> +	if (!val)
> +		return 0;
> +
> +	sz = atoi(val);

Maybe atol and unsigned long instead of size_t would be better for the 
counts throughout? Or uint64_t?

> +
> +	p = index(val, ' ');

Ok, or can use the flexible method from find_kv which skips any amount 
of any whitespace.

> +	if (!p)
> +		return sz;
> +
> +	unit = ++p;
> +
> +	if (!strcmp(unit, "KiB")) {
> +		sz *= 1024;
> +	} else if (!strcmp(unit, "MiB")) {
> +		sz *= 1024 * 1024;
> +	} else if (!strcmp(unit, "GiB")) {
> +		sz *= 1024 * 1024 * 1024;
> +	}
> +
> +	return sz;
> +}
> +
>   unsigned int
>   __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>   		       const char **name_map, unsigned int map_entries)
> @@ -140,6 +168,7 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>   	while ((l = strtok_r(_buf, "\n", &ctx))) {
>   		uint64_t val = 0;
>   		const char *v;
> +		size_t sz;
>   		int idx;
>   
>   		_buf = NULL;
> @@ -173,6 +202,16 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>   				info->capacity[idx] = val;
>   				num_capacity++;
>   			}
> +		} else if ((sz = find_mem_kv(l, "drm-shared-memory"))) {
> +			info->mem.shared = sz;
> +		} else if ((sz = find_mem_kv(l, "drm-private-memory"))) {
> +			info->mem.private = sz;
> +		} else if ((sz = find_mem_kv(l, "drm-resident-memory"))) {
> +			info->mem.resident = sz;
> +		} else if ((sz = find_mem_kv(l, "drm-purgeable-memory"))) {
> +			info->mem.purgeable = sz;
> +		} else if ((sz = find_mem_kv(l, "drm-active-memory"))) {
> +			info->mem.active = sz;

Okay there's an open on key naming and if we went with drm-memory-... 
this could be done with just one strncmp on non-matching lines. Depends 
on the open.

>   		}
>   	}
>   
> diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
> index 6284e05e..dd4bdd54 100644
> --- a/lib/igt_drm_fdinfo.h
> +++ b/lib/igt_drm_fdinfo.h
> @@ -32,6 +32,14 @@
>   
>   #define DRM_CLIENT_FDINFO_MAX_ENGINES 16
>   
> +struct drm_client_meminfo {
> +	size_t shared;
> +	size_t private;
> +	size_t resident;
> +	size_t purgeable;
> +	size_t active;
> +};
> +
>   struct drm_client_fdinfo {
>   	char driver[128];
>   	char pdev[128];
> @@ -42,6 +50,7 @@ struct drm_client_fdinfo {
>   	unsigned int capacity[DRM_CLIENT_FDINFO_MAX_ENGINES];
>   	char names[DRM_CLIENT_FDINFO_MAX_ENGINES][256];
>   	uint64_t busy[DRM_CLIENT_FDINFO_MAX_ENGINES];
> +	struct drm_client_meminfo mem;
>   };
>   
>   /**

Regards,

Tvrtko

Re: [igt-dev] [PATCH igt 2/2] gputop: Add memory information

[Tvrtko Ursulin]

On 07/04/2023 22:56, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   lib/igt_drm_clients.c |  1 +
>   lib/igt_drm_clients.h |  3 +++
>   tools/gputop.c        | 24 ++++++++++++++++++++++--
>   3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c
> index b3eda39c..9cd5cc5d 100644
> --- a/lib/igt_drm_clients.c
> +++ b/lib/igt_drm_clients.c
> @@ -109,6 +109,7 @@ igt_drm_client_update(struct igt_drm_client *c, unsigned int pid, char *name,
>   		c->last[i] = info->busy[i];
>   	}
>   
> +	c->mem = info->mem;
>   	c->samples++;
>   	c->status = IGT_DRM_CLIENT_ALIVE;
>   }
> diff --git a/lib/igt_drm_clients.h b/lib/igt_drm_clients.h
> index df8022d4..1a631d20 100644
> --- a/lib/igt_drm_clients.h
> +++ b/lib/igt_drm_clients.h
> @@ -8,6 +8,8 @@
>   
>   #include <stdint.h>
>   
> +#include "lib/igt_drm_fdinfo.h"
> +
>   /**
>    * SECTION:igt_drm_clients
>    * @short_description: Parsing driver exposed fdinfo to track DRM clients
> @@ -56,6 +58,7 @@ struct igt_drm_client {
>   	unsigned long last_runtime; /* Aggregate busyness on all engines since previous scan. */
>   	unsigned long *val; /* Array of engine busyness data, relative to previous scan. */
>   	uint64_t *last; /* Array of engine busyness data as parsed from fdinfo. */
> +	struct drm_client_meminfo mem;
>   };
>   
>   struct igt_drm_clients {
> diff --git a/tools/gputop.c b/tools/gputop.c
> index d259cac1..76677e7d 100644
> --- a/tools/gputop.c
> +++ b/tools/gputop.c
> @@ -28,6 +28,7 @@
>   
>   #include "igt_drm_clients.h"
>   #include "igt_drm_fdinfo.h"
> +#include "drmtest.h"
>   
>   static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
>   
> @@ -67,7 +68,7 @@ static int
>   print_client_header(struct igt_drm_client *c, int lines, int con_w, int con_h,
>   		    int *engine_w)
>   {
> -	const char *pidname = "    PID               NAME ";
> +	const char *pidname = "    PID    MEM ACTIV              NAME ";

Odd information sandwich? :) I guess it ends up looking better with the 
current alignment. Perhaps I should swap pid and name. Or push name last 
like top(1).

Anyway, I am happy that you were able to plug this into my code. Means 
it may not be completely impenetrable.

>   	int ret, len = strlen(pidname);
>   
>   	if (lines++ >= con_h || len >= con_w)
> @@ -118,6 +119,21 @@ newheader(const struct igt_drm_client *c, const struct igt_drm_client *pc)
>   	return !pc || c->drm_minor != pc->drm_minor;
>   }
>   
> +static void
> +print_size(size_t sz)
> +{
> +	char units[] = {'B', 'K', 'M', 'G'};
> +	unsigned u;
> +
> +	for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> +		if (sz < 1024)
> +			break;
> +		sz /= 1024;
> +	}
> +
> +	printf("%4zd%c ", sz, units[u]);
> +}
> +
>   static int
>   print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
>   	     double t, int lines, int con_w, int con_h,
> @@ -138,7 +154,11 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
>   
>   	*prevc = c;
>   
> -	printf("%8u %17s ", c->pid, c->print_name);
> +	printf("%8u ", c->pid);
> +	print_size(c->mem.shared + c->mem.private);
> +	print_size(c->mem.active);

Active is more interesting than resident?

> +	printf("%17s ", c->print_name);
> +
>   	lines++;
>   
>   	for (i = 0; c->samples > 1 && i <= c->engines->max_engine_id; i++) {

Looks fine overall.

Regards,

Tvrtko

Re: [igt-dev] [PATCH igt 1/2] lib/igt_drm_fdinfo: Parse memory usage

[Rob Clark]

On Thu, Apr 13, 2023 at 8:19 AM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
>
> On 07/04/2023 22:56, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Add parsing for the memory usage related fdinfo stats.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >   lib/igt_drm_fdinfo.c | 39 +++++++++++++++++++++++++++++++++++++++
> >   lib/igt_drm_fdinfo.h |  9 +++++++++
> >   2 files changed, 48 insertions(+)
> >
> > diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> > index b850d221..6269e166 100644
> > --- a/lib/igt_drm_fdinfo.c
> > +++ b/lib/igt_drm_fdinfo.c
> > @@ -124,6 +124,34 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen)
> >       return *p ? p : NULL;
> >   }
> >
> > +static size_t find_mem_kv(const char *buf, const char *key)
> > +{
> > +     const char *val = find_kv(buf, key, strlen(key));
>
> If you were asking yourself why I made strlen an argument to find_kv I
> have to admit I micro-optimized it a bit based on profiling. Sad fact is
> hunting for drm fdinfo in procfs sucks and gputop uses more CPU time
> than top, to even display less data. More processes with more open files
> there are, even when the 99.9% are not DRM, more taxing it is.

Did a release build really not end up with it inlined??  If so I guess
we can make this a macro, but it seems a bit surprising.  This should
be a thing the compiler can figure out.

BR,
-R

> So I'd suggest sticking to the micro-optimized pattern. :(
>
> > +     char *p, *unit = NULL;
> > +     size_t sz;
> > +
> > +     if (!val)
> > +             return 0;
> > +
> > +     sz = atoi(val);
>
> Maybe atol and unsigned long instead of size_t would be better for the
> counts throughout? Or uint64_t?
>
> > +
> > +     p = index(val, ' ');
>
> Ok, or can use the flexible method from find_kv which skips any amount
> of any whitespace.
>
> > +     if (!p)
> > +             return sz;
> > +
> > +     unit = ++p;
> > +
> > +     if (!strcmp(unit, "KiB")) {
> > +             sz *= 1024;
> > +     } else if (!strcmp(unit, "MiB")) {
> > +             sz *= 1024 * 1024;
> > +     } else if (!strcmp(unit, "GiB")) {
> > +             sz *= 1024 * 1024 * 1024;
> > +     }
> > +
> > +     return sz;
> > +}
> > +
> >   unsigned int
> >   __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> >                      const char **name_map, unsigned int map_entries)
> > @@ -140,6 +168,7 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> >       while ((l = strtok_r(_buf, "\n", &ctx))) {
> >               uint64_t val = 0;
> >               const char *v;
> > +             size_t sz;
> >               int idx;
> >
> >               _buf = NULL;
> > @@ -173,6 +202,16 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> >                               info->capacity[idx] = val;
> >                               num_capacity++;
> >                       }
> > +             } else if ((sz = find_mem_kv(l, "drm-shared-memory"))) {
> > +                     info->mem.shared = sz;
> > +             } else if ((sz = find_mem_kv(l, "drm-private-memory"))) {
> > +                     info->mem.private = sz;
> > +             } else if ((sz = find_mem_kv(l, "drm-resident-memory"))) {
> > +                     info->mem.resident = sz;
> > +             } else if ((sz = find_mem_kv(l, "drm-purgeable-memory"))) {
> > +                     info->mem.purgeable = sz;
> > +             } else if ((sz = find_mem_kv(l, "drm-active-memory"))) {
> > +                     info->mem.active = sz;
>
> Okay there's an open on key naming and if we went with drm-memory-...
> this could be done with just one strncmp on non-matching lines. Depends
> on the open.
>
> >               }
> >       }
> >
> > diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
> > index 6284e05e..dd4bdd54 100644
> > --- a/lib/igt_drm_fdinfo.h
> > +++ b/lib/igt_drm_fdinfo.h
> > @@ -32,6 +32,14 @@
> >
> >   #define DRM_CLIENT_FDINFO_MAX_ENGINES 16
> >
> > +struct drm_client_meminfo {
> > +     size_t shared;
> > +     size_t private;
> > +     size_t resident;
> > +     size_t purgeable;
> > +     size_t active;
> > +};
> > +
> >   struct drm_client_fdinfo {
> >       char driver[128];
> >       char pdev[128];
> > @@ -42,6 +50,7 @@ struct drm_client_fdinfo {
> >       unsigned int capacity[DRM_CLIENT_FDINFO_MAX_ENGINES];
> >       char names[DRM_CLIENT_FDINFO_MAX_ENGINES][256];
> >       uint64_t busy[DRM_CLIENT_FDINFO_MAX_ENGINES];
> > +     struct drm_client_meminfo mem;
> >   };
> >
> >   /**
>
> Regards,
>
> Tvrtko

Re: [igt-dev] [PATCH igt 1/2] lib/igt_drm_fdinfo: Parse memory usage

[Tvrtko Ursulin]

On 13/04/2023 19:26, Rob Clark wrote:
> On Thu, Apr 13, 2023 at 8:19 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 07/04/2023 22:56, Rob Clark wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> Add parsing for the memory usage related fdinfo stats.
>>>
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> ---
>>>    lib/igt_drm_fdinfo.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>    lib/igt_drm_fdinfo.h |  9 +++++++++
>>>    2 files changed, 48 insertions(+)
>>>
>>> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>>> index b850d221..6269e166 100644
>>> --- a/lib/igt_drm_fdinfo.c
>>> +++ b/lib/igt_drm_fdinfo.c
>>> @@ -124,6 +124,34 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen)
>>>        return *p ? p : NULL;
>>>    }
>>>
>>> +static size_t find_mem_kv(const char *buf, const char *key)
>>> +{
>>> +     const char *val = find_kv(buf, key, strlen(key));
>>
>> If you were asking yourself why I made strlen an argument to find_kv I
>> have to admit I micro-optimized it a bit based on profiling. Sad fact is
>> hunting for drm fdinfo in procfs sucks and gputop uses more CPU time
>> than top, to even display less data. More processes with more open files
>> there are, even when the 99.9% are not DRM, more taxing it is.
> 
> Did a release build really not end up with it inlined??  If so I guess
> we can make this a macro, but it seems a bit surprising.  This should
> be a thing the compiler can figure out.

AFAIR it wasn't able to figure out to evaluate the strlen at compile 
time when inside find_kv. Pretty sure it was the default build 
(debugoptimized) but it was also more than a year ago now so I guess I'd 
need to repeat the experiment to be sure.

Regards,

Tvrtko

[igt-dev] ✗ Fi.CI.BUILD: failure for gputop memory information (rev2)

[Patchwork]

== Series Details ==

Series: gputop memory information (rev2)
URL   : https://patchwork.freedesktop.org/series/116236/
State : failure

== Summary ==

Applying: lib/igt_drm_fdinfo: Parse memory usage
Applying: gputop: Add memory information
Using index info to reconstruct a base tree...
M	lib/igt_drm_clients.c
M	lib/igt_drm_clients.h
M	tools/gputop.c
Falling back to patching base and 3-way merge...
Auto-merging tools/gputop.c
CONFLICT (content): Merge conflict in tools/gputop.c
Auto-merging lib/igt_drm_clients.h
Auto-merging lib/igt_drm_clients.c
Patch failed at 0002 gputop: Add memory information
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".