[Intel-gfx] [PATCH i-g-t 1/3] lib/igt_drm_fdinfo: Parse memory usage
From: Tvrtko Ursulin Add parsing and memory storage for the memory usage related fdinfo stats. Uses the same approach as the engine utilization code by either auto- discovering different memory regions, or allowing for the caller to pass in a map with predefined index to name relationship. v2: * Fix s-o-b and satisfy kernel checkpatch. (Kamil) Co-developed-by: Rob Clark Signed-off-by: Rob Clark Signed-off-by: Tvrtko Ursulin Cc: Kamil Konieczny --- lib/igt_drm_clients.c | 3 +- lib/igt_drm_fdinfo.c| 143 ++-- lib/igt_drm_fdinfo.h| 24 ++- tests/i915/drm_fdinfo.c | 8 +-- tools/intel_gpu_top.c | 2 +- 5 files changed, 166 insertions(+), 14 deletions(-) diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c index f0294ba81c42..fdea42752a81 100644 --- a/lib/igt_drm_clients.c +++ b/lib/igt_drm_clients.c @@ -491,7 +491,8 @@ igt_drm_clients_scan(struct igt_drm_clients *clients, if (!__igt_parse_drm_fdinfo(dirfd(fdinfo_dir), fdinfo_dent->d_name, &info, - name_map, map_entries)) + name_map, map_entries, + NULL, 0)) continue; if (filter_client && !filter_client(clients, &info)) diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c index b5f8a8679a71..f5a5b8e19dc3 100644 --- a/lib/igt_drm_fdinfo.c +++ b/lib/igt_drm_fdinfo.c @@ -124,13 +124,82 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen) return *p ? p : NULL; } +static int parse_region(char *line, struct drm_client_fdinfo *info, + size_t prefix_len, + const char **region_map, unsigned int region_entries, + uint64_t *val) +{ + char *name, *p, *unit = NULL; + ssize_t name_len; + int found = -1; + unsigned int i; + + p = index(line, ':'); + if (!p || p == line) + return -1; + + name_len = p - line - prefix_len; + if (name_len < 1) + return -1; + + name = line + prefix_len; + + if (region_map) { + for (i = 0; i < region_entries; i++) { + if (!strncmp(name, region_map[i], name_len)) { + found = i; + break; + } + } + } else { + for (i = 0; i < info->num_regions; i++) { + if (!strncmp(name, info->region_names[i], name_len)) { + found = i; + break; + } + } + + if (found < 0) { + assert((info->num_regions + 1) < ARRAY_SIZE(info->region_names)); + assert((strlen(name) + 1) < sizeof(info->region_names[0])); + strncpy(info->region_names[info->num_regions], name, name_len); + found = info->num_regions; + } + } + + if (found < 0) + goto out; + + while (*++p && isspace(*p)) + ; + *val = strtoull(p, NULL, 10); + + p = index(p, ' '); + if (!p) + goto out; + + unit = ++p; + if (!strcmp(unit, "KiB")) { + *val *= 1024; + } else if (!strcmp(unit, "MiB")) { + *val *= 1024 * 1024; + } else if (!strcmp(unit, "GiB")) { + *val *= 1024 * 1024 * 1024; + } + +out: + return found; +} + unsigned int __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info, - const char **name_map, unsigned int map_entries) + const char **name_map, unsigned int map_entries, + const char **region_map, unsigned int region_entries) { + bool regions_found[DRM_CLIENT_FDINFO_MAX_REGIONS] = { }; + unsigned int good = 0, num_capacity = 0; char buf[4096], *_buf = buf; char *l, *ctx = NULL; - unsigned int good = 0, num_capacity = 0; size_t count; count = read_fdinfo(buf, sizeof(buf), dir, fd); @@ -173,18 +242,79 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info, info->capacity[idx] = val; num_capacity++; } + } else if (!strncmp(l, "drm-total-", 10)) { + idx = parse_region(l, info, strlen("drm-total-"), + region_map, region_entries, &val); + if (idx >= 0) { + info->region_mem[idx].total = val; + if (!regions_found[idx]) { +
[Intel-gfx] [PATCH i-g-t 1/3] lib/igt_drm_fdinfo: Parse memory usage
From: Tvrtko Ursulin Add parsing and memory storage for the memory usage related fdinfo stats. Uses the same approach as the engine utilization code by either auto- discovering different memory regions, or allowing for the caller to pass in a map with predefined index to name relationship. Co-developed-by: Rob Clark Signed-off-by: Tvrtko Ursulin --- lib/igt_drm_clients.c | 3 +- lib/igt_drm_fdinfo.c| 142 ++-- lib/igt_drm_fdinfo.h| 24 ++- tests/i915/drm_fdinfo.c | 8 +-- tools/intel_gpu_top.c | 2 +- 5 files changed, 165 insertions(+), 14 deletions(-) diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c index f0294ba81c42..fdea42752a81 100644 --- a/lib/igt_drm_clients.c +++ b/lib/igt_drm_clients.c @@ -491,7 +491,8 @@ igt_drm_clients_scan(struct igt_drm_clients *clients, if (!__igt_parse_drm_fdinfo(dirfd(fdinfo_dir), fdinfo_dent->d_name, &info, - name_map, map_entries)) + name_map, map_entries, + NULL, 0)) continue; if (filter_client && !filter_client(clients, &info)) diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c index b5f8a8679a71..d08632dfb690 100644 --- a/lib/igt_drm_fdinfo.c +++ b/lib/igt_drm_fdinfo.c @@ -124,13 +124,81 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen) return *p ? p : NULL; } +static int parse_region(char *line, struct drm_client_fdinfo *info, + size_t prefix_len, + const char **region_map, unsigned int region_entries, + uint64_t *val) +{ + char *name, *p, *unit = NULL; + ssize_t name_len; + int found = -1; + unsigned int i; + + p = index(line, ':'); + if (!p || p == line) + return -1; + + name_len = p - line - prefix_len; + if (name_len < 1) + return -1; + + name = line + prefix_len; + + if (region_map) { + for (i = 0; i < region_entries; i++) { + if (!strncmp(name, region_map[i], name_len)) { + found = i; + break; + } + } + } else { + for (i = 0; i < info->num_regions; i++) { + if (!strncmp(name, info->region_names[i], name_len)) { + found = i; + break; + } + } + + if (found < 0) { + assert((info->num_regions + 1) < ARRAY_SIZE(info->region_names)); + assert((strlen(name) + 1) < sizeof(info->region_names[0])); + strncpy(info->region_names[info->num_regions], name, name_len); + found = info->num_regions; + } + } + + if (found < 0) + goto out; + + while (*++p && isspace(*p)); + *val = strtoull(p, NULL, 10); + + p = index(p, ' '); + if (!p) + goto out; + + unit = ++p; + if (!strcmp(unit, "KiB")) { + *val *= 1024; + } else if (!strcmp(unit, "MiB")) { + *val *= 1024 * 1024; + } else if (!strcmp(unit, "GiB")) { + *val *= 1024 * 1024 * 1024; + } + +out: + return found; +} + unsigned int __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info, - const char **name_map, unsigned int map_entries) + const char **name_map, unsigned int map_entries, + const char **region_map, unsigned int region_entries) { + bool regions_found[DRM_CLIENT_FDINFO_MAX_REGIONS] = { }; + unsigned int good = 0, num_capacity = 0; char buf[4096], *_buf = buf; char *l, *ctx = NULL; - unsigned int good = 0, num_capacity = 0; size_t count; count = read_fdinfo(buf, sizeof(buf), dir, fd); @@ -173,18 +241,79 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info, info->capacity[idx] = val; num_capacity++; } + } else if (!strncmp(l, "drm-total-", 10)) { + idx = parse_region(l, info, strlen("drm-total-"), + region_map, region_entries, &val); + if (idx >= 0) { + info->region_mem[idx].total = val; + if (!regions_found[idx]) { + info->num_regions++; + regions_found[idx] = true
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_drm_fdinfo: Parse memory usage
Hi Tvrtko, please check your patches with Linux kernel script checkpatch.pl Some of it's warnings seems strange, like: WARNING: Co-developed-by and Signed-off-by: name/email do not match #12: Co-developed-by: Rob Clark Signed-off-by: Tvrtko Ursulin On 2023-07-05 at 17:31:03 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Add parsing and memory storage for the memory usage related fdinfo stats. > > Uses the same approach as the engine utilization code by either auto- > discovering different memory regions, or allowing for the caller to pass > in a map with predefined index to name relationship. > > Co-developed-by: Rob Clark > Signed-off-by: Tvrtko Ursulin > --- > lib/igt_drm_clients.c | 3 +- > lib/igt_drm_fdinfo.c| 142 ++-- > lib/igt_drm_fdinfo.h| 24 ++- > tests/i915/drm_fdinfo.c | 8 +-- > tools/intel_gpu_top.c | 2 +- > 5 files changed, 165 insertions(+), 14 deletions(-) > > diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c > index f0294ba81c42..fdea42752a81 100644 > --- a/lib/igt_drm_clients.c > +++ b/lib/igt_drm_clients.c > @@ -491,7 +491,8 @@ igt_drm_clients_scan(struct igt_drm_clients *clients, > > if (!__igt_parse_drm_fdinfo(dirfd(fdinfo_dir), > fdinfo_dent->d_name, &info, > - name_map, map_entries)) > + name_map, map_entries, > + NULL, 0)) > continue; > > if (filter_client && !filter_client(clients, &info)) > diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c > index b5f8a8679a71..d08632dfb690 100644 > --- a/lib/igt_drm_fdinfo.c > +++ b/lib/igt_drm_fdinfo.c > @@ -124,13 +124,81 @@ static const char *find_kv(const char *buf, const char > *key, size_t keylen) > return *p ? p : NULL; > } > > +static int parse_region(char *line, struct drm_client_fdinfo *info, > + size_t prefix_len, > + const char **region_map, unsigned int region_entries, > + uint64_t *val) > +{ > + char *name, *p, *unit = NULL; > + ssize_t name_len; > + int found = -1; > + unsigned int i; > + > + p = index(line, ':'); > + if (!p || p == line) > + return -1; > + > + name_len = p - line - prefix_len; > + if (name_len < 1) > + return -1; > + > + name = line + prefix_len; > + > + if (region_map) { > + for (i = 0; i < region_entries; i++) { > + if (!strncmp(name, region_map[i], name_len)) { > + found = i; > + break; > + } > + } > + } else { > + for (i = 0; i < info->num_regions; i++) { > + if (!strncmp(name, info->region_names[i], name_len)) { > + found = i; > + break; > + } > + } > + > + if (found < 0) { > + assert((info->num_regions + 1) < > ARRAY_SIZE(info->region_names)); > + assert((strlen(name) + 1) < > sizeof(info->region_names[0])); > + strncpy(info->region_names[info->num_regions], name, > name_len); > + found = info->num_regions; > + } > + } > + > + if (found < 0) > + goto out; > + > + while (*++p && isspace(*p)); -- ^ According to checkpatch: while (*++p && isspace(*p)) ; > + *val = strtoull(p, NULL, 10); > + > + p = index(p, ' '); > + if (!p) > + goto out; > + > + unit = ++p; > + if (!strcmp(unit, "KiB")) { > + *val *= 1024; > + } else if (!strcmp(unit, "MiB")) { --- ^^ No need for separate 'else': if (!strcmp(unit, "MiB")) { > + *val *= 1024 * 1024; > + } else if (!strcmp(unit, "GiB")) { --- ^^ Same here. Regards, Kamil > + *val *= 1024 * 1024 * 1024; > + } > + > +out: > + return found; > +} > + > unsigned int > __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo > *info, > -const char **name_map, unsigned int map_entries) > +const char **name_map, unsigned int map_entries, > +const char **region_map, unsigned int region_entries) > { > + bool regions_found[DRM_CLIENT_FDINFO_MAX_REGIONS] = { }; > + unsigned int good = 0, num_capacity = 0; > char buf[4096], *_buf = buf; > char *l, *ctx = NULL; > - unsigned int good = 0, num_capacity = 0; > size_t count; > > count = read_fdinfo(buf, sizeof(buf), dir, fd); > @@ -173,18 +241,79 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struc
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_drm_fdinfo: Parse memory usage
On 26/07/2023 18:28, Kamil Konieczny wrote: Hi Tvrtko, please check your patches with Linux kernel script checkpatch.pl Some of it's warnings seems strange, like: WARNING: Co-developed-by and Signed-off-by: name/email do not match #12: Co-developed-by: Rob Clark Signed-off-by: Tvrtko Ursulin Done. On 2023-07-05 at 17:31:03 +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Add parsing and memory storage for the memory usage related fdinfo stats. Uses the same approach as the engine utilization code by either auto- discovering different memory regions, or allowing for the caller to pass in a map with predefined index to name relationship. Co-developed-by: Rob Clark Signed-off-by: Tvrtko Ursulin --- lib/igt_drm_clients.c | 3 +- lib/igt_drm_fdinfo.c| 142 ++-- lib/igt_drm_fdinfo.h| 24 ++- tests/i915/drm_fdinfo.c | 8 +-- tools/intel_gpu_top.c | 2 +- 5 files changed, 165 insertions(+), 14 deletions(-) diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c index f0294ba81c42..fdea42752a81 100644 --- a/lib/igt_drm_clients.c +++ b/lib/igt_drm_clients.c @@ -491,7 +491,8 @@ igt_drm_clients_scan(struct igt_drm_clients *clients, if (!__igt_parse_drm_fdinfo(dirfd(fdinfo_dir), fdinfo_dent->d_name, &info, - name_map, map_entries)) + name_map, map_entries, + NULL, 0)) continue; if (filter_client && !filter_client(clients, &info)) diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c index b5f8a8679a71..d08632dfb690 100644 --- a/lib/igt_drm_fdinfo.c +++ b/lib/igt_drm_fdinfo.c @@ -124,13 +124,81 @@ static const char *find_kv(const char *buf, const char *key, size_t keylen) return *p ? p : NULL; } +static int parse_region(char *line, struct drm_client_fdinfo *info, + size_t prefix_len, + const char **region_map, unsigned int region_entries, + uint64_t *val) +{ + char *name, *p, *unit = NULL; + ssize_t name_len; + int found = -1; + unsigned int i; + + p = index(line, ':'); + if (!p || p == line) + return -1; + + name_len = p - line - prefix_len; + if (name_len < 1) + return -1; + + name = line + prefix_len; + + if (region_map) { + for (i = 0; i < region_entries; i++) { + if (!strncmp(name, region_map[i], name_len)) { + found = i; + break; + } + } + } else { + for (i = 0; i < info->num_regions; i++) { + if (!strncmp(name, info->region_names[i], name_len)) { + found = i; + break; + } + } + + if (found < 0) { + assert((info->num_regions + 1) < ARRAY_SIZE(info->region_names)); + assert((strlen(name) + 1) < sizeof(info->region_names[0])); + strncpy(info->region_names[info->num_regions], name, name_len); + found = info->num_regions; + } + } + + if (found < 0) + goto out; + + while (*++p && isspace(*p)); -- ^ According to checkpatch: while (*++p && isspace(*p)) ; Done. + *val = strtoull(p, NULL, 10); + + p = index(p, ' '); + if (!p) + goto out; + + unit = ++p; + if (!strcmp(unit, "KiB")) { + *val *= 1024; + } else if (!strcmp(unit, "MiB")) { --- ^^ No need for separate 'else': if (!strcmp(unit, "MiB")) { + *val *= 1024 * 1024; + } else if (!strcmp(unit, "GiB")) { --- ^^ Same here. We don't want to run strcmp multiple times for no reason so I opted to leave as is. Regards, Tvrtko Regards, Kamil + *val *= 1024 * 1024 * 1024; + } + +out: + return found; +} + unsigned int __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info, - const char **name_map, unsigned int map_entries) + const char **name_map, unsigned int map_entries, + const char **region_map, unsigned int region_entries) { + bool regions_found[DRM_CLIENT_FDINFO_MAX_REGIONS] = { }; + unsigned int good = 0, num_capacity = 0; char buf[4096], *_buf = buf; char *l, *ctx = NULL; - unsigned int good = 0, num_capacity = 0; size_t count; count = read_fdinfo(buf, sizeof(buf), dir, fd); @@ -173,18 +241,79 @@ __igt_pars
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_drm_fdinfo: Parse memory usage
Hi Tvrtko, On 2023-07-27 at 10:20:23 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Add parsing and memory storage for the memory usage related fdinfo stats. > > Uses the same approach as the engine utilization code by either auto- > discovering different memory regions, or allowing for the caller to pass > in a map with predefined index to name relationship. > > v2: > * Fix s-o-b and satisfy kernel checkpatch. (Kamil) > > Co-developed-by: Rob Clark > Signed-off-by: Rob Clark > Signed-off-by: Tvrtko Ursulin > Cc: Kamil Konieczny LGTM, Reviewed-by: Kamil Konieczny > --- > lib/igt_drm_clients.c | 3 +- > lib/igt_drm_fdinfo.c| 143 ++-- > lib/igt_drm_fdinfo.h| 24 ++- > tests/i915/drm_fdinfo.c | 8 +-- > tools/intel_gpu_top.c | 2 +- > 5 files changed, 166 insertions(+), 14 deletions(-) > > diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c > index f0294ba81c42..fdea42752a81 100644 > --- a/lib/igt_drm_clients.c > +++ b/lib/igt_drm_clients.c > @@ -491,7 +491,8 @@ igt_drm_clients_scan(struct igt_drm_clients *clients, > > if (!__igt_parse_drm_fdinfo(dirfd(fdinfo_dir), > fdinfo_dent->d_name, &info, > - name_map, map_entries)) > + name_map, map_entries, > + NULL, 0)) > continue; > > if (filter_client && !filter_client(clients, &info)) > diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c > index b5f8a8679a71..f5a5b8e19dc3 100644 > --- a/lib/igt_drm_fdinfo.c > +++ b/lib/igt_drm_fdinfo.c > @@ -124,13 +124,82 @@ static const char *find_kv(const char *buf, const char > *key, size_t keylen) > return *p ? p : NULL; > } > > +static int parse_region(char *line, struct drm_client_fdinfo *info, > + size_t prefix_len, > + const char **region_map, unsigned int region_entries, > + uint64_t *val) > +{ > + char *name, *p, *unit = NULL; > + ssize_t name_len; > + int found = -1; > + unsigned int i; > + > + p = index(line, ':'); > + if (!p || p == line) > + return -1; > + > + name_len = p - line - prefix_len; > + if (name_len < 1) > + return -1; > + > + name = line + prefix_len; > + > + if (region_map) { > + for (i = 0; i < region_entries; i++) { > + if (!strncmp(name, region_map[i], name_len)) { > + found = i; > + break; > + } > + } > + } else { > + for (i = 0; i < info->num_regions; i++) { > + if (!strncmp(name, info->region_names[i], name_len)) { > + found = i; > + break; > + } > + } > + > + if (found < 0) { > + assert((info->num_regions + 1) < > ARRAY_SIZE(info->region_names)); > + assert((strlen(name) + 1) < > sizeof(info->region_names[0])); > + strncpy(info->region_names[info->num_regions], name, > name_len); > + found = info->num_regions; > + } > + } > + > + if (found < 0) > + goto out; > + > + while (*++p && isspace(*p)) > + ; > + *val = strtoull(p, NULL, 10); > + > + p = index(p, ' '); > + if (!p) > + goto out; > + > + unit = ++p; > + if (!strcmp(unit, "KiB")) { > + *val *= 1024; > + } else if (!strcmp(unit, "MiB")) { > + *val *= 1024 * 1024; > + } else if (!strcmp(unit, "GiB")) { > + *val *= 1024 * 1024 * 1024; > + } > + > +out: > + return found; > +} > + > unsigned int > __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo > *info, > -const char **name_map, unsigned int map_entries) > +const char **name_map, unsigned int map_entries, > +const char **region_map, unsigned int region_entries) > { > + bool regions_found[DRM_CLIENT_FDINFO_MAX_REGIONS] = { }; > + unsigned int good = 0, num_capacity = 0; > char buf[4096], *_buf = buf; > char *l, *ctx = NULL; > - unsigned int good = 0, num_capacity = 0; > size_t count; > > count = read_fdinfo(buf, sizeof(buf), dir, fd); > @@ -173,18 +242,79 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct > drm_client_fdinfo *info, > info->capacity[idx] = val; > num_capacity++; > } > + } else if (!strncmp(l, "drm-total-", 10)) { > + idx = parse_region(l, info, strlen("drm-total-"), > +