put sys_trace_isr_enter/sys_trace_isr_exit to user care about ISR instead of every ISR

[wentongwu]

Introduction

Currently the ISR tracing hook is put at the beginning and end of ISR wrapper, so every ISR will be traced, when there is ISR needed in the tracing path(especially tracing back-end, every output of tracing back-end will be traced once), it will cause the tracing buffer easily be exhausted, also it will increase system latency, so I suggest to remove the tracing hook at the _isr_wrapper, let end-users decide where to put sys_trace_isr_enter/sys_trace_isr_exit(usually end user cared ISRs), or at least we should have interface to filter out users not cared ISRs based on irq number.

[wentongwu]

@nashif

[nashif]

@wentongwu yes, makes sense, we probably want to leave the system timer though

[nashif]

something like that to get it working for nrf devices...

diff --git a/arch/arm/core/isr_wrapper.S b/arch/arm/core/isr_wrapper.S
index 466afa0512..30e2e06d6c 100644
--- a/arch/arm/core/isr_wrapper.S
+++ b/arch/arm/core/isr_wrapper.S
@@ -64,10 +64,6 @@ SECTION_FUNC(TEXT, _isr_wrapper)
        bl read_timer_start_of_isr
 #endif

-#ifdef CONFIG_TRACING
-       bl sys_trace_isr_enter
-#endif
-
 #ifdef CONFIG_SYS_POWER_MANAGEMENT
        /*
         * All interrupts are disabled when handling idle wakeup.  For tickless
@@ -151,10 +147,6 @@ _idle_state_cleared:
 #endif /* CONFIG_EXECUTION_BENCHMARKING */
        blx r3          /* call ISR */

-#ifdef CONFIG_TRACING
-       bl sys_trace_isr_exit
-#endif
-
 #if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE)
        pop {r0, r3}
        mov lr, r3
diff --git a/drivers/timer/nrf_rtc_timer.c b/drivers/timer/nrf_rtc_timer.c
index ba8dad9ebe..f50e6e69a8 100644
--- a/drivers/timer/nrf_rtc_timer.c
+++ b/drivers/timer/nrf_rtc_timer.c
@@ -12,6 +12,7 @@
 #include <sys_clock.h>
 #include <hal/nrf_rtc.h>
 #include <spinlock.h>
+#include <debug/tracing.h>

 #define RTC NRF_RTC1

@@ -51,6 +52,7 @@ static u32_t counter(void)
 void rtc1_nrf_isr(void *arg)
 {
        ARG_UNUSED(arg);
+       sys_trace_isr_enter();
        RTC->EVENTS_COMPARE[0] = 0;

        k_spinlock_key_t key = k_spin_lock(&lock);
@@ -73,6 +75,7 @@ void rtc1_nrf_isr(void *arg)

        k_spin_unlock(&lock, key);
        z_clock_announce(IS_ENABLED(CONFIG_TICKLESS_KERNEL) ? dticks : 1);
+       sys_trace_isr_exit();
 }

 int z_clock_driver_init(struct device *device)

[wentongwu]

@nashif ok, I will do this change for every platform.

[andrewboie]

also it will increase system latency, so I suggest to remove the tracing hook at the _isr_wrapper, let end-users decide where to put sys_trace_isr_enter/sys_trace_isr_exit(usually end user cared ISRs), or at least we should have interface to filter out users not cared ISRs based on irq number.

I'm not a huge fan of the approach where we have to move these tracing calls out to all the drivers.

least we should have interface to filter out users not cared ISRs based on irq number.

That could work much better - define a bitfield where bin N corresponds to IRQ line N, skip it if unset, and have an API to modify the bitfield. There would have to be something special for systick on ARM since that is an exception not an interrupt.

[mped-oticon]

If the user kconfig asked for tracing out ISRs, then let him.
Solution:

-#ifdef CONFIG_TRACING
+#ifdef CONFIG_TRACING_ISR
       bl sys_trace_isr_exit
#endif

[nashif]

covered with option to exclude ISRs for now.