mcu-debug / rtos-views

RTOS views for microcontrollers
MIT License
24 stars 11 forks source link

Add support for SMP on FreeRTOS #44

Open hwmland opened 5 months ago

hwmland commented 5 months ago

This PR resolves #14 and resolves #43

hwmland commented 5 months ago

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 commented 5 months ago

@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 https://github.com/mcu-debug/rtos-views/issues/14#issuecomment-1439201525. 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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 https://github.com/mcu-debug/rtos-views/issues/14#issuecomment-1439201525.

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 commented 5 months ago

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.

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 commented 5 months ago

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:

PhilippHaefele commented 5 months ago

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 commented 5 months ago

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 :

Screenshot 2024-01-09 at 14 17 27

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 commented 5 months ago

Example project with simple build instructions are here

hwmland commented 5 months ago

Multicore setup example result: image

Single-core example result: image