Re: [PATCH 2/6] perf lock contention: Add -Y/--type-filter option

Namhyung Kim
2022-12-21T17:55:07Z

Hi Arnaldo,

On Wed, Dec 21, 2022 at 9:50 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Dec 19, 2022 at 12:17:28PM -0800, Namhyung Kim escreveu:

[SNIP]
> > @@ -1465,6 +1483,21 @@ static const char *get_type_str(unsigned int flags)
> > return "unknown";
> > }
> >
> > +static unsigned int get_type_flag(const char *str)
> > +{
> > + for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
> > + if (!strcmp(lock_type_table[i].name, str))
> > + return lock_type_table[i].flags;
> > + }
> > + return -1U;
> > +}
> > +
> > +static void lock_filter_finish(void)
> > +{
> > + zfree(&filters.types);
> > + filters.nr_types = 0;
> > +}
> > +
> > static void sort_contention_result(void)
> > {
> > sort_result();
> > @@ -1507,6 +1540,9 @@ static void print_contention_result(struct lock_contention *con)
> > if (st->broken)
> > bad++;
> >
> > + if (!st->wait_time_total)
> > + continue;
> > +
> > list_for_each_entry(key, &lock_keys, list) {
> > key->print(key, st);
> > pr_info(" ");
> > @@ -1753,6 +1789,7 @@ static int __cmd_contention(int argc, const char **argv)
> > print_contention_result(&con);
> >
> > out_delete:
> > + lock_filter_finish();
> > evlist__delete(con.evlist);
> > lock_contention_finish();
> > perf_session__delete(session);
> > @@ -1884,6 +1921,79 @@ static int parse_max_stack(const struct option *opt, const char *str,
> > return 0;
> > }
> >
> > +static bool add_lock_type(unsigned int flags)
> > +{
> > + unsigned int *tmp;
> > +
> > + tmp = realloc(filters.types, (filters.nr_types + 1) * sizeof(*filters.types));
> > + if (tmp == NULL)
> > + return false;
> > +
> > + tmp[filters.nr_types++] = flags;
> > + filters.types = tmp;
> > + return true;
> > +}
> > +
> > +static int parse_lock_type(const struct option *opt __maybe_unused, const char *str,
> > + int unset __maybe_unused)
> > +{
> > + char *s, *tmp, *tok;
> > + int ret = 0;
> > +
> > + s = strdup(str);
> > + if (s == NULL)
> > + return -1;
> > +
> > + for (tok = strtok_r(s, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) {
> > + unsigned int flags = get_type_flag(tok);
> > +
> > + if (flags == -1U) {
> > + char buf[32];
> > +
> > + if (strchr(tok, ':'))
> > + continue;
> > +
> > + /* try :R and :W suffixes for rwlock, rwsem, ... */
> > + scnprintf(buf, sizeof(buf), "%s:R", tok);
> > + flags = get_type_flag(buf);
> > + if (flags != -1UL) {
> > + if (!add_lock_type(flags)) {
> > + ret = -1;
> > + break;
> > + }
> > + }
>
>
> clang doesn't like this:
>
> 34 97.97 fedora:36 : FAIL clang version 14.0.5 (Fedora 14.0.5-2.fc36)
> builtin-lock.c:2012:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
> if (flags != -1UL) {
> ~~~~~ ^ ~~~~
> builtin-lock.c:2021:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
> if (flags != -1UL) {
> ~~~~~ ^ ~~~~
> builtin-lock.c:2037:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
> if (flags != -1UL) {
> ~~~~~ ^ ~~~~
> 3 errors generated.
>
> I applied this on top, Ack?

Oh.. sorry for that. It could be just -1U. But I'm ok with UINT_MAX too.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung