Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Song Liu <songliubraving@fb.com>
To:     Jiri Olsa <jolsa@redhat.com>
CC:     Song Liu <song@kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Arnaldo Carvalho de Melo <acme@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        "jolsa@kernel.org" <jolsa@kernel.org>
Subject: Re: [PATCH v2 3/3] perf-stat: introduce config
 stat.bpf-counter-events
Thread-Topic: [PATCH v2 3/3] perf-stat: introduce config
 stat.bpf-counter-events
Thread-Index: AQHXK0YG4LdD7IbPc0ehmtAjm2+fxaqqg16AgABRlYA=
Date:   Thu, 8 Apr 2021 16:39:33 +0000
Message-ID: <FA667178-09F3-48C5-891C-9E44744276C0@fb.com>
References: <20210407003601.619158-1-song@kernel.org>
 <20210407003601.619158-4-song@kernel.org> <YG7tU/mVxQZLNCYL@krava>
In-Reply-To: <YG7tU/mVxQZLNCYL@krava>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6375BB2C43BFE04095AA4897889410A1@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 85acbf3e-7c2d-44e5-5dc2-08d8faace3ec
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Apr 2021 16:39:33.6193
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ghyf8Bt5/ncfN+qmOKH7A/6OxQBsm2uk7EepN4aSieTVcApoxbwnd5p3JdHnqHuWCHP4VS/vBlyF1LmmdNKMJg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR15MB4203
X-OriginatorOrg: fb.com
X-Proofpoint-ORIG-GUID: hol-I3hDyirc5JcfYVuNwiYepG1px7V7
X-Proofpoint-GUID: hol-I3hDyirc5JcfYVuNwiYepG1px7V7
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761
 definitions=2021-04-08_04:2021-04-08,2021-04-08 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 mlxscore=0
 mlxlogscore=999 phishscore=0 adultscore=0 priorityscore=1501
 lowpriorityscore=0 malwarescore=0 suspectscore=0 spamscore=0 clxscore=1015
 bulkscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-2104060000 definitions=main-2104080110
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Apr 8, 2021, at 4:47 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>=20
> On Tue, Apr 06, 2021 at 05:36:01PM -0700, Song Liu wrote:
>> Currently, to use BPF to aggregate perf event counters, the user uses
>> --bpf-counters option. Enable "use bpf by default" events with a config
>> option, stat.bpf-counter-events. This is limited to hardware events in
>> evsel__hw_names.
>>=20
>> This also enables mixed BPF event and regular event in the same sesssion=
.
>> For example:
>>=20
>>   perf config stat.bpf-counter-events=3Dinstructions
>>   perf stat -e instructions,cs
>>=20
>=20
> so if we are mixing events now, how about uing modifier for bpf counters,
> instead of configuring .perfconfig list we could use:
>=20
>  perf stat -e instructions:b,cs
>=20
> thoughts?
>=20
> the change below adds 'b' modifier and sets 'evsel::bpf_counter',
> feel free to use it

I think we will need both 'b' modifier and .perfconfig configuration.=20
For systems with BPF-managed perf events running in the background,=20
.perfconfig makes sure perf-stat sessions will share PMCs with these=20
background monitoring tools. 'b' modifier, on the other hand, is useful
when the user knows there is opportunity to share the PMCs.=20

Does this make sense?=20

Thanks,
Song

>=20
> jirka
>=20
>=20
> ---
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index ca52581f1b17..c55e4e58d1dc 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -82,6 +82,7 @@ struct evsel {
> 		bool			auto_merge_stats;
> 		bool			collect_stat;
> 		bool			weak_group;
> +		bool			bpf_counter;
> 		int			bpf_fd;
> 		struct bpf_object	*bpf_obj;
> 	};
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-event=
s.c
> index 9ecb45bea948..b5850f1ea90b 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1801,6 +1801,7 @@ struct event_modifier {
> 	int pinned;
> 	int weak;
> 	int exclusive;
> +	int bpf_counter;
> };
>=20
> static int get_event_modifier(struct event_modifier *mod, char *str,
> @@ -1821,6 +1822,7 @@ static int get_event_modifier(struct event_modifier=
 *mod, char *str,
> 	int exclude =3D eu | ek | eh;
> 	int exclude_GH =3D evsel ? evsel->exclude_GH : 0;
> 	int weak =3D 0;
> +	int bpf_counter =3D 0;
>=20
> 	memset(mod, 0, sizeof(*mod));
>=20
> @@ -1864,6 +1866,8 @@ static int get_event_modifier(struct event_modifier=
 *mod, char *str,
> 			exclusive =3D 1;
> 		} else if (*str =3D=3D 'W') {
> 			weak =3D 1;
> +		} else if (*str =3D=3D 'b') {
> +			bpf_counter =3D 1;
> 		} else
> 			break;
>=20
> @@ -1895,6 +1899,7 @@ static int get_event_modifier(struct event_modifier=
 *mod, char *str,
> 	mod->sample_read =3D sample_read;
> 	mod->pinned =3D pinned;
> 	mod->weak =3D weak;
> +	mod->bpf_counter =3D bpf_counter;
> 	mod->exclusive =3D exclusive;
>=20
> 	return 0;
> @@ -1909,7 +1914,7 @@ static int check_modifier(char *str)
> 	char *p =3D str;
>=20
> 	/* The sizeof includes 0 byte as well. */
> -	if (strlen(str) > (sizeof("ukhGHpppPSDIWe") - 1))
> +	if (strlen(str) > (sizeof("ukhGHpppPSDIWeb") - 1))
> 		return -1;
>=20
> 	while (*p) {
> @@ -1950,6 +1955,7 @@ int parse_events__modifier_event(struct list_head *=
list, char *str, bool add)
> 		evsel->sample_read         =3D mod.sample_read;
> 		evsel->precise_max         =3D mod.precise_max;
> 		evsel->weak_group	   =3D mod.weak;
> +		evsel->bpf_counter         =3D mod.bpf_counter;
>=20
> 		if (evsel__is_group_leader(evsel)) {
> 			evsel->core.attr.pinned =3D mod.pinned;
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-event=
s.l
> index 0b36285a9435..fb8646cc3e83 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -210,7 +210,7 @@ name_tag	[\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=
=3D]*[\']
> name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
> drv_cfg_term	[a-zA-Z0-9_\.]+(=3D[a-zA-Z0-9_*?\.:]+)?
> /* If you add a modifier you need to update check_modifier() */
> -modifier_event	[ukhpPGHSDIWe]+
> +modifier_event	[ukhpPGHSDIWeb]+
> modifier_bp	[rwx]{1,3}
>=20
> %%
>=20