[PATCH i-g-t 01/12] lib/igt_drm_fdinfo: Stop passing key twice
Tvrtko Ursulin
tursulin at ursulin.net
Fri Apr 19 07:42:50 UTC 2024
On 05/04/2024 07:00, Lucas De Marchi wrote:
> Change strstartswith() so it also returns the length, which then can be
> used inside the branch when it matches. A good compiler shall optimize
> out the strlen call since the argument is a constant literal.
>
> With this, the find_kv() is not needed anymore and the difference with
> regard the other branches can be summarized with a new ignore_space()
> helper and the fact it matches the entire key by appending the ':'. The
> helper is added on top of the file so it can be reused later.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> lib/igt_drm_fdinfo.c | 87 +++++++++++++++++++++++---------------------
> 1 file changed, 45 insertions(+), 42 deletions(-)
>
> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> index 18dbb5d0b..bfc946f24 100644
> --- a/lib/igt_drm_fdinfo.c
> +++ b/lib/igt_drm_fdinfo.c
> @@ -53,6 +53,14 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
> return count > 0 ? count : 0;
> }
>
> +static const char *ignore_space(const char *s)
> +{
> + for (; *s && isspace(*s); s++)
> + ;
> +
> + return s;
> +}
> +
> static int parse_engine(char *line, struct drm_client_fdinfo *info,
> size_t prefix_len,
> const char **name_map, unsigned int map_entries,
> @@ -104,23 +112,6 @@ static int parse_engine(char *line, struct drm_client_fdinfo *info,
> return found;
> }
>
> -static const char *find_kv(const char *buf, const char *key, size_t keylen)
> -{
> - const char *p;
> -
> - if (strncmp(buf, key, keylen))
> - return NULL;
> -
> - p = buf + keylen;
> - if (*p != ':')
> - return NULL;
> -
> - for (p++; *p && isspace(*p); p++)
> - ;
> -
> - 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,
> @@ -205,6 +196,11 @@ out:
> } \
> } while (0)
>
> +#define strstartswith(a, b, plen__) ({ \
> + *plen__ = strlen(b); \
> + strncmp(a, b, *plen__) == 0; \
> +})
Did it have to be a macro for optimization to work?
> +
> unsigned int
> __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> const char **name_map, unsigned int map_entries,
> @@ -222,31 +218,39 @@ __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;
> + size_t keylen;
> const char *v;
> int idx;
>
> _buf = NULL;
>
> - if ((v = find_kv(l, "drm-driver", strlen("drm-driver")))) {
> - strncpy(info->driver, v, sizeof(info->driver) - 1);
> - good++;
> - } else if ((v = find_kv(l, "drm-client-id",
> - strlen("drm-client-id")))) {
> - info->id = atol(v);
> - good++;
> - } else if ((v = find_kv(l, "drm-pdev", strlen("drm-pdev")))) {
> - /* optional */
> + if (strstartswith(l, "drm-driver:", &keylen)) {
> + v = l + keylen;
> + v = ignore_space(v);
> + if (*v) {
> + strncpy(info->driver, v, sizeof(info->driver) - 1);
> + good++;
> + }
> + } else if (strstartswith(l, "drm-client-id:", &keylen)) {
> + v = l + keylen;
> + v = ignore_space(v);
> + if (*v) {
> + info->id = atol(v);
> + good++;
> + }
> + } else if (strstartswith(l, "drm-pdev:", &keylen)) {
> + v = l + keylen;
> + v = ignore_space(v);
Removing find_kv looks like a slight negative, given how now all
branches have to repeat v = + keylen; v = ignore_space(v).
I wonder if it would work by extending find_kv to allow prefix matching
mode, something like this (only two branches show to illustrate the modes):
...
} else if ((v = find_kv(l, "drm-pdev", MATCH_FULL, &keylen))) {
/* optional */
strncpy(info->pdev, v, sizeof(info->pdev) - 1);
} else if (find_kv(l, "drm-engine-capacity-", MATCH_PREFIX)) {
idx = parse_engine(l, info, keylen,
name_map, map_entries, &val);
if (idx >= 0) {
info->capacity[idx] = val;
num_capacity++;
}
...
?
Regards,
Tvrtko
> strncpy(info->pdev, v, sizeof(info->pdev) - 1);
> - } else if (!strncmp(l, "drm-engine-capacity-", 20)) {
> - idx = parse_engine(l, info,
> - strlen("drm-engine-capacity-"),
> + } else if (strstartswith(l, "drm-engine-capacity-", &keylen)) {
> + idx = parse_engine(l, info, keylen,
> name_map, map_entries, &val);
> if (idx >= 0) {
> info->capacity[idx] = val;
> num_capacity++;
> }
> - } else if (!strncmp(l, "drm-engine-", 11)) {
> - idx = parse_engine(l, info, strlen("drm-engine-"),
> + } else if (strstartswith(l, "drm-engine-", &keylen)) {
> + idx = parse_engine(l, info, keylen,
> name_map, map_entries, &val);
> if (idx >= 0) {
> if (!info->capacity[idx])
> @@ -256,25 +260,24 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> if (idx > info->last_engine_index)
> info->last_engine_index = idx;
> }
> - } else if (!strncmp(l, "drm-total-", 10)) {
> - idx = parse_region(l, info, strlen("drm-total-"),
> + } else if (strstartswith(l, "drm-total-", &keylen)) {
> + idx = parse_region(l, info, keylen,
> region_map, region_entries, &val);
> UPDATE_REGION(idx, total, val);
> - } else if (!strncmp(l, "drm-shared-", 11)) {
> - idx = parse_region(l, info, strlen("drm-shared-"),
> + } else if (strstartswith(l, "drm-shared-", &keylen)) {
> + idx = parse_region(l, info, keylen,
> region_map, region_entries, &val);
> UPDATE_REGION(idx, shared, val);
> -
> - } else if (!strncmp(l, "drm-resident-", 13)) {
> - idx = parse_region(l, info, strlen("drm-resident-"),
> + } else if (strstartswith(l, "drm-resident-", &keylen)) {
> + idx = parse_region(l, info, keylen,
> region_map, region_entries, &val);
> UPDATE_REGION(idx, resident, val);
> - } else if (!strncmp(l, "drm-purgeable-", 14)) {
> - idx = parse_region(l, info, strlen("drm-purgeable-"),
> + } else if (strstartswith(l, "drm-purgeable-", &keylen)) {
> + idx = parse_region(l, info, keylen,
> region_map, region_entries, &val);
> UPDATE_REGION(idx, purgeable, val);
> - } else if (!strncmp(l, "drm-active-", 11)) {
> - idx = parse_region(l, info, strlen("drm-active-"),
> + } else if (strstartswith(l, "drm-active-", &keylen)) {
> + idx = parse_region(l, info, keylen,
> region_map, region_entries, &val);
> UPDATE_REGION(idx, active, val);
> }
More information about the igt-dev
mailing list