casaroli commented on code in PR #21475:
URL: https://github.com/apache/nuttx/pull/21475#discussion_r4501777590
##########
libs/libc/sched/clock_perf.c:
##########
@@ -35,7 +44,46 @@
* perf_gettime
****************************************************************************/
-clock_t perf_gettime(void)
+clock_t noinstrument_function perf_gettime(void)
+{
+ clock_t overflow;
+ clock_t result;
+ uint32_t seq;
+
+ do
+ {
+ seq = read_seqbegin(&g_perf.lock);
Review Comment:
In a kernel build this copy reads the kernel's `g_perf`, which a process
cannot reach. `ARCH_PERF_EVENTS_USER_ACCESS` defaults to y there, and on
`qemu-armv7a:knsh` an application that calls `perf_gettime()` does not link:
`libs/libc/sched/clock_perf.c:56: undefined reference to 'up_perf_gettime'` and
`clock_perf.c:72: undefined reference to 'g_perf'`.
Fix: add `depends on !BUILD_KERNEL` to `ARCH_PERF_EVENTS_USER_ACCESS` (or do
not build this copy in a kernel build).
##########
sched/Kconfig:
##########
@@ -259,8 +259,8 @@ config PREALLOC_TIMERS
config PERF_OVERFLOW_CORRECTION
bool "Compensate perf count overflow"
- depends on ALARM_ARCH || TIMER_ARCH || ARCH_PERF_EVENTS
- default n
+ depends on !ARCH_64BIT && (ALARM_ARCH || TIMER_ARCH || ARCH_PERF_EVENTS)
+ default y
Review Comment:
The PR body says this option defaults to y "only when `ARCH_PERF_EVENTS` is
enabled", but it is still `default y`. MemBrowse at this head still shows
hifive1-revb +452 B .text and +64 B .bss.
Fix: `default y if ARCH_PERF_EVENTS`.
##########
arch/Kconfig:
##########
@@ -675,11 +675,18 @@ config ARCH_HAVE_PERF_EVENTS
config ARCH_HAVE_PERF_EVENTS_USER_ACCESS
bool
- default n
select ARCH_HAVE_PERF_EVENTS
+ default n
---help---
Select if hardware allows userspace perf counter access.
+config ARCH_PERF_EVENTS_USER_ACCESS
+ bool "Enable userspace perf counter access"
+ default !BUILD_FLAT
+ depends on ARCH_HAVE_PERF_EVENTS_USER_ACCESS
Review Comment:
The PR body says this option "also depends on" `ARCH_PERF_EVENTS`, but it
still depends only on `ARCH_HAVE_PERF_EVENTS_USER_ACCESS`. `qemu-armv7a:knsh`
gets `ARCH_PERF_EVENTS_USER_ACCESS=y` with `ARCH_PERF_EVENTS=n`, so
`up_perf_init()` never enables user access to the counter.
Fix: add `depends on ARCH_PERF_EVENTS`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
