Tracing Improvements

Kernel
1

Hi!

After my last post and pr I have continued to work with the tracing API and found a few more details that I would like to address. Please let me know if you would be interested in PRs.

As a little background: I am in the process of re-working the practical exercises of the embedded systems course for my university, and I am in the process of building a simple tracer, so students can visualize the operation of FreeRTOS. I plan to open-source the tracer when completed. The course will likely also be published when in a presentable state.

Idle/Timer Service task identification

Problem

When started, the kernel creates a timer service task and idle task for every core, using the normal task creation API. This causes the normal traceTASK_CREATE hooks to be called. There is, however, currently no way for the tracer to identify these specific tasks as the system tasks. Their name is configurable and not guaranteed to be unique so it cannot reliably be used to identify them.

A tracer could require the user to manually identify them using xTaskGetIdletTaskHandle and xTimerGetTimerDaemonTaskHandle, but this is error-prone and annoying because it has to be done in some thread after the kernel has started.

Implementation Idea Option 1:

It would be nicest to have the ‘type’ of task (normal/timer/idle) reported when the traceTASK_CREATE macro is called. An ‘extended’ version of this macro could be included that

Unfortunately this would be rather difficult to implement: The ‘special’ tasks are created using the public task creation functions, so no extra information can be passed along. There is also no clear way to identify if a given task is a timer or idle task at the location of the traceTASK_CREATE macro as far as I can tell. I guess it would be possible to introduce a static/global variable to track this, but that is also messy.

Implementation Idea Option 2:

Alternatively, after the task is created, new trace macros could be introduced that identify a given task as the timer service or idle task:

#define traceTASK_IS_IDLE_TASK( pxNewTCB, xCoreID )

#define traceTASK_IS_TIMER_TASK( pxNewTCB )

With this style it could also be useful to introduce a macro that identifies the timer cmd queue:

#define traceQUEUE_IS_TIMER_QUEUE( xTimerQueue )

This change would be completely backwards compatible and not require any refactoring.

Notfiy does not expose xClearCountOnExit

Problem

This problem is very similar to the discussion in the previous post about queue send tracing. The traceTASK_NOTIFY_TAKE macro does not expose the value of xClearCountOnExit, so a tracer cannot determine the state of the task notification value after they have been taken - especially because it is updated after the macro is called.

Implementation Idea

As was done for traceQUEUE_SEND, I think it makes sense to introduce a traceTASK_NOTIFY_TAKE_EXT hook that exposes the current value of ulNotifiedValue[ uxIndexToWaitOn ] and the value of xClearCountOnExit:

#ifndef traceTASK_NOTIFY_TAKE_EXT
    #define traceTASK_NOTIFY_TAKE_EXT( uxIndexToWaitOn, ulNotifiedValue, xClearCountOnExit  ) traceTASK_NOTIFY_TAKE( uxIndexToWaitOn )
#endif

