| Message ID | 20200512124451.061059334@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: perf: Proper cap_user_time* support | expand |
On Tue, May 12, 2020 at 02:41:03PM +0200, Peter Zijlstra wrote: > This completes the ARM64 cap_user_time support. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/arm64/kernel/perf_event.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > --- a/arch/arm64/kernel/perf_event.c > +++ b/arch/arm64/kernel/perf_event.c > @@ -1173,6 +1173,7 @@ void arch_perf_update_userpage(struct pe > > userpg->cap_user_time = 0; > userpg->cap_user_time_zero = 0; > + userpg->cap_user_time_short = 0; > > do { > rd = sched_clock_read_begin(&seq); > @@ -1183,13 +1184,13 @@ void arch_perf_update_userpage(struct pe > userpg->time_mult = rd->mult; > userpg->time_shift = rd->shift; > userpg->time_zero = rd->epoch_ns; > + userpg->time_cycle = rd->epoch_cyc; s/time_cycle/time_cycles, maybe consider to change the naming to 'time_cycle'. This patch set looks good to me after I tested it on Arm64 board. Thanks, Leo > + userpg->time_mask = rd->sched_clock_mask; > > /* > - * This isn't strictly correct, the ARM64 counter can be > - * 'short' and then we get funnies when it wraps. The correct > - * thing would be to extend the perf ABI with a cycle and mask > - * value, but because wrapping on ARM64 is very rare in > - * practise this 'works'. > + * Subtract the cycle base, such that software that > + * doesn't know about cap_user_time_short still 'works' > + * assuming no wraps. > */ > userpg->time_zero -= (rd->epoch_cyc * rd->mult) >> rd->shift; > > @@ -1214,4 +1215,5 @@ void arch_perf_update_userpage(struct pe > */ > userpg->cap_user_time = 1; > userpg->cap_user_time_zero = 1; > + userpg->cap_user_time_short = 1; > } > >
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/perf/core]
[also build test ERROR on arm64/for-next/core arm-perf/for-next/perf linus/master v5.7-rc5 next-20200512]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Peter-Zijlstra/arm64-perf-Proper-cap_user_time-support/20200512-205141
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 059c6d68cfc5f85ba3ab71d71a6de380016f7936
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/arm64/kernel/perf_event.c: In function 'arch_perf_update_userpage':
>> arch/arm64/kernel/perf_event.c:1187:11: error: 'struct perf_event_mmap_page' has no member named 'time_cycle'; did you mean 'time_cycles'?
1187 | userpg->time_cycle = rd->epoch_cyc;
| ^~~~~~~~~~
| time_cycles
arch/arm64/kernel/perf_event.c:1207:12: error: 'struct perf_event_mmap_page' has no member named 'shift'
1207 | if (userpg->shift == 32) {
| ^~
arch/arm64/kernel/perf_event.c:1208:9: error: 'struct perf_event_mmap_page' has no member named 'shift'
1208 | userpg->shift = 31;
| ^~
vim +1187 arch/arm64/kernel/perf_event.c
1167
1168 void arch_perf_update_userpage(struct perf_event *event,
1169 struct perf_event_mmap_page *userpg, u64 now)
1170 {
1171 struct clock_read_data *rd;
1172 unsigned int seq;
1173
1174 userpg->cap_user_time = 0;
1175 userpg->cap_user_time_zero = 0;
1176 userpg->cap_user_time_short = 0;
1177
1178 do {
1179 rd = sched_clock_read_begin(&seq);
1180
1181 if (rd->read_sched_clock != arch_timer_read_counter)
1182 return;
1183
1184 userpg->time_mult = rd->mult;
1185 userpg->time_shift = rd->shift;
1186 userpg->time_zero = rd->epoch_ns;
> 1187 userpg->time_cycle = rd->epoch_cyc;
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
--- a/arch/arm64/kernel/perf_event.c +++ b/arch/arm64/kernel/perf_event.c @@ -1173,6 +1173,7 @@ void arch_perf_update_userpage(struct pe userpg->cap_user_time = 0; userpg->cap_user_time_zero = 0; + userpg->cap_user_time_short = 0; do { rd = sched_clock_read_begin(&seq); @@ -1183,13 +1184,13 @@ void arch_perf_update_userpage(struct pe userpg->time_mult = rd->mult; userpg->time_shift = rd->shift; userpg->time_zero = rd->epoch_ns; + userpg->time_cycle = rd->epoch_cyc; + userpg->time_mask = rd->sched_clock_mask; /* - * This isn't strictly correct, the ARM64 counter can be - * 'short' and then we get funnies when it wraps. The correct - * thing would be to extend the perf ABI with a cycle and mask - * value, but because wrapping on ARM64 is very rare in - * practise this 'works'. + * Subtract the cycle base, such that software that + * doesn't know about cap_user_time_short still 'works' + * assuming no wraps. */ userpg->time_zero -= (rd->epoch_cyc * rd->mult) >> rd->shift; @@ -1214,4 +1215,5 @@ void arch_perf_update_userpage(struct pe */ userpg->cap_user_time = 1; userpg->cap_user_time_zero = 1; + userpg->cap_user_time_short = 1; }
This completes the ARM64 cap_user_time support. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/arm64/kernel/perf_event.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)