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

[Namhyung Kim]

Hi Arnaldo,

On Wed, Dec 21, 2022 at 9:50 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> 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@xxxxxxxxxx>

Thanks,
Namhyung