// in ulTaskGenericNotifyTake, line 7682:
taskENTER_CRITICAL();
{
    ulReturn = pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ];
    traceTASK_NOTIFY_TAKE_EXT( uxIndexToWaitOn, ulReturn, xClearCountOnExit );

    if( ulReturn != 0U )
    {
        if( xClearCountOnExit != pdFALSE )
        {
            ...

TaskNumber vs TCBNumber API confusion

Problem

The TCB currently contains two tracing numbers:

typedef struct tskTaskControlBlock
{
    // --- snip ---

    #if ( configUSE_TRACE_FACILITY == 1 )
        UBaseType_t uxTCBNumber;  /**< Stores a number that increments each time a TCB is created.  It allows debuggers to determine when a task has been deleted and then recreated. */
        UBaseType_t uxTaskNumber; /**< Stores a number specifically for use by third party trace code. */
    #endif

    // --- snip ---

} tskTCB;

The comments here are accurate: The uxTaskNumber member is not used by the kernel and can be set/retrieved by the user with the following two methods:

UBaseType_t uxTaskGetTaskNumber( TaskHandle_t xTask );
void vTaskSetTaskNumber( TaskHandle_t xTask, const UBaseType_t uxHandle );

The uxTCBNumber member is initialized on task creation to a unique ID which is created by incrementing a global counter that is (confusingly) called uxTaskNumber.

The public API confusion starts with vTaskGetInfo, which in the returned TaskStatus_t struct provides a field called xTaskNumber:

typedef struct xTASK_STATUS
{
    // --- snip ---

    UBaseType_t xTaskNumber; /* A number unique to the task. */

    // --- snip ---
} TaskStatus_t;

While the comment makes sense, the name is rather confusing because this field is used to return the pxTCB->uxTCBNumber member:

pxTaskStatus->xTaskNumber = pxTCB->uxTCBNumber;

This is rather confusing:
- Call vTaskSetTaskNumber(mytask, 42);
- uxTaskGetNumber will return 42.
- The xTaskNumber field of a task status struct, when queried, will return
some other number.

Also it is rather inefficient for tracers that currently the only API to access the value of uxTCBNumber outside of tasks.c is through vTaskGetInfo, requiring all status struct members to be fetched.

Implementation Idea: Provide a method to get uxTCBNumber directly

I suggest adding a dedicated uxTaskGetTCBNumber function to provide access to
the uxTCBNumber TCB member:

#if ( configUSE_TRACE_FACILITY == 1 )

    UBaseType_t uxTaskGetTCBNumber( TaskHandle_t xTask )
    {
        UBaseType_t uxReturn;
        TCB_t const * pxTCB;

        traceENTER_uxTaskGetTCBNumber( xTask );

        if( xTask != NULL )
        {
            pxTCB = xTask;
            uxReturn = pxTCB->uxTCBNumber;
        }
        else
        {
            uxReturn = 0U;
        }

        traceRETURN_uxTaskGetTCBNumber( uxReturn );

        return uxReturn;
    }

#endif /* configUSE_TRACE_FACILITY */

This allows tracer code to access the unique task TCB ID in other places except trace hooks inside tasks.c.

Implementation Idea: Cleanup the task status struct.

I think it makes sense for vTaskGetInfo to return both the TCB number and task number and be less confusing, but would look for your input on how this should be done best.

I would assume changing the behavior of vTaskGetInfo to return TCB->uxTaskNumber in xTaskNumber and introducing a new xTCBNumber field is a breaking change and not desired? Maybe updating the documentation of the xTaskNumber info struct member to be very clear that this is a different task number than the one that can be set/gotten using the tracing functions, and introducing a second variable called xUserTaskNumber is acceptable? It is still rather confusing that way…

Implementation Idea: Rename the global uxTaskNumber counter to uxTCBNumber

This is absolutely not necessary, so again I am looking for your feedback if renaming the counter that is used to assign TCB numbers from uxTaskNumber to uxTCBNumber is acceptable. Some of its documentation references that it is used by kernel-aware debuggers, so presumably this would break things?

Queue Set identification

Problem

Queue Sets use an extra queue to track the state of all member queues. At the moment, this queue is created with queue type queueQUEUE_TYPE_SET, but this has he same value as queueQUEUE_TYPE_BASE:

#define queueQUEUE_TYPE_BASE                  ( ( uint8_t ) 0U )
#define queueQUEUE_TYPE_SET                   ( ( uint8_t ) 0U )
#define queueQUEUE_TYPE_MUTEX                 ( ( uint8_t ) 1U )
#define queueQUEUE_TYPE_COUNTING_SEMAPHORE    ( ( uint8_t ) 2U )
#define queueQUEUE_TYPE_BINARY_SEMAPHORE      ( ( uint8_t ) 3U )
#define queueQUEUE_TYPE_RECURSIVE_MUTEX       ( ( uint8_t ) 4U )

This makes it impossible for tracers to properly identify queue sets as queue sets.

Proposed Solution

As far as I can tell, the queueQUEUE_TYPE macros and their inclusion in the queue struct are only present for tracing, and changing queueQUEUE_TYPE_SET to be of value 5U does not break anything in the kernel and the CMock test suite continues to pass.

I am unsure, however, if this is an acceptable breaking change, and if this could only be implemented with some sort of backwards compatibility in mind. Possible the queue set type could be 0x80, and all calls to “legacy” tracing hooks could mask the queue type with 0x7F? Or is this kind of breaking change acceptable?

It would also be possible to introduce a new separate hook to identify queues as queue sets traceQUEUE_IS_QUEUE_SET but that seems rather messy considering that there already is a mechanism for tracing the type of a queue.

Adding an equivalent of uxTCBNumber to other FreeRTOS constructs

Problem

Currently, only tasks contain both a uxTaskNumber that is set by the tracer and a uxTCBNumber that is automatically uniquely assigned by the kernel.

The uxTCBNumber is rather useful for tracers, as it provides a unique ID that a given task can be identified with.

All other constructs (queues/semaphores/mutex/queue set, stream/message buffers, timers, event groups) only feature a number that has to be manually set by the tracer.

Proposed Solution

Add a unique ID to queues, stream buffers, timers, and event groups that mimics that functions as the uxTCBNumber does for tasks: A global counter is incremented to generate new IDs for each new construct.

A new function (gated being configUSE_TRACE_FACILITY) would be added for each of the above to allow the ID to be queried.

As to what this field should be named, I am open to suggestions. id? Stick with the same style as before and use MCBNumber for queues?

I would use separate counters for queues, stream buffers, timers, and event groups but share the timer for each ‘subtype’ - In other words, Queues, semaphores, mutex, etc would share a counter and never have the same IDs. This seems to be the most logical behavior, as, sticking with the example of queues, most trace events are shared between all queue types. If Semaphores and queues had separate ID counters, each one of those shared tracing hooks would have to use the queue type and queue ID to uniquely identify the queue.

Having to assigning these unique IDs in the tracer is not a huge problem, but including it into the kernel seems sensible because it creates parity between all ‘objects’ that a tracer may track. Furthermore, it opens up the ‘number’ struct member for the tracer to store other metadata. One could imagine enabling/disabling tracing on a per-queue basis, or assigning queues to different categories for example.

The only downside I see is a minimal increase in memory usage (1 UBaseType_t per construct, if configUSE_TRACE_FACILITY == 1).

Sorry for the long message and trouble I am causing. I put this all together into a long post because I did not want to spam the form with multiple posts.

Please let me know what you think!

All the best,
Philipp

1 Like
2

On further thought, maybe the issues I am having with tracing the state of queues (previous post/pr) and notifications (this post) are a more general problem worthy of a more general solution.

There are (to my eyes) four FreeRTOS constructs that contain some state which is updated, retrieved, and blocked on by different threads that a tracer may be interested in tracking:

  • Queues (including all types/variants): Number of times in queue
  • Stream buffers (including message buffers): Number of bytes in queue
  • Event groups: State of the event bits
  • Direct to task notifications: State of the task notifications

Generally there are two strategies a tracer can do this when the firmware interacts with them:

  • Query and trace/transmit their current state. Let’s call this “tracking state”.
  • Trace/transmit what was done to them, and infer their new state based on this. For example, if a queue has N items and
    an item is being received, it now contains N-1 items. Let’s call this “tracking state changes”.

Which is appropriate depends on objectives of the tracer. The first likely requires sending more data, while the second is vulnerable to missing events and does not easily allow starting a tracer after the firmware has already run for some time. Likely a hybrid approach is best, which only sends the absolute state sometimes and then relies on state change events throughout.

For the four RTOS constructs above, here are the (public) API calls that can be used to modify their internal state (not including creating/deleting them):

  • Queues:
    • xQueueSend() and xQueueSendFromISR() (including xQueueSendToBack() and xQueueSendToBackFromISR())
    • xQueueSendToFront() and xQueueSendToFrontFromISR()
    • xQueueReceive and xQueueReceiveFromISR()
    • xQueueReset
    • xQueueOverwrite and xQueueOverwriteFromISR()
    • xQueueSelectFromSet() and xQueueSelectFromSetFromISR() (which modify the queue set tracking queue)
  • Stream Buffers:
    • xStreamBufferSend() and xStreamBufferSendFromISR()
    • xStreamBufferReceive() and xStreamBufferReceiveFromISR()
    • xStreamBufferReset()
    • xMessageBufferSend() and xMessageBufferSendFromISR()
    • xMessageBufferReceive() and xMessageBufferReceiveFromISR()
    • xMessageBufferReset()
  • Event Groups:
    • xEventGroupWaitBits()
    • xEventGroupSetBits() and xEventGroupSetBitsFromISR()
    • xEventGroupClearBits() and xEventGroupClearBitsFromISR()
    • xEventGroupSync()
  • Direct to task notifications:
    • xTaskNotifyGive() and vTaskNotifyGiveFromISR()
    • xTaskNotifyTake()
    • xTaskNotify() and xTaskNotifyFromISR()
    • xTaskNotifyAndQuery() and xTaskNotifyAndQueryFromISR()
    • xTaskNotifyWait()
    • xTaskNotifyStateClear()
    • xTaskNotifyValueClear()

I think it would be sensible to ensure that each of the above emits some tracing event that includes the following pieces of information:

  • Pointer to the handle of the resource that got modified
  • Type of modification that was made (Queue send vs overwrite, clear on exit or not etc)
  • The state of the resource, either before or after the above modification was made (which should be clearly documented or obvious).

I have not yet gone through all of the above, but in most cases this is either already possible, or could be provided by adding an _EXT tracing hook that falls back on the previous tracing hook.

It would be nice to be consistent and have all hooks be called with the state after modification (or all called with state before modification). As far as I can tell most tracing hooks are positioned before the state of the resource is modified, but I have not looked exhaustively. If there are any that do not cleanly fit this, I would keep them at their current location and cleanly document their behavior.

As a rough example, I imagine hooks like this:

#ifndef traceQUEUE_SEND_EXT
    #define traceQUEUE_SEND_EXT( pxQueue, xCopyPosition, uxItemsInQueueBeforeSend )    traceQUEUE_SEND( pxQueue )
#endif

The hook provides the handle, type of modification (xCopyPosition) and state before the modification (uxItemsInQueueBeforeSend). Not all hooks would require all three: A call to traceSTREAMBUFFER_RESET does not need to specify the operation and/or state - it is clear.

If there is interest in this, I am happy to have a go at implementing it. In that case, maybe hold off on merging the PR for queue tracing because it likely will change in that case.
Thanks!
Philipp

3

For your “problem” #1, I fail to see a problem there. From the point of view of the kernel, there IS nothing special about at least the timer task; as you correctly point out, both are created using the same functions as application tasks and thus enjoy no privileges over them.

As far as the timer task is concerned - from my understanding, it is purely optional; an application not using software timer services will not need that task at all. It simply provides a context to serialize software timers in which can be registered optionally.

The idle task is necessary because there may be times in which no task is ready to run at all, so eating unnecessary CPU cycles can take place here (for targets without sleep options that must execute some code at every time). This task was chosen to also do some cleanup on behalf of the system, but that is somewhat arbitrary.

In any case, I do not see a need to make any changes to assign those tasks special treatment which they do not have not deserve. All tracers/visualizers I have seen so far list those by their names which are fairly self explanatory. As the user of such add on packages, I have never felt that there is any added value in distinguishing them from other tasks.

Or am I missing something here?

4

Hi!

Thanks for your feedback!

I apologize if my wording was aggressive. I don’t mean to say that these are actual problems that prevent the FreeRTOS kernel from doing its job. I also don’t expect to go on this forum, complain about small details that I perceive to be problems and see them fixed by the FreeRTOS team - I am happy to do the work, I just wanted to ge a sense if there is interest for these changes to be upstreamed.

Absolutely. I am aware that - to the kernel - they are simply normal tasks that run like all others. The only difference is that they are created automatically by FreeRTOS when the kernel starts.

To be clear: I don’t suggest these tasks to receive any special treatment. They will still be created by the public task creation API as they are now. I only would add an extra trace hook after that is done that identifies them as the timer or one of the idle tasks after they have been created. This hook changes nothing about how the kernel works.

I agree that it does not make much difference to ‘label’ the timer task. I was mostly looking to properly identify the IDLE tasks: I plan to support tracing SMP implementations, which result in multiple IDLE tasks with identical names.

In my tracer I want to show both the currently active task on each core as most tracers do, and also provide a CPU core overview where the IDLE tasks are excluded/highlighted in a different color to visually indicate the amount of time spent performing “work” vs idling. This would be much more robust if the kernel allowed me to identify those tasks instead of relying on something a little flaky like task name + core affinity.

In that light it seems sensible that if one set of tasks created by the FreeRTOS kernel get an identifying hook, all do. That is why I suggested also including a hook for the timer task.

But in all I agree - my “problem” 1 is not super critical. It is a nice-to-have that I don’t think has many (any?) downsides.

All the best,
Philipp

5

I should add that this tracer is for an Embedded Systems university course. The function of the IDLE task may be obvious to an expert, but I have found that students appreciate having both a ‘simplified’ view to get an intuitive understanding, and the full proper tracer view to dig into the details.

6

Wouldn’t it work if you used the configIDLE_TASK_NAME macro in your tracer. In that case, it would not matter whether this name is configured by the user or not.

Just to be double sure, another solution is to access these members in the expansion of the macro? The reason I ask is because we need to assess that we are not making macros one use case specific.

I agree that the name is confusing when you read it with the definition of TCB. We can explain more in the comment.

Can you not access this member directly as it is meant for tracing tools?

Agree - as I mentioned above, we should update this comment.

I am not so keen on adding this new member as we already have getter and setter for those. Are those not enough?

Changing it to 5 seems okay to me.

This probably seems reasonable to your use case and may not seem so for other use cases. I’d rather have the tracer set and get a ID/number using the provided APIs as per their needs.

1 Like
7

Given that that other approach of querying and sending state info avoids this, is this optimization even worth it?

Since it is going to touch a lot of trace macros, let me talk to the team and get back to you.

Thank you for the detailed post!

1 Like
8

Thank you for the detail in your post. I look forward to seeing your tracer.

The “task counter” Vs “TCB counter” confusion is a legacy issue dating to when only one of the two existed. We can change the names without breaking kernel aware debuggers that access these symbols directly. I think you will find a check-in somewhere that did just that which had to be reverted. Hopefully the code doesn’t expose the confusion externally to an application, only internally, which is why you see the “get info” function return what on the face of it looks like the wrong structure member.

Aggarg@ already mentioned this, but the normal way of obtaining an object’s state (for example, the event bits that are set, or the number of items in a queue) is to access the object directly. The macros are incline, so can access anything they want provided you have the handle that points to the object’s implementing structure. The downside of direct access is coupling to the C code’s implementation – as already described above where we can’t change the name of the task and TCB counter structure members without breaking third party debuggers. It should be noted these debuggers don’t use the macros, just the pointers to the C structures so they can access the structures directly.

I also don’t, from a quick look, see any problem with giving queue sets a unique type number, which would be done as a secondary start after the queue is created. I don’t think that will break any existing debuggers.

I also share the view that, provided you know the names of the idle tasks, you will be able to determine which tasks they are.

1 Like
9

Timer/Idle Task identification

The tracer has access to configIDLE_TASK_NAME so a comparison is possible. My concern is that there is no guarantee that no other task is given the same name. I don’t know who common it would be to, in an effort to save memory, set configMAX_TASK_NAME_LEN to zero?

Having the tracing hook, in my eyes, would remove this ambiguity at no cost. Admittedly it is not a huge deal and having some documentation about this in the tracer usage instructions (keep task names unique, esp. don’t name another task configIDLE_TASK_NAME) and is very unlikely to ever come up. My perspective is that this tracer will be (hopefully) used by around 300 students every year, so I am afraid of nothing more than possible foot guns - my experience shows that students have an incredible ability to find such edge cases :slight_smile:

But if it is your judgment that adding an traceTASK_IS_IDLE_TASK( pxTCB, uxCoreNum ) after the idle tasks are created is not fit for FreeRTOS I understand :slight_smile:

Changing queueQUEUE_TYPE_SET

Great. Noted. I will open PRs for this (and all other matters) when discussion seems to have stopped to avoid creating more PR load from changing stuff back and forth.

Renaming/Documenting xTaskNumber member in TaskStatus_t

Unfortunately, I think this confusion is exposed in the public API here - if you count vTask{Set,Get}TaskNumber as public API (which I am not 100% clear about. There is no API documentation on the API docs website, but the header file does not qualify it as ‘for internal use only’, like is done, for example, with the queueQUEUE_TYPE_* macros): As a user, if I call vTaskSetTaskNumber, I would expect that number to also be returned in TaskStatus_t, which it is not.

Anyway - it seems that renaming the xTaskNumber member is TaskStatus_t is not an option, so improving the documentation/comment is the best option. I have also noted that down for a future PR.

Renaming the global uxTaskNumber counter used to set uxTCBNumber

To be clear - does that mean you suggest I can open a PR to rename it? If so I am happy to open one. If not, that is just one of those internal inconsistencies we have to live with :slight_smile:

traceTASK_NOTIFY_TAKE_EXT, tracing macro semantics, and tracing state of RTOS resources

Tossing these three together, because they are related:

The issue here (I think) is that we don’t have a clear idea of what the tracing macro ‘semantics’ are. Richard touches on this:

I presumed that the ‘idea’ or ‘contract’ with the tracing macros was that all the variables they may access are either passed as argument (like with pxNewTCB in the case of traceTASK_CREATE( pxNewTCB )) or are clearly documented as being intended to be accessed from within the macro:

/* Called before a task has been selected to run.  pxCurrentTCB holds a pointer
 * to the task control block of the task being switched out. */
#define traceTASK_SWITCHED_OUT()

This limits the ‘exposure’ of the tracing macro to accessing what it is explicitly given or what is explicitly documented as being available. Other wise all tracing macros could simply not take any arguments at all.

If this is not the ‘semantics’ that are intended for tracing hooks, please let me know. Many of my suggestions are based on the fact that not enough information is exposed through this ‘semantic’ to fully allow the tracer to understand what is happening.

For a moment, instead of looking at traceTASK_NOTIFY_TAKE consider the somewhat awkward-to-define set of ‘tracing hooks that indicate that the state of some RTOS resource has been modified’ (which includes crowd favorites such as traceQUEUE_RECEIVE() that indicates a queue has one less item in it, or traceEVENT_GROUP_CLEAR_BITS, or many others that I tried to list in my second post).

In my eyes, all such tracing hooks should, through the semantics of ‘accessing stuff it has been explicitly given or documented as having access to’, be able to determine:

  • Which resource changed
  • How it was changed
  • What its state is after it has been changed.

This does not have to be this explicit. The traceQUEUE_SEND_EXT hook in my open PR, for example, only directly exposes which resource is being changed (including its original state) and how it is being changed:

traceQUEUE_SEND_EXT( pxQueue, xCopyPosition );

That is sufficient for the tracer to figure out all three pieces of information above. The queue is clear from pxQueue, how it is being changed is clear from xCopyPosition, and its state after the change can be inferred using its original state in pxQueue and how it is being changed through xCopyPosition.

This is not the case for traceTASK_NOTIFY_TAKE:

taskENTER_CRITICAL();
{
    traceTASK_NOTIFY_TAKE( uxIndexToWaitOn );
    ulReturn = pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ];

    if( ulReturn != 0U )
    {
        if( xClearCountOnExit != pdFALSE )
        {
            pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] = ( uint32_t ) 0U;
        }
        else
        {
            pxCurrentTCB->ulNotifiedValue[ uxIndexToWaitOn ] = ulReturn - ( uint32_t ) 1;
        }
    }
    // -- snip --
}

