Skip to content

Conversation

@pussuw
Copy link
Contributor

@pussuw pussuw commented Aug 1, 2024

Summary

Two things need to be done when vfork'ing:

  • Must attach to parent's address environment (the addrenv is shared)
  • Must allocate a kernel stack (where would the register context go otherwise)

Note that this code assumes the address environment is shared, since we don't support fork() which would clone the address environment instead.

Impact

Fix vfork() for BUILD_KERNEL

Testing

rv-virt:knsh64 + ostest pass

Two things need to be done when vfork'ing:
- Must attach to parent's address environment (the addrenv is shared)
- Must allocate a kernel stack (where would the register context go otherwise)

Note that this code assumes the address environment is shared, since we
don't support fork() which would _clone_ the address environment instead.
@acassis acassis merged commit 6047a9f into apache:master Aug 1, 2024
@pussuw pussuw deleted the vfork_kernelmode_fix branch August 1, 2024 15:57
@lupyuen
Copy link
Member

lupyuen commented Aug 2, 2024

Sorry @pussuw I think we might have a problem:

(1) knsh32 (on macOS) hangs at OSTest vfork: knsh32 Log / Build Script

$ git reset --hard 6047a9fe146ad4b4e8b2dc7bffe92d9075db429f
HEAD is now at 6047a9fe14 task_fork.c: Fix vfork for BUILD_KERNEL
...
NuttShell (NSH) NuttX-12.6.0-RC1
nsh> uname -a
NuttX 12.6.0-RC1 6047a9fe14 Aug  2 2024 09:41:15 risc-v rv-virt

nsh> ostest
user_main: vfork() test
[Hangs here]

(2) knsh64 (on macOS) loops forever at OSTest vfork with an Assertion Failure: knsh64 Log / Build Script

user_main: vfork() test
_assert: Current Version: NuttX  12.6.0-RC1 6047a9fe14 Aug  2 2024 09:37:38 risc-v
_assert: Assertion failed : at file:
_assert: Current Version: NuttX  12.6.0-RC1 6047a9fe14 Aug  2 2024 09:37:38 risc-v

Any idea why this happens? Thanks!

Update: I see that the vfork test has been recently enabled: apache/nuttx-apps#2457

(FYI: I discovered this through the Daily Regression Test for knsh32 and Milk-V Duo S)

@pussuw
Copy link
Contributor Author

pussuw commented Aug 2, 2024

@lupyuen maybe there is a dependency to #12812

Hopefully I'll get it merged today. Sorry for the regression and the inconvenience.

@lupyuen
Copy link
Member

lupyuen commented Aug 2, 2024

@pussuw Cool no worries thanks! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants