riscv: use g_running_task store current regs #13585

hujun260 · 2024-09-24T04:18:25Z

Summary

This commit fixes the regression from #13561

In order to determine whether a context switch has occurred, we can use g_running_task to store the current regs. This allows us to compare the current register state with the previously stored state to identify if a context switch has taken place.

Impact

riscv

Testing

rv-virt:knsh
rv-virt:knsh64
rv-virt:smp

nuttxpr · 2024-09-24T04:23:53Z

[Experimental Bot, please feedback here]

The PR summary is missing some information, and the Testing section is incomplete.

Here's a breakdown:

Summary - Needs Improvement

What functional part of the code is being changed? Be specific. Mention the files, functions, or modules affected.
How does the change exactly work (what will change and how)? Provide a technical explanation of your solution. How does using g_running_task to store registers help determine a context switch?

Impact - Needs Improvement

Is new feature added? Is existing feature changed? - This seems like a bug fix, so clearly state that.
Impact on user (will user need to adapt to change)? - Even if the answer is NO, you should still briefly explain why.
The other "Impact" sections may not all be relevant. It's okay to remove sections if there's genuinely no impact.

Testing - Incomplete

Build Host(s): Specify your build host details.
Testing logs before change / Testing logs after change: You need to provide actual logs demonstrating the problem before your fix and the improvement afterward.

To improve this PR:

Expand the Summary: Clearly describe what part of the code is affected and the technical details of your fix.
Complete the Impact section: Address all points, even if it's to say "NO" and provide a brief justification.
Provide Actual Testing Logs: Show the issue before your change and the successful result after.

By providing this additional information, you'll make it easier for reviewers to understand and approve your PR.

lupyuen

Tested OK with OSTest on rv-virt:knsh, rv-virt:knsh64 and milkv_duos:nsh. Thanks!

lupyuen · 2024-09-24T04:32:47Z

Hi @tmedicci would you need to test this on ESP32 RISC-V? Thanks!

lupyuen · 2024-09-24T05:00:18Z

@hujun260 I rebooted and retested OSTest, 10 times on rv-virt:knsh64. Out of the 10 tests, 1 test failed at vfork. Would you know what's causing this? Thanks! https://gist.github.com/lupyuen/cebee9e69f23047476a7a5305f595fa6

user_main: vfork() test
[   71.520000] _assert: Current Version: NuttX  0.0.0 7b5ca0ec41 Sep 24 2024 12:41:49 risc-v
[   71.520000] _assert: Assertion failed : at file: common/riscv_fork.c:133 task: ostest process: ostest 0xc000001a
[   71.520000] up_dump_register: EPC: 00000000802133e4
[   71.520000] up_dump_register: A0: 00000000804072b0 A1: 0000000000000085 A2: 0000000200042020 A3: 0000000080408560
[   71.520000] up_dump_register: A4: 000000000000000a A5: 0000000000000004 A6: 0000000000000000 A7: 0000000000000000
[   71.520000] up_dump_register: T0: 0000000080206898 T1: 0000000000000007 T2: 0000000000000000 T3: 00000000c0206070
[   71.520000] up_dump_register: T4: 00000000c0206068 T5: 00000000c000b8f9 T6: 000000000000003d
[   71.520000] up_dump_register: S0: 0000000000000000 S1: 0000000080409180 S2: 0000000080409180 S3: 0000000000000000
[   71.520000] up_dump_register: S4: 0000000200042020 S5: 0000000000000000 S6: 0000000080219328 S7: 0000000000000000
[   71.520000] up_dump_register: S8: 0000000000000006 S9: 00000000804074d0 S10: 0000000000000085 S11: 0000000080407550
[   71.520000] up_dump_register: SP: 000000008040c850 FP: 0000000000000000 TP: 0000000080409180 RA: 00000000802133e4
[   71.520000] dump_stack: Kernel Stack:
[   71.520000] dump_stack:   base: 0x8040c030
[   71.520000] dump_stack:   size: 00003072
[   71.520000] dump_stack:     sp: 0x8040c850
[   71.520000] stack_dump: 0x8040c810: 0000000000000009 00000000804092d8 000000008021be18 0000000000000000 00000000c0202070 0000000080409180 0000000000000000 0000000080213666
[   71.520000] stack_dump: 0x8040c850: 00000000c000001a fffffffffffffffc 0000000000006010 00000000c0200000 0000000080409180 00000000804072b0 0000000000000000 0000000080219328
[   71.520000] stack_dump: 0x8040c890: 0000000000000085 ffff00587474754e 0000000000000c10 0000000080408180 0000000080408180 0000000080206af0 00000000c0206000 302e300080408180
[   71.520000] stack_dump: 0x8040c8d0: 000000008000302e 00000000802086ca 3061633562370000 7065532031346365 3432303220343220 343a31343a323120 0000000080400039 0000000080203352
[   71.520000] stack_dump: 0x8040c910: 7369720000000000 0000000000762d63 0000000000000000 000000008040cc40 000000008040cf40 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040c950: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000005 0000000200042022 0000000080409180 00000000c000919a
[   71.520000] stack_dump: 0x8040c990: ffffffffffffffda 0000000080206912 ffffffffffffffda 000000008020a32c ffffffffffffffff 0000000000000000 0000000000000005 0000000000000020
[   71.520000] stack_dump: 0x8040c9d0: 0000000000000008 00000000c000919a ffffffffffffffda 00000000802068a0 0000000000000008 0000000080200940 0000000000000000 0000000080200972
[   71.520000] stack_dump: 0x8040ca10: 0000000200042020 00000000802001a2 00000000c000919a 00000000c0008af8 00000000c0203ef0 0000000080409180 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040ca50: 00000000c01017b0 0000000000000000 00000000c0202040 0000000000000000 0000000000000022 00000000c02003d8 00000000c0200448 0000000000000018
[   71.520000] stack_dump: 0x8040ca90: 000000000000000a 00000000c0101f4e 0000000000000009 0000000000000000 0000000000000005 00000000c0010a48 0000000000000005 0000000000000000
[   71.520000] stack_dump: 0x8040cad0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 00000000c0008146 00000000c000b6f8
[   71.520000] stack_dump: 0x8040cb10: 00000000c000b8f9 000000000000003d 0000000200042020 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cb50: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cb90: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cbd0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 00000000802034b2
[   71.520000] stack_dump: 0x8040cc10: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
ostest_main: Exiting with status 256

Update: Out of 20 reboots and retests, 2 failed with the vfork error. So the failure rate is 10%.

hujun260 · 2024-09-24T05:07:49Z

@hujun260 I rebooted and retested OSTest, 10 times on rv-virt:knsh64. Out of the 10 tests, 1 test failed at vfork. Would you know what's causing this? Thanks! https://gist.github.com/lupyuen/cebee9e69f23047476a7a5305f595fa6