Firstly, the tracing hook does not directly know how the identity of the task that is being updated, but it can query this using xTaskGetCurrentTaskHandle. From there and uxIndexToWaitOn it knows the initial state. But it cannot determine how it is being changed and what the final value will be, since xClearCountOnExit is not exposed and since the value is updated after the macro gets called. This means that, at a minimum, the tracing hook needs explicit access to xClearCountOnExit to do its job.

I would petition that is also sensible to explicitly provide access to pxCurrentTCB, either via allowing it in documentation or passing it as an argument, because I dislike the fact that a tracing hook has to rely on xTaskGetCurrentTaskHandle to do its job. After all, it is being called in a critical section and the documentation for critical sections state:

FreeRTOS API functions must not be called from within a critical section.

I have not yet gone through all the 'tracing hooks that indicate that the state of some RTOS resource has been modified` to see if they are all “up to my expectations”, so it is probably good to have this fundamental discussion now and avoid me bugging you with similar problems if that is not how you see tracing hooks working.

To your point:

I think it is reasonable to provide tracing hooks with all relevant information (the resource that changed, how it has changed, and what its new state will be). This leaves tracers open to use whatever implementation they choose.

Adding a getter for uxTCBNumber

This only works if it is being used in tracing hooks. I think it is valid to assume that because uxTCBNumber exists, tracing tools may use it as an ID to identify a given task with.

