Date:   Thu, 26 Jan 2023 17:06:01 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To:     Namhyung Kim <namhyung@kernel.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>
CC:     Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ian Rogers <irogers@google.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-perf-users@vger.kernel.org,
        Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH] perf script: Add 'cgroup' field for output
User-Agent: K-9 Mail for Android
In-Reply-To: <20230126194230.3339019-1-namhyung@kernel.org>
References: <20230126194230.3339019-1-namhyung@kernel.org>
Message-ID: <ACACAC8D-3390-4D47-B227-5C3182109847@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


On January 26, 2023 4:42:30 PM GMT-03:00, Namhyung Kim <namhyung@kernel=2E=
org> wrote:
>There's no field for the cgroup, let's add one=2E  To do that, users need=
 to
>specify --all-cgroup option for perf record to capture the cgroup info=2E


This information about the new field being only available when --all-cgrou=
p is specified should be in the documentation (man page) and probably in th=
e tool itself, refusing it when the needed command line isn't present=2E

- Arnaldo=20

>
>  $ perf record --all-cgroups -- true
>
>  $ perf script -F comm,pid,cgroup
>            true 337112  /user=2Eslice/user-657345=2Eslice/user@657345=2E=
service/=2E=2E=2E
>            true 337112  /user=2Eslice/user-657345=2Eslice/user@657345=2E=
service/=2E=2E=2E
>            true 337112  /user=2Eslice/user-657345=2Eslice/user@657345=2E=
service/=2E=2E=2E
>            true 337112  /user=2Eslice/user-657345=2Eslice/user@657345=2E=
service/=2E=2E=2E
>
>Signed-off-by: Namhyung Kim <namhyung@kernel=2Eorg>
>---
> tools/perf/Documentation/perf-script=2Etxt |  2 +-
> tools/perf/builtin-script=2Ec              | 16 +++++++++++++++-
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
>diff --git a/tools/perf/Documentation/perf-script=2Etxt b/tools/perf/Docu=
mentation/perf-script=2Etxt
>index 8d77182fbf31=2E=2E82c4dcb9c42e 100644
>--- a/tools/perf/Documentation/perf-script=2Etxt
>+++ b/tools/perf/Documentation/perf-script=2Etxt
>@@ -134,7 +134,7 @@ OPTIONS
>         srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-o=
utput,
>         brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnl=
en, synth,
>         phys_addr, metric, misc, srccode, ipc, data_page_size, code_page=
_size, ins_lat,
>-        machine_pid, vcpu=2E
>+        machine_pid, vcpu, cgroup=2E
>         Field list can be prepended with the type, trace, sw or hw,
>         to indicate to which event type the field list applies=2E
>         e=2Eg=2E, -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trac=
e
>diff --git a/tools/perf/builtin-script=2Ec b/tools/perf/builtin-script=2E=
c
>index 69394ac0a20d=2E=2E79976dde99b3 100644
>--- a/tools/perf/builtin-script=2Ec
>+++ b/tools/perf/builtin-script=2Ec
>@@ -59,6 +59,7 @@
> #include "util/dlfilter=2Eh"
> #include "util/record=2Eh"
> #include "util/util=2Eh"
>+#include "util/cgroup=2Eh"
> #include "perf=2Eh"
>=20
> #include <linux/ctype=2Eh>
>@@ -130,6 +131,7 @@ enum perf_output_field {
> 	PERF_OUTPUT_BRSTACKINSNLEN  =3D 1ULL << 36,
> 	PERF_OUTPUT_MACHINE_PID     =3D 1ULL << 37,
> 	PERF_OUTPUT_VCPU            =3D 1ULL << 38,
>+	PERF_OUTPUT_CGROUP          =3D 1ULL << 39,
> };
>=20
> struct perf_script {
>@@ -200,6 +202,7 @@ struct output_option {
> 	{=2Estr =3D "brstackinsnlen", =2Efield =3D PERF_OUTPUT_BRSTACKINSNLEN},
> 	{=2Estr =3D "machine_pid", =2Efield =3D PERF_OUTPUT_MACHINE_PID},
> 	{=2Estr =3D "vcpu", =2Efield =3D PERF_OUTPUT_VCPU},
>+	{=2Estr =3D "cgroup", =2Efield =3D PERF_OUTPUT_CGROUP},
> };
>=20
> enum {
>@@ -2220,6 +2223,17 @@ static void process_event(struct perf_script *scri=
pt,
> 	if (PRINT_FIELD(CODE_PAGE_SIZE))
> 		fprintf(fp, " %s", get_page_size_name(sample->code_page_size, str));
>=20
>+	if (PRINT_FIELD(CGROUP)) {
>+		const char *cgrp_name;
>+		struct cgroup *cgrp =3D cgroup__find(machine->env,
>+						   sample->cgroup);
>+		if (cgrp !=3D NULL)
>+			cgrp_name =3D cgrp->name;
>+		else
>+			cgrp_name =3D "unknown";
>+		fprintf(fp, " %s", cgrp_name);
>+	}
>+
> 	perf_sample__fprintf_ipc(sample, attr, fp);
>=20
> 	fprintf(fp, "\n");
>@@ -3856,7 +3870,7 @@ int cmd_script(int argc, const char **argv)
> 		     "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
> 		     "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
> 		     "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
>-		     "code_page_size,ins_lat",
>+		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup",
> 		     parse_output_fields),
> 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
> 		    "system-wide collection from all CPUs"),