Add Trace Hook Macros and function that returns the start of the stack. #659

conara · 2023-04-05T11:41:43Z

Add Trace Hook Macros and function that returns the start of the stack.

The purpose of this pull request is to provide better support for tracing tools such as systemview. Some time ago I made a post on the FreeRTOS forum describing the current problem: https://forums.freertos.org/t/add-additional-tracing-functionality-to-kernel/16513 . This patchset adds a few new trace hooks that can be used by trace tools such as systemview. For now, I've only added support for the Cortex-M7 port, but if the change is a good improvement, I can add support for other ports. Note that this patch set is heavily inspired on the deprecated SystemView patch set.

Test Steps

I have tested my changes. No regression in existing tests.
I have modified and/or added unit-tests to cover the code changes in this Pull Request.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

n9wxu · 2023-04-05T16:44:30Z

tasks.c

@@ -1708,6 +1708,7 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB )
                mtCOVERAGE_TEST_MARKER();
            }

+            traceMOVED_TASK_TO_SUSPENDED_LIST(pxTCB);


Is this macro needed? A few lines earlier is the traceTASK_SUSPEND(pxTCB) macro (line 1688) So we already know that the function is called and the task is being suspended. Is it necessary to know that the suspension is happening 23 lines of code later?

I agree this macro is not necessarily needed.

n9wxu

traceMOVED_TASK_TO_SUSPEND_LIST seems redundant to traceTASK_SUSPEND.

aggarg · 2023-04-06T10:47:18Z

include/task.h

+ *
+ * @return A pointer to the start of the stack.
+ */
+uint8_t* pxTaskGetStackStart( TaskHandle_t xTask) PRIVILEGED_FUNCTION;


You can get this information using vTaskGetInfo API -

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/include/task.h#L1041

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/include/task.h#L163

I agree. Removed it from the patchset.

conara · 2023-04-06T18:51:41Z

traceMOVED_TASK_TO_SUSPEND_LIST seems redundant to traceTASK_SUSPEND.

Removed it from the patchset.

codecov · 2023-04-06T22:43:07Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (f13ad77) 94.35% compared to head (6248d98) 94.35%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #659   +/-   ##
=======================================
  Coverage   94.35%   94.35%           
=======================================
  Files           6        6           
  Lines        2446     2446           
  Branches      598      598           
=======================================
  Hits         2308     2308           
  Misses         85       85           
  Partials       53       53

Flag	Coverage Δ
unittests	`94.35% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
tasks.c	`94.83% <ø> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

n9wxu · 2023-04-06T23:13:53Z

FreeRTOS.h is failing uncrustify. Please take a look so we can wrap this PR up.

RichardBarry · 2023-04-08T23:07:40Z

portable/GCC/ARM_CM7/r0p1/portmacro.h

@@ -94,7 +94,7 @@

    #define portNVIC_INT_CTRL_REG     ( *( ( volatile uint32_t * ) 0xe000ed04 ) )
    #define portNVIC_PENDSVSET_BIT    ( 1UL << 28UL )
-    #define portEND_SWITCHING_ISR( xSwitchRequired )    do { if( xSwitchRequired != pdFALSE ) portYIELD(); } while( 0 )
+    #define portEND_SWITCHING_ISR( xSwitchRequired )    do { if( xSwitchRequired != pdFALSE ) { traceISR_EXIT_TO_SCHEDULER(); portYIELD(); } else { traceISR_EXIT(); } } while( 0 )


Nit only - I wonder if this could result in confusion when reading the resultant trace if the interrupts is preempted between the execution of the trace macro and the portYIELD.

portable/GCC/ARM_CM7/r0p1/port.c

RichardBarry

https://www.freertos.org/rtos-trace-macros.html and the reference manula also needs updating to document the new macros - which the PR author can't do themselves.

conara · 2023-04-23T11:27:34Z

Good point.

Skptak · 2023-05-22T23:11:26Z

Hey @conara, thanks for submitting this pull request! I wanted to reach out and ask if you wanted some help with closing out this PR? Or if you had planned on responding to the feedback and closing it out yourself?

kar-rahul-aws · 2023-09-05T14:47:39Z

Hi @RichardBarry
We have updated https://www.freertos.org/rtos-trace-macros.html with the new macro changes for this PR and will be visible after the deployment.
We are keeping these macros ,
traceISR_ENTER
traceISR_EXIT
traceISR_EXIT_TO_SCHEDULER
and not separate ones for SysTick interrupt, since as @conara mentioned , they are called from other ISRs.