I was experimenting with the idea of creating a function to manually allow the user to identify the idle task with (yes I know… I know… you all don’t mind relying on the task name…). Something like:

trace_task_is_idle_task(xTaskGetIdleTaskHandle());

The issue is that now this function is not called from tasks.c and does not have direct access to uxTCBNumber except by going through TaskStatus_t. Arguably the performance hit is not relevant.

Similarly, I wanted my tracer to allow users to manually instrument certain interrupts (that even may be of high enough priority to not be RTOS relevant):

void my_isr() {
    trace_isr_enter(ISR_ID_MY_ISR);
    foo();
    trace_isr_exit(ISR_ID_MY_ISR);
}

If those tracing hooks wanted to access uxTCBNumber to identify which task was being interrupted, they would have to go through TaskStatus_t. Here I really would like to avoid need such a ‘heavy’ approach.

Providing a getter for uxTCBNumber would get around this.

Access to the ‘real’ task number in ‘TaskStatus_t’

Fair. It was mostly intended as “if we are going give access to one, why not give access to both for a simpler API” but its not critical.

Adding counters + unique IDs to other RTOS resources, not just tasks.

Fair. It is not a huge concern for me. Again, it was mostly intended to have a “symmetrical” tracing API for everything FreeRTOS.

Thank you all for taking the time to work with me here - I have never built a tracer before, so I am sure that some of the ideas I raise are symptoms of my lack of experience. I am more than happy to accept your judgment calls on all of the above.