user_main: vfork() test
[   71.520000] _assert: Current Version: NuttX  0.0.0 7b5ca0ec41 Sep 24 2024 12:41:49 risc-v
[   71.520000] _assert: Assertion failed : at file: common/riscv_fork.c:133 task: ostest process: ostest 0xc000001a
[   71.520000] up_dump_register: EPC: 00000000802133e4
[   71.520000] up_dump_register: A0: 00000000804072b0 A1: 0000000000000085 A2: 0000000200042020 A3: 0000000080408560
[   71.520000] up_dump_register: A4: 000000000000000a A5: 0000000000000004 A6: 0000000000000000 A7: 0000000000000000
[   71.520000] up_dump_register: T0: 0000000080206898 T1: 0000000000000007 T2: 0000000000000000 T3: 00000000c0206070
[   71.520000] up_dump_register: T4: 00000000c0206068 T5: 00000000c000b8f9 T6: 000000000000003d
[   71.520000] up_dump_register: S0: 0000000000000000 S1: 0000000080409180 S2: 0000000080409180 S3: 0000000000000000
[   71.520000] up_dump_register: S4: 0000000200042020 S5: 0000000000000000 S6: 0000000080219328 S7: 0000000000000000
[   71.520000] up_dump_register: S8: 0000000000000006 S9: 00000000804074d0 S10: 0000000000000085 S11: 0000000080407550
[   71.520000] up_dump_register: SP: 000000008040c850 FP: 0000000000000000 TP: 0000000080409180 RA: 00000000802133e4
[   71.520000] dump_stack: Kernel Stack:
[   71.520000] dump_stack:   base: 0x8040c030
[   71.520000] dump_stack:   size: 00003072
[   71.520000] dump_stack:     sp: 0x8040c850
[   71.520000] stack_dump: 0x8040c810: 0000000000000009 00000000804092d8 000000008021be18 0000000000000000 00000000c0202070 0000000080409180 0000000000000000 0000000080213666
[   71.520000] stack_dump: 0x8040c850: 00000000c000001a fffffffffffffffc 0000000000006010 00000000c0200000 0000000080409180 00000000804072b0 0000000000000000 0000000080219328
[   71.520000] stack_dump: 0x8040c890: 0000000000000085 ffff00587474754e 0000000000000c10 0000000080408180 0000000080408180 0000000080206af0 00000000c0206000 302e300080408180
[   71.520000] stack_dump: 0x8040c8d0: 000000008000302e 00000000802086ca 3061633562370000 7065532031346365 3432303220343220 343a31343a323120 0000000080400039 0000000080203352
[   71.520000] stack_dump: 0x8040c910: 7369720000000000 0000000000762d63 0000000000000000 000000008040cc40 000000008040cf40 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040c950: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000005 0000000200042022 0000000080409180 00000000c000919a
[   71.520000] stack_dump: 0x8040c990: ffffffffffffffda 0000000080206912 ffffffffffffffda 000000008020a32c ffffffffffffffff 0000000000000000 0000000000000005 0000000000000020
[   71.520000] stack_dump: 0x8040c9d0: 0000000000000008 00000000c000919a ffffffffffffffda 00000000802068a0 0000000000000008 0000000080200940 0000000000000000 0000000080200972
[   71.520000] stack_dump: 0x8040ca10: 0000000200042020 00000000802001a2 00000000c000919a 00000000c0008af8 00000000c0203ef0 0000000080409180 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040ca50: 00000000c01017b0 0000000000000000 00000000c0202040 0000000000000000 0000000000000022 00000000c02003d8 00000000c0200448 0000000000000018
[   71.520000] stack_dump: 0x8040ca90: 000000000000000a 00000000c0101f4e 0000000000000009 0000000000000000 0000000000000005 00000000c0010a48 0000000000000005 0000000000000000
[   71.520000] stack_dump: 0x8040cad0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 00000000c0008146 00000000c000b6f8
[   71.520000] stack_dump: 0x8040cb10: 00000000c000b8f9 000000000000003d 0000000200042020 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cb50: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cb90: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[   71.520000] stack_dump: 0x8040cbd0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 00000000802034b2
[   71.520000] stack_dump: 0x8040cc10: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000
ostest_main: Exiting with status 256

ok i'll try to reproduce

lupyuen · 2024-09-24T09:55:31Z

@hujun260 I did some more Stress Testing:

For the code in this PR: Out of 20 reboots and retests, 2 failed with the vfork error. So the failure rate is 10%.
For the code dated 12 Sep, 2 weeks ago (9bee10f): Out of 200 reboots and retests, all 200 succeeded without vfork error.

So this vfork problem might be introduced in the past 2 weeks?

lupyuen

Let's do more analysis on the vfork issue, to ensure it's not caused by this PR. Thanks!

hujun260 · 2024-09-24T13:30:02Z

