Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

On Mon, May 25, 2026 at 12:50 PM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> On Tue, May 19, 2026 at 10:09:57AM -0700, Namhyung Kim wrote:
> > On Tue, May 19, 2026 at 08:13:02AM -0700, Ian Rogers wrote:
> > > On Mon, May 18, 2026 at 10:54 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> > > > On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote:
> > > > > +             # Parse each metric line
> > > > > +             while read -r line; do
> > > > > +                     [ -z "${line}" ] && continue
> > > > > +
> > > > > +                     local cycles_val
> > > > > +                     cycles_val=$(echo "${line}" | cut -d, -f1)
> > > > > +
> > > > > +                     local event_name
> > > > > +                     event_name=$(echo "${line}" | cut -d, -f3)
> > > > > +
> > > > > +                     # Find corresponding instructions event line
> > > > > +                     local inst_event_name
> > > > > +                     inst_event_name="${event_name/cpu-cycles/instructions}"
> > > > > +                     inst_event_name="${inst_event_name/cycles/instructions}"
> > > > > +
> > > > > +                     local inst_line
> > > > > +                     inst_line=$(echo "${output}" | \
> > > > > +                             grep -F ",${cgrp}," | \
> > > > > +                             grep -F "${inst_event_name}" || true)
> > > > > +
> > > > > +                     local inst_val
> > > > > +                     inst_val=$(echo "${inst_line}" | cut -d, -f1)
> > > > > +
> > > > > +                     log_verbose "Cgroup '${cgrp}': event '${event_name}' \
> > > > > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'"
>
> > > > Why not make log_verbose() take multiple arguments and align them
> > > > properly?  Maybe you can just use 'echo'?  The same goes to the below
> > > > lines.
>
> > > This fixes checkpatch warnings on line length and the odd alignment
> > > avoids spaces appearing in the output.
>
> > How about this?
>
> Ian posted a v4, are you ok with that now, Tested-by or Reviewed-by?

I can add the log_verbose change Namhyung asked for.

Thanks,
Ian

> - Arnaldo
>
> > Thanks,
> > Namhyung
> >
> >
> > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh
> > index d4226ee0ae9801cb..ecb2c053ea5158d4 100755
> > --- a/tools/perf/tests/shell/stat_metrics_cgrp.sh
> > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh
> > @@ -7,7 +7,7 @@ set -e
> >  test_cgroups=
> >
> >  log_verbose() {
> > -     echo "$1"
> > +     echo "$*"
> >  }
> >
> >  is_numeric_and_non_zero() {
> > @@ -145,22 +145,22 @@ check_metric_reported()
> >                               cycles_val=$(echo "${cycles_line}" | cut -d, -f1)
> >                       fi
> >
> > -                     log_verbose "Cgroup '${cgrp}': event '${event_name}' \
> > -val '${cycles_val}', inst val '${inst_val}'"
> > +                     log_verbose "Cgroup '${cgrp}': event '${event_name}'" \
> > +                                 "val '${cycles_val}', inst val '${inst_val}'"
> >
> >                       # Only enforce metric check if both cycles and
> >                       # instructions have non-zero numeric counts
> >                       if is_numeric_and_non_zero "${cycles_val}" && \
> >                          is_numeric_and_non_zero "${inst_val}"
> >                       then
> > -                             log_verbose "Enforcing metric check for cgroup '${cgrp}' \
> > -event '${event_name}'"
> > +                             log_verbose "Enforcing metric check for cgroup '${cgrp}'" \
> > +                                         "event '${event_name}'"
> >                               # Check for nan or nested in the metric value (7th field)
> >                               # Actually we can just check the whole line for simplicity
> >                               if echo "${line}" | grep -q -i -E ",nan,|,nested,"
> >                               then
> > -                                     echo "FAIL: Invalid metric value (nan/nested) \
> > -for cgroup '${cgrp}'"
> > +                                     echo "FAIL: Invalid metric value (nan/nested)" \
> > +                                          "for cgroup '${cgrp}'"
> >                                       echo "Line: ${line}"
> >                                       exit 1
> >                               fi
> > @@ -173,8 +173,8 @@ for cgroup '${cgrp}'"
> >                                       exit 1
> >                               fi
> >                       else
> > -                             log_verbose "Skipping metric check for cgroup '${cgrp}' \
> > -event '${event_name}' (idle or not counted)"
> > +                             log_verbose "Skipping metric check for cgroup '${cgrp}'" \
> > +                                         "event '${event_name}' (idle or not counted)"
> >                       fi
> >               done <<< "${cgrp_lines}"
> >       done