Add support for SMP on FreeRTOS

[hwmland]

This PR resolves #14 and resolves #43

[hwmland]

I have close to 0 experience with pull requests on github, similar with VS code extension developement and typescript... so I'd be very gratefull if someone could help me with the process.
I checked those changes with FreeRTOS project on RP2040, both single-core and multi-core - both work.

[PhilippHaefele]

@hwmland First of all thank you very much for your contribution.

I did have a quick look look over the code and seems to be ok in general (no real world testing and FreeRTOS code check done).

Still @haneefdm and my concerns that we should properly support SMP (multiple cores) are not addressed #14 (comment). I do need to check latest FreeRTOS code and see if anything general changed in the meanwhile (e.g. removal of old pxCurrentTCB) which maybe would admit an intermediate / incomplete adaption.

What was the issue with #43? Which part of your changes fixed it? I personally can't see a relation in the changes.

[PhilippHaefele]

Just did check the code and when we do not have more than one core via configNUMBER_OF_CORES (this is most likely what also disables other SMP stuff) the old pxCurrentTCB is used. Or are there other configuration flags for this?

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/1947dd2f94419d368316052e92f4bd396bce5f55/tasks.c#L444

So the extension/FreeRTOS part should build up multiple tables, one for each core. When there's only one core we should not have an issue regarding the general functionality (maybe other things have changed that result in problems like #43, which should be addressed in a separate PR).