Let's do more analysis on the vfork issue, to ensure it's not caused by this PR. Thanks!
ok

hujun260 · 2024-09-24T13:51:03Z

@lupyuen Could you send me your stress testing script? I can occasionally reproduce the issue on my side, but the efficiency is relatively low.

lupyuen · 2024-09-24T14:00:47Z

@hujun260 Here's my Stress Test Script that will run 20 times: https://github.com/lupyuen/nuttx-riscv64/blob/main/special-qemu-riscv-knsh64.sh

Please change https://github.com/apache/nuttx/tree/master to https://github.com/hujun260/nuttx/tree/apache_11

arch/risc-v/src/common/riscv_doirq.c

arch/risc-v/src/common/supervisor/riscv_perform_syscall.c

arch/risc-v/src/common/riscv_swint.c

lupyuen

Tested OK with OSTest on rv-virt:knsh, rv-virt:knsh64 and milkv_duos:nsh.

Stressed Tested 200 times (reboot + ostest) on rv-virt:knsh64. All 200 times succeeded 🎉 Thanks :-)

arch/risc-v/src/common/riscv_fork.c

lupyuen

Thanks @hujun260! Let's wait for #13633 to be merged. Then we can rebase and retest your PR. Thank you so much :-)

lupyuen · 2024-09-27T02:43:09Z

Hi @hujun260: #13633 has been merged. Could you please rebase to the master branch and retest your PR? Thanks!

hujun260 · 2024-09-27T03:35:59Z

Thanks @hujun260! Let's wait for #13633 to be merged. Then we can rebase and retest your PR. Thank you so much :-)

ok

lupyuen

Stress Test of rv-virt:knsh64 (reboot qemu + restart ostest) succeeded for all 200 iterations. Also tested OK with OSTest on milkv_duos:nsh. Thank you so much!

arch/arm/src/armv8-r/Toolchain.defs

lupyuen

Retested: Stress Test of rv-virt:knsh64 (reboot qemu + restart ostest) succeeded for all 200 iterations. Also tested OK with OSTest on milkv_duos:nsh. Thank you so much!

This commit fixes the regression from apache#13561 In order to determine whether a context switch has occurred, we can use g_running_task to store the current regs. This allows us to compare the current register state with the previously stored state to identify if a context switch has taken place. Signed-off-by: hujun5 <[email protected]>

lupyuen · 2024-09-28T02:53:35Z

FYI All the Daily Tests are successful now 🎉

github-actions bot added Area: Tooling Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: L The size of the change in this PR is large labels Sep 24, 2024

hujun260 force-pushed the apache_11 branch from 14bde80 to 7b5ca0e Compare September 24, 2024 04:19

github-actions bot added Size: S The size of the change in this PR is small and removed Area: Tooling Size: L The size of the change in this PR is large labels Sep 24, 2024

hujun260 mentioned this pull request Sep 24, 2024

riscv: add a return value to riscv_swint #13564

Merged

lupyuen approved these changes Sep 24, 2024

View reviewed changes

lupyuen requested changes Sep 24, 2024

View reviewed changes

xiaoxiang781216 reviewed Sep 24, 2024

View reviewed changes

arch/risc-v/src/common/riscv_doirq.c Outdated Show resolved Hide resolved

arch/risc-v/src/common/supervisor/riscv_perform_syscall.c Outdated Show resolved Hide resolved

arch/risc-v/src/common/riscv_swint.c Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_11 branch from 7b5ca0e to f1564b6 Compare September 25, 2024 11:21

github-actions bot added Size: M The size of the change in this PR is medium and removed Size: S The size of the change in this PR is small labels Sep 25, 2024

lupyuen approved these changes Sep 25, 2024

View reviewed changes

lupyuen mentioned this pull request Sep 25, 2024

Optimize the code flow of dynamic loading #13579

Merged

pussuw requested changes Sep 25, 2024

View reviewed changes

arch/risc-v/src/common/riscv_fork.c Outdated Show resolved Hide resolved

