arch/arm: optimize context switch speed #5645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anchao
force-pushed
the
22022802
branch
2 times, most recently
from
1e5dce4
to
8bc40ce
Compare
|
I tried this PR with my latest imx6-netknsh branches (https://github.com/masayuki2009/incubator-nuttx/tree/imx6-netknsh https://github.com/masayuki2009/incubator-nuttx-apps/tree/imx6-netknsh) and found that init and hello work but getprime fails when it exits. |
@masayuki2009 san, I can reproduce this issue on your branch but without this PR, seems some wrong with syscall clock_gettime: |
@anchao |
Yes, could you try your latest branch? it seems does not work as expected |
Are you using the latest my apps branch as well? It seems that you are using the old version (i.e. non thumb build version). |
Yes, apps branch is latest, I reinitialized another repository, but crash still happen. |
Hmm, it's very strange to me. |
Please check the attachment |
I confirmed that the binaries are slightly different from mine. |
|
|
I notice that my toolchain is a little bit old. So let me confirm the latest toolchain. |
|
I tried the latest toolchain Now I can reproduce the crash which you are facing now. |
That's great! thanks for your time, I will do more test on kernel mode. |
anchao
force-pushed
the
22022802
branch
3 times, most recently
from
09a9999
to
2659327
Compare
anchao
force-pushed
the
22022802
branch
2 times, most recently
from
4fd5645
to
9405df4
Compare
xiaoxiang781216
approved these changes
Mar 4, 2022
|
@masayuki2009 could you try the update? |
|
I noticed that lm3s6965-ek:qemu-kostest (QEMU) failed with this PR. |
|
The original issue with sabre-6quad:netknsh (QEMU) still happens. The text region of the getprime (elf) starts at 0x8000:0000, Here, r3 is loaded from |
@masayuki2009 san, There is a issue with the consistency of banked registers in SVC and USR mode. I submitted an separate PR, Please review. |
Great idea, I'll add the corresponding performance tests later |
anchao
force-pushed
the
22022802
branch
2 times, most recently
from
754e332
to
faa86d3
Compare
xiaoxiang781216
approved these changes
Mar 14, 2022
pkarashchenko
approved these changes
Mar 14, 2022
|
@masayuki2009 please merge this PR as soon as you confirm that everything is ok on your side. |
|
Did you fix the issue with lm3s6965-ek:qemu-kostest (QEMU) ? |
No, I am still investigating this issue, update will be later if I have any progress. |
no1wudi
reviewed
Mar 15, 2022
no1wudi
reviewed
Mar 15, 2022
The current context save implementation saves registers of each task to xcp context, which is unnecessary because most of the arm registers are already saved in the task stack, this commit replace the xcp context with stack context to improve context switching performance and reduce the tcb space occupation of tcb instance. Signed-off-by: chao.an <anchao@xiaomi.com>
Signed-off-by: chao.an <anchao@xiaomi.com>
@masayuki2009 san, In the previous implementation, I did not consider the signal delivery in the interrupt context without interrupt stack, Therefore, the solution is that if the interrupt stack is disabled, reserve an signal xcpcontext to ensure signal processing can have a separate xcpcontext to handle signal context. dffd8f6#diff-31f02252f0714ae3f268b632eebec6f1dff7369c6caaa8625fcf597612ecd694R203-R230 |
Thanks for your investigation. |
@masayuki2009 san, In the latest commit set, I also added the signal handling if CONFIG_ARCH_INTERRUPTSTACK=0, The following 3 configs works fine with this PR: |
Thanks for your comments. |
|
I noticed that applications such as NTP daemon now consume more stack with this PR. The following results are from spresense:wifi_smp. Without this PR. With this PR. Please note that spresense:wifi_smp assigns the interrupt stack as follows. |
No, this is not expected behavior, SP is set to the incorrect during up_initial_state(): https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/armv7-a/arm_initialstate.c#L96 I will raise the fix PR after finish the test case. |
@masayuki2009 san, please review this PR: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
arch/arm: optimize context switch speed
The current context save implementation saves registers of each task
to xcp context, which is unnecessary because most of the arm registers are
already saved in the task stack, this commit replace the xcp context with
stack context to improve context switching performance and reduce the tcb
space occupation of tcb instance.
Signed-off-by: chao.an anchao@xiaomi.com
Impact
arm context switch
Testing
Qemu:
sabre-6quad/smp, sabre-6quad/netknsh
Cortex-m33: cycle count test
#define DWT_CYCCNT (DWT_BASE + 0x0004) /* Cycle Count Register */thread 1: (priority 100)
thread 2: (priority 200)
Before:
semaphore wait test: 797After:
semaphore wait test: 604+ ~24%