You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "pussuw (via GitHub)" <gi...@apache.org> on 2024/05/31 12:06:17 UTC
[PR] Riscv syscall optimize [nuttx]
pussuw opened a new pull request, #12435:
URL: https://github.com/apache/nuttx/pull/12435
## Summary
This set of patches change how user service calls are executed:
Instead of using the common interrupt logic, execute the user service
call directly.
Why? When a user makes a service call request, all of the service call
parameters are already loaded into the correct registers, thus it makes
no sense to first clobber them and then reload them, which is what the
old logic does. It is much more effective to run the system call directly.
During a user system call the interrupts must be re-enabled, which the
new logic does as soon as we know the exception is a user service call
request.
This patch set does NOT change the behavior of reserved system calls (like
switch_context), only the user service call request is affected.
## Impact
User system call logic basically rewritten to improve performance.
Performance was measured with custom app that counts cycles of getpid()
system call over 1000 iterations, recording the min/max/avg values.
Performance impact is ~400 cycles (~40% less overhead) saved per system call:
nsh> hello -v
avg:2111, min:950, max:527713, rounds:1000
nsh> hello -v
avg:1580, min:586, max:392567, rounds:1000
## Testing
rv-virt:knsh64 ostest
rv-virt:ksmp64 ostest
MPFS target with ~100 user & kernel threads
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #12435:
URL: https://github.com/apache/nuttx/pull/12435
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2260419098
@pussuw so consider opening an issue to report the problem when CONFIG_ARCH_KERNEL_STACK=y is enabled
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2260487008
> > @pussuw so consider opening an issue to report the problem when CONFIG_ARCH_KERNEL_STACK=y is enabled
>
> The problem is almost fixed. If you want I can do the issue.
Oh sorry, I think you don't need to. I thought it was something to be fixed in the future!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2258455423
vfork() is just completely broken via system calls, no idea how it worked, even before #12435
Like you noticed, the parent's stack is loaded when the child returns to user space. This was broken prior to 12435 as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2245510745
> @pussuw could you provide a patch to improve the fix.
Yes I can, however I don't have time to do it right now. I'll try to come back to this asap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2259514435
> vfork() is just completely broken via system calls, no idea how it worked, even before #12435
as per my notes in issue #12635, `vfork_test()` passes for both FLAT and PROTECTED build if commit e6973c764cd531 in pull #12435 is reverted.
So before pull#12717, `vfork()` was too sensitive to parent/child task scheduling order after fork, if parent reaches `waitpid` earlier, then pollution doesn't hurt at all. The syscall optimization affects the task schedule ordering thus pollution happened before parent reached `waitpid`.
Later in pull#12717, the SP is restored relatively, thus pollution reduced a little. Even when child runs before parent reaches `waitpid`, it `vfork_test` can still pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2143368552
> Tested OK on Ox64 BL808 SBC. Thanks!
>
> Test Log: https://gist.github.com/lupyuen/26ecb09dfdb3dfef456e61b237e6668b
Thanks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "lupyuen (via GitHub)" <gi...@apache.org>.
lupyuen commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2142678779
@pussuw I'll test on Ox64 BL808 SBC and post my results. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2259870016
Yes so it worked by chance then. Looking at the vfork() implementation it relies on the stack unwind to restore the child's state, so your fix works fine. CONFIG_ARCH_KERNEL_STACK=y does not work still.
I'll make the child return directly to userspace directly instead of unwinding the copied parent kernel stack. That should work fine and will skip unnecessary work on the child.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2247209614
@pussuw thanks for the comments, I will be looking forward to your fix later and help to check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2260439807
> @pussuw so consider opening an issue to report the problem when CONFIG_ARCH_KERNEL_STACK=y is enabled
The problem is almost fixed. If you want I can do the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2245467192
@pussuw could you provide a patch to improve the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2244841345
> @pussuw does this support PROTECTED build where `ARCH_KERNEL_STACK` nor `RISCV_PERCPU_SCRATCH` is defined?
I forgot about fork altogether, as it does not work in kernel mode. Sorry about the regression in protected mode.
Initially, I think enabling system calls should enforce RISCV_PERCPU_SCRATCH. Enforcing ARCH_KERNEL_STACK is not necessary though, it should work without.
Why is the scratch area needed ? Because TP has dual meaning depending on which privilege mode we are running;
When in usermode: TP holds the user thread pointer (__thread / TLS) (When CONFIG_SCHED_THREAD_LOCAL=y)
When in kernelmode: TP holds the current TCB
Thus, We need the scratch area to atomically swap between user TLS / kernel TCB upon trap entry!
The fix in 12717 works if CONFIG_SCHED_THREAD_LOCAL=n but (I think it) destroys the user __thread / TLS pointer otherwise.
Thanks for doing this patch, however in order to restore CONFIG_SCHED_THREAD_LOCAL=y functionality we need to do more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] riscv/syscall: Optimize user service call performance [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on PR #12435:
URL: https://github.com/apache/nuttx/pull/12435#issuecomment-2259512732
> vfork() is just completely broken via system calls, no idea how it worked, even before #12435
as per my notes in issue #12635, `vfork_test()` passes for both FLAT and PROTECTED build if commit e6973c764cd531 in pull #12435 is reverted.
So before pull#12717, `vfork()` was too sensitive to parent/child task scheduling order after fork, if parent reaches `waitpid` earlier, then pollution doesn't hurt at all. The syscall optimization affects the task schedule ordering thus pollution happened before parent reached `waitpid`.
Later in pull#12717, the SP is restored relatively, thus pollution reduced a little even when child runs before parent reaches `waitpid`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org