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:     Arnaldo Carvalho de Melo <acme@kernel.org>
CC:     open list <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Jiri Olsa <jolsa@redhat.com>,
        "Kernel Team" <Kernel-team@fb.com>
Subject: Re: FIX Re: [PATCH v7 3/3] perf-stat: enable counting events for BPF
 programs
Thread-Topic: FIX Re: [PATCH v7 3/3] perf-stat: enable counting events for BPF
 programs
Thread-Index: AQHW3iuRYm1YXRRzOkq8jjAuk5Kmsqot5piAgABWngCAAOYQgIAAAxqAgAAeVYCAAFpfAIAACeyAgADsroCAAAbAgIAADZSAgAAsyoCAAALdAA==
Date:   Wed, 20 Jan 2021 16:40:46 +0000
Message-ID: <498F663D-B3D5-4339-B076-B8D24FFD8B9A@fb.com>
References: <20210118193817.GG12699@kernel.org>
 <379919CC-594F-40C5-A10E-97E048F73AE2@fb.com>
 <20210119143143.GJ12699@kernel.org> <20210119144249.GK12699@kernel.org>
 <20210119163123.GM12699@kernel.org>
 <417ABE57-E527-4944-BE16-35AB92D071E9@fb.com>
 <20210119223021.GO12699@kernel.org> <20210120123727.GR12699@kernel.org>
 <20210120130137.GS12699@kernel.org> <20210120135013.GT12699@kernel.org>
 <20210120163031.GU12699@kernel.org>
In-Reply-To: <20210120163031.GU12699@kernel.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <90C8467B31B7DE459997AE423FC14C39@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 63f89e0f-be06-4d6e-0f72-08d8bd622333
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jan 2021 16:40:46.6879
 (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: vP9Lby0aFmt+T9hnN9jvsteAxKMwZIHWSOQCqOm8K2/rUiTi0akC1aja6O62jaqynhhkPQlM8XmXh+X7UC17MQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR15MB4291
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737
 definitions=2021-01-20_10:2021-01-20,2021-01-20 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 mlxlogscore=999
 spamscore=0 malwarescore=0 phishscore=0 impostorscore=0 lowpriorityscore=0
 clxscore=1015 suspectscore=0 bulkscore=0 mlxscore=0 priorityscore=1501
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2101200097
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Jan 20, 2021, at 8:30 AM, Arnaldo Carvalho de Melo <acme@kernel.org> w=
rote:
>=20
> Em Wed, Jan 20, 2021 at 10:50:13AM -0300, Arnaldo Carvalho de Melo escrev=
eu:
>> So sizeof(struct bpf_perf_event_value) =3D=3D 24 and it is a per-cpu arr=
ay, the
>> machine has 24 cpus, why is the kernel thinking it has more and end up z=
eroing
>> entries after the 24 cores? Some percpu map subtlety (or obvious thing ;=
-\) I'm
>> missing?
>>=20
>> Checking lookups into per cpu maps in sample code now...
>=20
> (gdb) run stat -b 244 -I 1000 -e cycles
> Starting program: /root/bin/perf stat -b 244 -I 1000 -e cycles
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> libbpf: elf: skipping unrecognized data section(9) .eh_frame
> libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_f=
rame
>=20
> Breakpoint 1, bpf_program_profiler__read (evsel=3D0xce02c0) at util/bpf_c=
ounter.c:217
> 217		if (list_empty(&evsel->bpf_counter_list))
> (gdb) p num_
> num_cpu              num_groups           num_leaps            num_print_=
iv         num_stmts            num_transitions      num_warnings_issued
> num_cpu_bpf          num_ifs              num_print_interval   num_srcfil=
es         num_to_str           num_types
> (gdb) p num_cpu
> $1 =3D 24
> (gdb) p num_cpu_bpf
> $2 =3D 32
> (gdb)
>=20
> Humm, why?
>=20
> But then libbpf and the sample/bpf/ code use it this way:
>=20
>=20
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.=
c
> index 8c977f038f497fc1..7dd3d57aba4f620c 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -207,7 +207,8 @@ static int bpf_program_profiler__enable(struct evsel =
*evsel)
> static int bpf_program_profiler__read(struct evsel *evsel)
> {
> 	int num_cpu =3D evsel__nr_cpus(evsel);
> -	struct bpf_perf_event_value values[num_cpu];
> +	int num_cpu_bpf =3D libbpf_num_possible_cpus();
> +	struct bpf_perf_event_value values[num_cpu > num_cpu_bpf ? num_cpu : nu=
m_cpu_bpf];
> 	struct bpf_counter *counter;
> 	int reading_map_fd;
> 	__u32 key =3D 0;
>=20
> -------------------------------------------------------------
>=20
> [root@five ~]# cat /sys/devices/system/cpu/possible
> 0-31
> [root@five ~]#
>=20
> I bet that in your test systems evsel__nr_cpus(evsel) matches
> /sys/devices/system/cpu/possible and thus you don't see the problem.

Thanks Arnaldo!

Yes, my system have same online and possible CPUs.=20

Since possible_cpu >=3D online_cpu, maybe we can use num_cpu_bpf in=20
bpf_program_profiler__read() without he extra check?=20

Song

[...]=