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:     Namhyung Kim <namhyung@kernel.org>
CC:     Song Liu <song@kernel.org>,
        linux-kernel <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>,
        Jiri Olsa <jolsa@kernel.org>
Subject: Re: [PATCH v3 3/4] perf-stat: introduce config
 stat.bpf-counter-events
Thread-Topic: [PATCH v3 3/4] perf-stat: introduce config
 stat.bpf-counter-events
Thread-Index: AQHXMw3DyOxUaVUpS0ODNNUcJNtKe6q46/0AgANiUwA=
Date:   Mon, 19 Apr 2021 20:26:02 +0000
Message-ID: <71CC9E29-ECBD-47DB-AE85-1477BF54C45D@fb.com>
References: <20210416221325.2373497-1-song@kernel.org>
 <20210416221325.2373497-4-song@kernel.org>
 <CAM9d7ciVj+d=Bgqmwu+v9mA1CGTbtr8pfXzgNKJs0Nh3BJtt8A@mail.gmail.com>
In-Reply-To: <CAM9d7ciVj+d=Bgqmwu+v9mA1CGTbtr8pfXzgNKJs0Nh3BJtt8A@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <BFE31E0CB41C3941910DF3CA72A6FD32@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: b6b79da2-2ec5-4b6d-7a6f-08d903715a50
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Apr 2021 20:26:02.9997
 (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: fCs1+Lyyc0eu5K2zfD7m6YmEUoWn3TWuKbK/nVN3Q8QVZaOjwAu7hptmeQF9A03mfVD40odZHjqq4SE0a9IEsA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB3366
X-OriginatorOrg: fb.com
X-Proofpoint-GUID: Jw-Z5gYRKiTfGlI90VmIQXz748czila3
X-Proofpoint-ORIG-GUID: Jw-Z5gYRKiTfGlI90VmIQXz748czila3
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761
 definitions=2021-04-19_11:2021-04-19,2021-04-19 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 phishscore=0
 priorityscore=1501 mlxscore=0 spamscore=0 adultscore=0 impostorscore=0
 suspectscore=0 lowpriorityscore=0 mlxlogscore=999 clxscore=1015
 bulkscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104060000 definitions=main-2104190140
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Apr 17, 2021, at 9:45 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>=20
> Hi Song,
>=20
> On Sat, Apr 17, 2021 at 7:13 AM Song Liu <song@kernel.org> wrote:
>>=20
>> 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. Events with name in the option will use
>> BPF.
>>=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
>> The second command will use BPF for "instructions" but not "cs".
>>=20
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> @@ -535,12 +549,13 @@ static int enable_counters(void)
>>        struct evsel *evsel;
>>        int err;
>>=20
>> -       if (target__has_bpf(&target)) {
>> -               evlist__for_each_entry(evsel_list, evsel) {
>> -                       err =3D bpf_counter__enable(evsel);
>> -                       if (err)
>> -                               return err;
>> -               }
>> +       evlist__for_each_entry(evsel_list, evsel) {
>> +               if (!evsel__is_bpf(evsel))
>> +                       continue;
>> +
>> +               err =3D bpf_counter__enable(evsel);
>> +               if (err)
>> +                       return err;
>=20
> I just realized it doesn't have a disable counterpart.

I guess it is not really necessary for perf-stat? It is probably good
to have it though. How about we do it in a follow up patch?

[...]

>> +       if (!evsel__bpf_counter_events)
>> +               return false;
>> +
>> +       ptr =3D strstr(evsel__bpf_counter_events, name);
>> +       name_len =3D strlen(name);
>> +
>> +       /* check name matches a full token in evsel__bpf_counter_events =
*/
>> +       match =3D (ptr !=3D NULL) &&
>> +               ((ptr =3D=3D evsel__bpf_counter_events) || (*(ptr - 1) =
=3D=3D ',')) &&
>> +               ((*(ptr + name_len) =3D=3D ',') || (*(ptr + name_len) =
=3D=3D '\0'));
>=20
> I'm not sure we have an event name which is a substring of another.
> Maybe it can retry if it fails to match.

We have ref-cycles and cycles. And some raw events may be substring of othe=
rs?

Thanks,
Song

[...]