lupyuen mentioned this pull request Sep 26, 2024

riscv_fork.c: Fix vfork() for kernel mode + SMP #13633

Merged

pussuw reviewed Sep 26, 2024

View reviewed changes

arch/risc-v/src/common/riscv_fork.c Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_11 branch from f1564b6 to 391d338 Compare September 26, 2024 11:15

github-actions bot added Size: S The size of the change in this PR is small and removed Size: M The size of the change in this PR is medium labels Sep 26, 2024

lupyuen requested changes Sep 26, 2024

View reviewed changes

hujun260 force-pushed the apache_11 branch from 391d338 to 7c49b3d Compare September 27, 2024 03:35

github-actions bot added the Arch: arm Issues related to ARM (32-bit) architecture label Sep 27, 2024

lupyuen approved these changes Sep 27, 2024

View reviewed changes

xiaoxiang781216 approved these changes Sep 27, 2024

View reviewed changes

pussuw reviewed Sep 27, 2024

View reviewed changes

arch/arm/src/armv8-r/Toolchain.defs Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_11 branch from 7c49b3d to 444e7ec Compare September 27, 2024 06:42

github-actions bot removed the Arch: arm Issues related to ARM (32-bit) architecture label Sep 27, 2024

lupyuen approved these changes Sep 27, 2024

View reviewed changes

hujun260 force-pushed the apache_11 branch from 444e7ec to f84407f Compare September 27, 2024 07:48

xiaoxiang781216 approved these changes Sep 27, 2024

View reviewed changes

xiaoxiang781216 merged commit 3e459c0 into apache:master Sep 27, 2024
29 checks passed

hujun260 deleted the apache_11 branch September 29, 2024 03:27

lupyuen mentioned this pull request Oct 2, 2024

Documentation: add NuttX-12.7.0 release notes #13785

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

riscv: use g_running_task store current regs #13585

riscv: use g_running_task store current regs #13585

hujun260 commented Sep 24, 2024

nuttxpr commented Sep 24, 2024

lupyuen left a comment

lupyuen commented Sep 24, 2024

lupyuen commented Sep 24, 2024 •

edited

Loading

hujun260 commented Sep 24, 2024

lupyuen commented Sep 24, 2024

lupyuen left a comment

hujun260 commented Sep 24, 2024

hujun260 commented Sep 24, 2024 •

edited

Loading

lupyuen commented Sep 24, 2024

lupyuen left a comment •

edited

Loading

lupyuen left a comment

lupyuen commented Sep 27, 2024

hujun260 commented Sep 27, 2024

lupyuen left a comment

lupyuen left a comment

lupyuen commented Sep 28, 2024

riscv: use g_running_task store current regs #13585

riscv: use g_running_task store current regs #13585

Conversation

hujun260 commented Sep 24, 2024

Summary

Impact

Testing

nuttxpr commented Sep 24, 2024

lupyuen left a comment

Choose a reason for hiding this comment

lupyuen commented Sep 24, 2024

lupyuen commented Sep 24, 2024 • edited Loading

hujun260 commented Sep 24, 2024

lupyuen commented Sep 24, 2024

lupyuen left a comment

Choose a reason for hiding this comment

hujun260 commented Sep 24, 2024

hujun260 commented Sep 24, 2024 • edited Loading

lupyuen commented Sep 24, 2024

lupyuen left a comment • edited Loading

Choose a reason for hiding this comment

lupyuen left a comment

Choose a reason for hiding this comment

lupyuen commented Sep 27, 2024

hujun260 commented Sep 27, 2024

lupyuen left a comment

Choose a reason for hiding this comment

lupyuen left a comment

Choose a reason for hiding this comment

lupyuen commented Sep 28, 2024

lupyuen commented Sep 24, 2024 •

edited

Loading

hujun260 commented Sep 24, 2024 •

edited

Loading

lupyuen left a comment •

edited

Loading