Mean while the tracer is coming along:

image
image1266×784 64.6 KB

Thank you again and all the best,
Philipp

1 Like
10

From a seperate post:

Seems there would be a space for a free/foss tracing tool without direct hardware dependencies:)

1 Like
11

Yeah…
But I’m doing research kind of thing from my days but can’t find it.

12

Thanks.

Thanks again.

I think Richard meant that we cannot rename it as we had to revert the change earlier.

Will get back to you on this.

The challenge here is to define the information that is enough for all tracers. @kristoffer_percepio , do you have any input on this as you guys have been using these macros?

I think you can access pxCurrentTCB and do not need to call xTaskGetCurrentTaskHandle.

I am trying to figure out if it is possible to define what “all” of the information will be.

Why not use uxTaskNumber for that purpose which the tracer can both set and retrieve?

I am not sure if this is the best solution for your case but you can use configINCLUDE_FREERTOS_TASK_C_ADDITIONS_H to insert code in tasks.c:

  1. Define configINCLUDE_FREERTOS_TASK_C_ADDITIONS_H in your FreeRTOSConfig.h.
  2. Create a file named “freertos_tasks_c_additions.h” and put your code in it.

This really looks good. I am looking forward to seeing it.

Thank you again for writing such detailed posts.

1 Like
13