conara · 2023-09-05T15:46:19Z

Thank you so much for helping me finalize my PR! Due to vacation I forgot this pull request. If I can help, let me know!

rawalexe · 2023-09-07T22:26:39Z

Just a concern here, why the Cortex-M7 port layer is being updated but no others. The M7 port is only for r0p1 cores which contain a bug - all other core versions should use the M4F port.

From,
Richard Barry.

The website has been updated to reflect the changes

Add trace hook macro for most ports In pull request #659 we introduced better support for tracing tools like systemview. This patchset adds support for more ports as requested in the original pull request.

Add Trace Hook Macros and function that returns the start of the stack.

d8ca0e1

conara requested a review from a team as a code owner April 5, 2023 11:41

n9wxu reviewed Apr 5, 2023

View reviewed changes

aggarg reviewed Apr 6, 2023

View reviewed changes

Remove obsolete functions.

8bae910

n9wxu previously approved these changes Apr 6, 2023

View reviewed changes

RichardBarry reviewed Apr 8, 2023

View reviewed changes

portable/GCC/ARM_CM7/r0p1/port.c Show resolved Hide resolved

RichardBarry previously requested changes Apr 8, 2023

View reviewed changes

Mancent linked an issue May 21, 2023 that may be closed by this pull request

> - [ ] ``` #678

Closed

kar-rahul-aws and others added 2 commits August 30, 2023 18:31

Merge branch 'main' into main

92cd939

Fix formatting

1ecf24c

kar-rahul-aws dismissed n9wxu’s stale review via 1ecf24c August 30, 2023 13:40

Fix formatting in port.c

dad7a5e

n9wxu previously approved these changes Aug 31, 2023

View reviewed changes

Merge branch 'main' into main

3121075

Merge branch 'main' into main

b500794

kar-rahul-aws dismissed n9wxu’s stale review via b500794 September 6, 2023 04:57

kar-rahul-aws and others added 4 commits September 6, 2023 05:21

Fix formatting portmacro.h

7b3efdd

Fix formatting after update branch

6f509d7

Fix formatting post resolve merge conflict

5f8c75d

Merge branch 'main' into main

fe2965e

chinglee-iot approved these changes Sep 6, 2023

View reviewed changes

n9wxu approved these changes Sep 6, 2023

View reviewed changes

Merge branch 'main' into main

6248d98

rawalexe merged commit 2f94b18 into FreeRTOS:main Sep 7, 2023
15 checks passed

conara mentioned this pull request Sep 18, 2023

Add trace hook macro for most ports #794

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Trace Hook Macros and function that returns the start of the stack. #659

Add Trace Hook Macros and function that returns the start of the stack. #659

conara commented Apr 5, 2023 •

edited

Loading

n9wxu Apr 5, 2023

conara Apr 6, 2023

n9wxu left a comment

aggarg Apr 6, 2023

conara Apr 6, 2023

conara commented Apr 6, 2023

codecov bot commented Apr 6, 2023 •

edited

Loading

n9wxu commented Apr 6, 2023

RichardBarry Apr 8, 2023

RichardBarry left a comment

conara commented Apr 23, 2023

Skptak commented May 22, 2023

kar-rahul-aws commented Sep 5, 2023

conara commented Sep 5, 2023

rawalexe commented Sep 7, 2023

Add Trace Hook Macros and function that returns the start of the stack. #659

Add Trace Hook Macros and function that returns the start of the stack. #659

Conversation

conara commented Apr 5, 2023 • edited Loading

Test Steps

n9wxu Apr 5, 2023

Choose a reason for hiding this comment

conara Apr 6, 2023

Choose a reason for hiding this comment

n9wxu left a comment

Choose a reason for hiding this comment

aggarg Apr 6, 2023

Choose a reason for hiding this comment

conara Apr 6, 2023

Choose a reason for hiding this comment

conara commented Apr 6, 2023

codecov bot commented Apr 6, 2023 • edited Loading

Codecov Report

n9wxu commented Apr 6, 2023

RichardBarry Apr 8, 2023

Choose a reason for hiding this comment

RichardBarry left a comment

Choose a reason for hiding this comment

conara commented Apr 23, 2023

Skptak commented May 22, 2023

kar-rahul-aws commented Sep 5, 2023

conara commented Sep 5, 2023

rawalexe commented Sep 7, 2023

conara commented Apr 5, 2023 •

edited

Loading

codecov bot commented Apr 6, 2023 •

edited

Loading