Also when using only one core with a core configuration of > 1, we maybe need to use the second instance in the array (haven't checked how cores are addressed in pxCurrentTCBs in that case.

[haneefdm]

So the extension/FreeRTOS part should build up multiple tables, one for each core.

Ummm. With SMP there should still be one table. That is my thought. SMP means that the RTOS will schedule a thread to any available core (with possible affinity or binding). To me, that is what SMP means. Or, does FreeRTOS not work like how normal SMP works for CPUs and Desktop OSes

If there is one instance of RTOS, there should be one table. That is another thought.

This is why I was having trouble tackling this. I need to understand how this actually works. I didn't want to guess. I don't have a test case and the right HW was also another problem.

[hwmland]

Hello, I'll try to answe to all remarks:)

Still @haneefdm and my concerns that we should properly support SMP (multiple cores) are not addressed #14 (comment).

I think I did so. I show proper status of all threads, for running threads I show on which core it runs RUNNING(x)

(e.g. removal of old pxCurrentTCB)

pxCurrentTCB (is/was used just for detection of RUNNING thread in single-core builds) is replaced by pxCurrentTCBs in SMP. I use it just for detection os single/SMP. Running state information is in the case of SMP additionaly present in TCB.

Just did check the code and when we do not have more than one core via configNUMBER_OF_CORES (this is most likely what also disables other SMP stuff) the old pxCurrentTCB is used. Or are there other configuration flags for this?

Generally port defines FREE_RTOS_KERNEL_SMP in cmake, but in the code configNUMBER_OF_CORES is used for single/SMP detection, where configNUMBER_OF_CORES == 1 falls-back to non-SMP build (old situation) There are few more settings that could affect scheduling (like configRUN_MULTIPLE_PRIORITIES, configUSE_CORE_AFFINITY, configTICK_CORE), but I think nothing really interesting for rtos-view.

Ummm. With SMP there should still be one table. That is my thought. SMP means that the RTOS will schedule a thread to any available core (with possible affinity or binding). To me, that is what SMP means.

Exactly this way it works. There are still 'common' lists of threads (blocked, ready, ...) for both cores, scheduler is just able to schedule them on multiple cores, so there could be more threads in the running state.

I didn't want to guess. I don't have a test case and the right HW was also another problem.

There are now 2 oficiall examples (XCORE AI, RPi Pico), where the lates is very cheap and well available option (about $4 + shipping) in official distributors, around the world.

[haneefdm]

There are now 2 oficiall examples (XCORE AI, RPi Pico), where the lates is very cheap and well available option (about $4 + shipping) in official distributors, around the world.

Can you create an actual application and put it on github.

• It should say what HW it needs
• Build instructions - hopefully for any OS. My goto machine is a Mac but Linux is okay
• Brief description of what this application does.

Unlike other extensions I own/support, this is a unique one. Anyone who contributes has to do that is it is impossible to know all the different OSes.

[hwmland]

Can you create an actual application and put it on github.

• It should say what HW it needs
• Build instructions - hopefully for any OS. My goto machine is a Mac but Linux is okay
• Brief description of what this application does.

Sure, I’ll. Just some questions to be sure:

• It’ll support ‘any’ RP2040-based board. I’ll use official Pico board, but any Chinese/adafruit/…. board should do the trick (I’ll mention it there of course)
• There are very good documents from RPi how to setup build/debug environment for Linux (both RaspberryPi and ‘normal’ PC), Mac and Windows (I use windows, but this is the most complex to setup). Hope this is enough (I’ll link documentation from the project)
  • Building FreeRTOS project requires additional manual step after checking-out the project (FreeRTOS is sub-module there). Those are just few command-line command, I’ll include them in build instructions of course.
  • Project will be for prepared in Visual Studio Code (what else 😊 ).
  • Debugging will not be described as there it depends very much on debug hw you use (I’m using PicoProbe built from another pico, but this is most complex for setup – you have to build your own OpenOCD). But most probably anything you have should be fine, RP2040 is just ARM with SWCLK/SWDIO. Again – description by Raspberry is quite good I think if you decide to use 2nd pico for debugging.
• to have the application as simple as possible, I’ll include just few tasks doing ‘nothing’ useful. Just some combination of sleep/busy-wait and some simple synchronization (to have as well ‘blocked’ task) Is it OK? (description what which task does included of course)
• I’ll describe of course configuration changes necessary to switch between single-core/SMP, so people could test both scenarios on RP2040 as I’m doing.

[PhilippHaefele]

I think I did so. I show proper status of all threads, for running threads I show on which core it runs RUNNING(x)

Sorry missed that on my smartphone 🫢.

And of course in SMP we're good with one table. Mixed AMP, SMP and SMP with core pinning somehow in my head.
Seems it was too late to respond for me yesterday.

@haneefdm Thanks for jumping in here.

I do have some RP2040 boards lying around, so I'm happy to help with testing once the example project ist setup 😃

[SoftTransistor]

Hello,
I would gladly use this feature, so I thought I'd try to help as well.
I already have a setup using FreeRTOS SMP on a RP2040 based custom board.

Running the application with the modified extension provide indeed more information than before. The os is now correctly detected as well as the different tasks :

I don't know enough on how freertos works to say anything relevant about the code, but I can run tests if it helps.

[hwmland]

Example project with simple build instructions are here

[hwmland]

Multicore setup example result:

Single-core example result:

[opetany93]

some updates? is it going to be merged?

[hwmland]

Well - last merge to this repo is March 2023... If you want it, I'm afraid you have to fork and be on your own...

[PhilippHaefele]

Will try to test this soon. Was very busy and @haneefdm also (see Marus/cortex-debug#1027)

[USTHzhanglu]

hi @PhilippHaefele ,i try this at my esp32 board which use freertos. it use smp so mainline can`t match rtos type.

but @hwmland esp32 also have double core, and it all show RUNNING(undefined),maybe some err with other chip

[hwmland]

Hello, for whatever reason it does not read xTaskRunState from TCB here:

const xTaskRunState = thInfo['xTaskRunState']?.val;

I don't have environment set for esp32 nor time now to do so now (and honestly no motivation anymore as there is no view to progress with rtos-views now). If you want to investigate yourself and perhaps improve this PR - try to check why xTaskRunState is not there. Just put breakpoint after this block:

const thInfo = await this.getExprValChildrenObj(
                                `((TCB_t*)${RTOSCommon.hexFormat(threadId)})`,
                                frameId
                            );

and check what is awailable in thInfo - perhaps this explains us something...

Edit: additional though: @USTHzhanglu, do you have perhaps link to repo with your FreeRTOS you are using? Perhaps we'll see there something obvious in TCB. Please check as well how you defined configNUMBER_OF_CORES

[USTHzhanglu]

Hello, for whatever reason it does not read xTaskRunState from TCB here:

const xTaskRunState = thInfo['xTaskRunState']?.val;

I don't have environment set for esp32 nor time now to do so now (and honestly no motivation anymore as there is no view to progress with rtos-views now). If you want to investigate yourself and perhaps improve this PR - try to check why xTaskRunState is not there. Just put breakpoint after this block:

const thInfo = await this.getExprValChildrenObj(
                                `((TCB_t*)${RTOSCommon.hexFormat(threadId)})`,
                                frameId
                            );

and check what is awailable in thInfo - perhaps this explains us something...

Edit: additional though: @USTHzhanglu, do you have perhaps link to repo with your FreeRTOS you are using? Perhaps we'll see there something obvious in TCB. Please check as well how you defined configNUMBER_OF_CORES

thanks for answer.i don't know why i configNUMBER_OF_CORES 2 but TCB_t * not use xTaskRunState.it's a hard task to find the resion. but may we can avoid undefined like this

                            if (this.pxCurrentTCB !== null) {
                                threadRunning = threadId === this.curThreadInfo;
                                mySetter(DisplayFields.Status, threadRunning ? 'RUNNING' : state);
                            } else {
                                const xTaskRunState = thInfo['xTaskRunState']?.val;
                                if (xTaskRunState !== undefined) {
                                    if (xTaskRunState === '-2') {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, 'YIELD');
                                    } else if (xTaskRunState === '-1') {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, state);
                                    } else {
                                        threadRunning = true;
                                        mySetter(DisplayFields.Status, 'RUNNING(' + xTaskRunState + ')');
                                    }
                                } else {
                                    if (this.pxCurrentTCBs !== null) {
                                        threadRunning = threadId === this.curThreadInfo;
                                        mySetter(DisplayFields.Status, threadRunning ? 'RUNNING' : state);
                                    } else {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, 'UNKWON');
                                    }
                                }
                            }

finally, very thanks for yor PR, it`s very useful for me.

[haneefdm]

Will merge it soon. Sorry, but my absence was unavoidable. Questions

• We need to find out what is going on with RUNNING(undefined)
• Why do I have that many tasks that all say RUNNING? The number of tasks RUNNING should be max the number of cores.
• Curious. The runtime is somehow normalized (in FW). What if the two cores are running at different clock speeds? Is this not expected?
• I can't see the entire screenshot but does the runtime add up tp 100*num-cores

The rest looks good, but I haven't reviewed the code.

but @hwmland esp32 also have double core, and it all show RUNNING(undefined),maybe some err with other chip

I would not expect the chip has anything to do with it. It has to be FW or our code right?

[hwmland]

Hello @haneefdm , nice to have you back! I hope you don't take my post as blaming you - it was not ment so. I hope you are fine now! But back to the business :)
We have so many RUNNING(undefined) because (my conclussion based on looking into code and UNDEFINED part) there is a problem with const xTaskRunState = thInfo['xTaskRunState']?.val; It's suppose to show core ID in the place of UNDEFINED
Possible reasons:

• xTaskRunState is not in TCB. Well this is possible if configNUMBER_OF_CORES is 1 (but @USTHzhanglu confirms he has 2). HERE or in SMP branch of FreeRTOS HERE
• @USTHzhanglu uses some FW where it's not present in TCB (hence I asked for his FW)
• There is some prblem with const thInfo = await this.getExprValChildrenObj(... - but this is out of my expertise
• I'm blind and overlooked something else

I agree that it has nothing to do with chip (but don't know it there is some special/hacked version of FreeRTOS for this chip) -> FW

As to your concert about runtime on cores with different clock speed - up to my knowladge it's calculated based on ticks and those are the same (I hope) independent on frequency

[haneefdm]

As to your concert about runtime on cores with different clock speed - up to my knowladge it's calculated based on ticks and those are the same (I hope) independent on frequency

Yes, guess it is normalized to a tick. It is fine. It was more of a curiosity.

xTaskRunState is not in TCB. Well this is possible if configNUMBER_OF_CORES is 1 (but @USTHzhanglu confirms he has 2). HERE or in SMP branch of FreeRTOS HERE

Clearly the FW is different between the two examples. The sheer number of configXXX defines make it very difficult. I have numerous instances where users didn't even know what they were disabling/enabling but expect this extension to show info that does not exist.

Back to the FW, how can we figure out whose version is the truth? One of the links you provided matches what is the current release. (V11.1.0)

In the past, I had said that there were changes in FreeRTOS data structures and I was told that doesn't happen. I think it does happen. The last LTS release seems to be 10.4.3 on Sep 16, 2022. It appears that neither links to tasks.c are using officially released versions.

First

 * FreeRTOS Kernel <DEVELOPMENT BRANCH>

Second

 * FreeRTOS SMP Kernel V202110.00

LTS Release

 * FreeRTOS Kernel V10.4.3 LTS Patch 3

Latest Release

 * FreeRTOS Kernel V11.1.0

Granted that it just shows the comments that are mass edited. But what it tells me is that there are too many versions floating around and sometimes people are picking up whatever is current at the time.

I see the type of changes since last LTS and they have to do with SMP support, security, support, new architectures, MPU, etc. I don't think FreeRTOS people consider it to be LTS ready.

Why am I saying all of this? In the least, we should settle on one published minimum version and claim support for that and let people use it at their own risk, other/newer versions. And, report issues and volunteer to help.

[USTHzhanglu]

@hwmland @haneefdm i know why.
esp32 use ESP_IDF freeRTOS which merge many SMP code to freeRTOS :)
it use'd pxCurrentTCBs but non't use xTaskRunState. a magic rtos
and i fix it. maybe batter pr to ESP_IDF freeRTOS. :)

    private updateCurrentsThreadAddr(frameId: number): Promise<void> {
        return new Promise<void>((resolve, reject) => {
            if (this.pxCurrentTCBs !== null) {
                this.pxCurrentTCBs?.getValue(frameId).then(
                    (ret) => {
                        if (ret !== undefined) {
                            const match = ret.match(/\d+/);
                            this.pxCurrentTCBsNum = match ? parseInt(match[0]) : 0;
                        } else {
                            this.pxCurrentTCBsNum = 0;
                        }
                        my_log(`pxCurrentTCBsNum: ${this.pxCurrentTCBsNum}`);
                        for (let i = 0; i < this.pxCurrentTCBsNum; i++) {
                            this.getExprVal('pxCurrentTCBs[' + i + ']', frameId).then(
                                (ret) => {
                                    this.curThreadInfos[i] = parseInt(ret || '');
                                },
                                (e) => {
                                    reject(e);
                                }
                            );
                        }
                        resolve();
                    },
                    (e) => {
                        reject(e);
                    }
                );
            } else {
                resolve();
            }
        });
    }

                                const xTaskRunState = thInfo['xTaskRunState']?.val;
                                if (xTaskRunState !== undefined) {
                                    if (xTaskRunState === '-2') {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, 'YIELD');
                                    } else if (xTaskRunState === '-1') {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, state);
                                    } else {
                                        threadRunning = true;
                                        mySetter(DisplayFields.Status, 'RUNNING(' + xTaskRunState + ')');
                                    }
                                } else {
                                    if (this.pxCurrentTCBs !== null) {
                                        threadRunning = false;
                                        for (const num in this.curThreadInfos) {
                                            if (this.curThreadInfos[num] === threadId) {
                                                threadRunning = true;
                                                mySetter(DisplayFields.Status, 'RUNNING(' + num + ')');
                                                break;
                                            }
                                        }
                                        if (!threadRunning) {
                                            mySetter(DisplayFields.Status, state);
                                        }
                                    } else {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, 'UNKNOWN');
                                    }
                                }
                            }