Great. Thank you again for taking the time to work with me on this.

Absolutely - my point is that this comes back to the discussion of the semantics above. In my eyes - and again, maybe I am misinterpreting how the trace hooks should be used - because pxCurrentTCB is neither passed as an argument nor explicitly mentioned in the comments as being accessible, it seems fragile/hacky rely on accessing it.

Fair point - I was using uxTCBNumber because it was built in and available, but if the bookeeping for generating new IDs anyways has to be done for queues, I may as well use that for tasks as well.

Reading into the SMP stuff more, it seems like reducing the amount of global state that could not be protected with resource-specific locks is a good thing.

Oh! I did not realize that this was available. That is very good to know. I guess that takes the “pressure” of my point to adding a getter for uxTCBNumber, because this is an escape hatch.

I presumed so. Understood.

Thank you again for taking the time!

14

I think it is okay to take a dependency on it OR else you can continue using TaskGetCurrentTaskHandle.

I discussed it within my team and we are okay to take this suggestion. Please raise it in a separate PR as we would like to ensure that we can implement Percepio TraceRecorder using the new macros without needing access to any other variable. This is just to build confidence that the new versions of macros are passing enough information in parameters.

Please raise a PR for this too.

Thank you!

15

Great!

I will get working on PRs. There is some travel coming up on the weekend/next week, so it may take a few days for me to go to them.

1 Like
16

Hi!

After returning from my travels I have worked through most of the changes discussed above. See the following PRs:

The first 3 are smaller changes, while the last contains the bulk of the proposed tracing improvements. I have marked it as draft as I expect there to be some changes/feedback still, and there are some points I would like your thoughts on. Please see the PR.

Thank you again!

2 Likes
17

Thank you for your contributions!