You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2024/11/06 06:20:15 UTC
[PR] smpcall: we directly call the function to handle local smpcall [nuttx]
hujun260 opened a new pull request, #14663:
URL: https://github.com/apache/nuttx/pull/14663
## Summary
smpcall: we directly call the function to handle local smpcall
some arch do not support issuing interrupts to the local CPU.
This commit fixes the regression from https://github.com/apache/nuttx/pull/14656
## Impact
smpcall
## Testing
ci ostest
**Build Host:**
* OS: Ubuntu 20.04
* CPU: x86_64
* Compiler: GCC 9.4.0
Configuring NuttX and compile:
$ ./tools/configure.sh -l qemu-armv8a:nsh_smp
$ make
Running with qemu
$ qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic \
-machine virt,virtualization=on,gic-version=3 \
-net none -chardev stdio,id=con,mux=on -serial chardev:con \
-mon chardev=con,mode=readline -kernel ./nuttx
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #14663:
URL: https://github.com/apache/nuttx/pull/14663#discussion_r1830721459
##########
sched/sched/sched_smp.c:
##########
@@ -314,6 +318,17 @@ int nxsched_smp_call_async(cpu_set_t cpuset,
sched_lock();
}
+ if (CPU_ISSET(this_cpu(), &cpuset))
+ {
+ ret = data->func(data->arg);
+ if (ret < 0)
+ {
+ goto out;
+ }
+
+ CPU_CLR(this_cpu(), &cpuset);
Review Comment:
let's call nxsem_post here too to remove the special process in nxsched_smp_call
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #14663:
URL: https://github.com/apache/nuttx/pull/14663#discussion_r1830779115
##########
sched/sched/sched_smp.c:
##########
@@ -314,6 +318,17 @@ int nxsched_smp_call_async(cpu_set_t cpuset,
sched_lock();
}
+ if (CPU_ISSET(this_cpu(), &cpuset))
+ {
+ ret = data->func(data->arg);
+ if (ret < 0)
+ {
+ goto out;
+ }
+
+ CPU_CLR(this_cpu(), &cpuset);
Review Comment:
ok
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #14663:
URL: https://github.com/apache/nuttx/pull/14663#issuecomment-2459786190
@masayuki2009 could you verify this patch whether fix your problem?
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #14663:
URL: https://github.com/apache/nuttx/pull/14663#discussion_r1830999522
##########
sched/sched/sched_smp.c:
##########
@@ -314,6 +314,22 @@ int nxsched_smp_call_async(cpu_set_t cpuset,
sched_lock();
}
+ if (CPU_ISSET(this_cpu(), &cpuset))
+ {
+ ret = data->func(data->arg);
+ if (data->cookie != NULL)
+ {
+ nxsem_post(&data->cookie->sem);
+ if (ret < 0)
+ {
+ data->cookie->error = ret;
Review Comment:
move before line 322 to avoid the race condition
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14663:
URL: https://github.com/apache/nuttx/pull/14663#issuecomment-2458814458
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
This PR appears to mostly meet the NuttX requirements, but is missing some key information. Here's a breakdown:
**Strengths:**
* **Clear Summary:** The summary explains the "why," "what," and "how" of the change, and links a related PR. This is good.
* **Testing Information:** Includes build host details and target architecture. The configuration and build commands are helpful. Including the `qemu` command line is excellent.
**Weaknesses:**
* **Missing Issue Reference:** While a related PR is linked, if this fixes a reported issue, that issue should be linked as well.
* **Incomplete Impact Assessment:** The impact section is extremely brief. While "smpcall" is mentioned, it needs more detail. Explicitly state YES/NO for each impact category and provide descriptions where necessary. For example:
* **Is new feature added? Is existing feature changed?** Existing feature changed (smpcall functionality)
* **Impact on user:** NO (unless they rely on the broken behavior)
* **Impact on build:** NO
* **Impact on hardware:** YES (affects architectures that don't support self-interrupts) Specify *which* architectures.
* **Impact on documentation:** NO/YES - Justify.
* **Impact on security:** NO/YES - Justify.
* **Impact on compatibility:** Potentially YES. Since this reverts a regression, describe compatibility regained.
* **Insufficient Testing Logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. These are crucial to demonstrate the fix. Show specific commands and their output that demonstrate the problem before the change and the correct behavior after. For example, demonstrate that an SMP-related command now works that previously failed.
**Recommendation:**
To fully meet the NuttX requirements, the PR needs to:
1. **Link any related NuttX issues.**
2. **Expand the Impact section significantly.** Provide YES/NO answers for each item and justifications. Be specific about affected architectures.
3. **Populate the Testing Logs sections with concrete examples.** Show commands and output demonstrating the bug and the fix. Ideally, the logs should demonstrate the regression introduced by the linked PR and how this PR fixes it.
By addressing these points, the PR will be much stronger and easier for reviewers to evaluate.
--
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] smpcall: we directly call the function to handle local smpcall [nuttx]
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 merged PR #14663:
URL: https://github.com/apache/nuttx/pull/14663
--
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