at mainline freeRTOS, the pr is good.

[hwmland]

Ok, so we have answer what was going on with ESP32 (FreeRTOS hacked by ESP) I'll ignore this in following anylyzis and return to it later.
As to versions:
LTS release does not support SMP. It's covered in our code by check for existence of pxCurrentTCB (it exists => original processing)
First (developement) and LastRelease (v11.1.0) are the same from our code point of view:

• if (configNUMBER_OF_CORES == 1) => no SMP, we have pxCurrentTCB => original processing
• otherwise SMP, we DON'T have pxCurrentTCB, we have pxCurrentTCBs and xTaskRunState in the TCB => my changed code came into action
  SMP branch: (independent on number of cores) we DON'T have pxCurrentTCB, we have pxCurrentTCBs and xTaskRunState in the TCB => my changed code came into action

So I'd say that extension should be valid/working for LTS (no SMP), Release and develop versions as well as for SMP version. Of course both develop and SMP branches could change any time, but what could we do... We cannot forseen changes :)

Finally - to have ESP32 support, @USTHzhanglu changes should be merged (as his code would not work for other FreeRTOS) My problem is that I still did not recreate my dev environment after computer crash in December and I'm not able to test it. Based on my work I don't see possibility to find enough time for this before summer :(

[haneefdm]

@USTHzhanglu changes should be merged (as his code would not work for other FreeRTOS)

@hwmland Did you mean that? So, we only support ESP version? Or do I have it backwards?

In case there is a misunderstanding, we should be worried about the general user first. Then add other implementations if they are important. It is not okay to say, my needs are met and not worry about other users. Not saying you are thinking like that, but just a reminder.

[hwmland]

Sorry, I was not clear.
Our current official branch does not support any SMP as mentioned in #14
My PR (up to my knowledge) does support both SMP and non-SMP FreeRTOS as known now except ESP
Changes presented here by @USTHzhanglu support only ESP32. Other FreeRTOS versions will not work with his code (SMP perhas yes - not 100% sure, non-SMP will not work)

[USTHzhanglu]

now we have Three situations

1. freertos use pxCurrentTCB.
2. freertos SMP,use pxCurrentTCBs and xTaskRunState
3. other freertos SMP use pxCurrentTCBs but no xTaskRunState.

i think this pr just not included 3.
by other tools like freertos_gdb https://github.com/espressif/freertos-gdb/
just use pxCurrentTCB and pxCurrentTCBs to Identify running tasks
so i add some code like it,and it can work as expected
Maybe i can add a ESP_IDF freertos type, but it's almost the same as freertos , so i don't know how to distinguish them。
i think maybe we can push patch to this pr. how do yo think it? @hwmland
this my patch code.
add some data to class RTOSFreeRTOS

    private pxCurrentTCBsNum = 0;
    private curThreadInfo = 0; // address (from pxCurrentTCB) of status (when multicore)
    private curThreadInfos: number[] = []; //address (from pxCurrentTCBs) of status of all threads (when multicore)

add a new fuction

    private updateCurrentsThreadAddr(frameId: number): Promise<void> {}

use this fuction at

                            promises.push(this.updateCurrentThreadAddr(frameId));
                            promises.push(this.updateCurrentsThreadAddr(frameId));

finally if pxCurrentTCB is null, xTaskRunState is undefined,and pxCurrentTCBs not null,
We pass whether the thread is in pxCurrentTCBs ，To determine whether the current thread is running

                                        for (const num in this.curThreadInfos) {
                                            if (this.curThreadInfos[num] === threadId) {
                                                threadRunning = true;
                                                mySetter(DisplayFields.Status, 'RUNNING(' + num + ')');
                                                break;
                                            }
                                        }

and if all of them is null ,we Prompt unknown

 else {
                                        threadRunning = false;
                                        mySetter(DisplayFields.Status, 'UNKNOWN');
                                    }

[hwmland]

Hello, from my point of view no problem if you push changes to 'my' PR, our common goal is to make things better. Much more question to @haneefdm what he thinks - what is better/more safe.
I have just one plea:

promises.push(this.updateCurrentThreadAddr(frameId));
promises.push(this.updateCurrentsThreadAddr(frameId));

those 2 methods have too similar names for my taste, it's confusing, took me time till I realized they are different...