Received-SPF: pass (google.com: domain of linux-kernel+bounces-124169-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240326202859.960577-1-weilin.wang@intel.com> <20240326202859.960577-2-weilin.wang@intel.com>
In-Reply-To: <20240326202859.960577-2-weilin.wang@intel.com>
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 28 Mar 2024 22:46:00 -0700
Message-ID: <CAM9d7cjm6uOhffsiuVKk=2GBEUBZUyhNe-5QM5--tw3-X+UBJw@mail.gmail.com>
Subject: Re: [RFC PATCH v5 1/6] perf stat: Parse and find tpebs events when
 parsing metrics to prepare for perf record sampling
To: weilin.wang@intel.com
Cc: Ian Rogers <irogers@google.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, 
	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	Adrian Hunter <adrian.hunter@intel.com>, Kan Liang <kan.liang@linux.intel.com>, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Perry Taylor <perry.taylor@intel.com>, Samantha Alt <samantha.alt@intel.com>, 
	Caleb Biggers <caleb.biggers@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 26, 2024 at 1:29=E2=80=AFPM <weilin.wang@intel.com> wrote:
>
> From: Weilin Wang <weilin.wang@intel.com>
>
> Metrics that use tpebs values would use the :retire_latency keyword in
> formulas. We put all these events into a list and pass the list to perf
> record to collect their retire latency value.
>
> Signed-off-by: Weilin Wang <weilin.wang@intel.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> ---
[SNIP]
> @@ -681,8 +685,56 @@ static int metricgroup__build_event_string(struct st=
rbuf *events,
>         hashmap__for_each_entry(ctx->ids, cur, bkt) {
>                 const char *sep, *rsep, *id =3D cur->pkey;
>                 enum perf_tool_event ev;
> +               /*
> +                * Parse and search for event name with retire_latency mo=
difier.
> +                * If found, put event name into the tpebs_events list. T=
his list
> +                * of events will be passed to perf record for sampling t=
o get
> +                * their reitre_latency value.
> +                * Search for ":R" in event name without "@". Search for =
the
> +                * last "@R" in event name with "@".

What is the retire_latency modifier exactly?
Why do you search for :R and @R?  What's the difference?
Why don't you search for "retire_latency"?

Thanks,
Namhyung


> +                */
> +               char *p =3D strstr(id, ":R");
> +               char *p1 =3D strstr(id, "@R");
> +
> +               if (p =3D=3D NULL && p1) {
> +                       p =3D strstr(p1+1, "@R");
> +                       if (p =3D=3D NULL)
> +                               p =3D p1;
> +                       p =3D p+1;
> +               }
> +
> +               if (p) {
> +                       char *name;
> +                       char *at;
> +                       struct tpebs_event *new_event =3D malloc(sizeof(s=
truct tpebs_event));
>
> -               pr_debug("found event %s\n", id);
> +                       if (!new_event)
> +                               return -ENOMEM;
> +
> +                       new_event->tpebs_name =3D strdup(id);
> +                       *p =3D '\0';
> +                       name =3D malloc(strlen(id) + 2);
> +                       if (!name)
> +                               return -ENOMEM;
> +
> +                       at =3D strchr(id, '@');
> +                       if (at !=3D NULL) {
> +                               *at =3D '/';
> +                               at =3D strchr(id, '@');
> +                               *at =3D '/';
> +                               strcpy(name, id);
> +                               strcat(name, "p");
> +                       } else {
> +                               strcpy(name, id);
> +                               strcat(name, ":p");
> +                       }
> +                       new_event->name =3D name;
> +                       *tpebs_event_size +=3D 1;
> +                       pr_debug("retire_latency required, tpebs_event_si=
ze=3D%lu, new_event=3D%s\n",
> +                               *tpebs_event_size, new_event->name);
> +                       list_add_tail(&new_event->nd, tpebs_events);
> +                       continue;
> +               }
>
>                 /* Always move tool events outside of the group. */
>                 ev =3D perf_tool_event__from_str(id);