You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/01/30 04:38:56 UTC
[GitHub] [incubator-nuttx-apps] masayuki2009 opened a new pull request #37:
examples: hello: Show CPU index when running in SMP mode
masayuki2009 opened a new pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37
### Summary
- The hello app now shows cpu index when running in SMP mode.
### Impact
- This PR only affects the hello app.
### Testing
- I only tested the app with Sony Spresense in SMP/non-SMP modes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo edited a comment on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580250205
For pthreads, GNU supports the non-POSIX (_np) function so set affinity and also:
int pthread_getaffinity_np(pthread_t thread, size_t cpusetsize, FAR cpu_set_t *cpuset);
But there is no application interface like this for tasks. There is the internal (only) nxsched_getaffinity().
In NuttX all tasks are the same, so you could argue that nxsched_getaffinity() could then become a systen call, replacing the pthread_getaffinity_np() system call . sched_getaffinity() and pthread_getaffinity_np() would then be demoted to a libs/libc/pthread functions that just invoke the nxsched_getaffinity() system call (nxsched_getaffinity() would be the system call because the return values are different for the two libs/libc/pthread functions.
NOTE that those functions do NOT return the current CPU. They return the CPU set assigned by the setaffinity() interfaces. They do NOT tell you which CPU the thread is running on.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580159799
Yes, but the better approach is expose this information through the offiical API, something lke sched_getcpu:
http://man7.org/linux/man-pages/man3/sched_getcpu.3.html
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] jerpelea commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580246167
I agree and respect your decision
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580560510
"Yes, hello isn't a good place but how about apps/testing/smp/smp_main.c? It's reasonable to query the current cpu for the diagnose, actually it call up_cpu_index now!"
This is incorrect. Nothing in apps/ can call up_cpuindex(). That violates the portable POSIX interface and will not work in PROTECTED or KERNEL mode. In those modes, there must be system calls.
I am still opposed to this as will and will close and/or revert any such PR. It is just a bad idea no metter where you put it.
The interface does not work. It serves no functional pupose because it DOES NOT tell you CPU you are running on; it tells what CPU you were running on in the past. So the returned CPU number cannot be used programmatically for anything meaninful.
I will do everything to prevent the implementation of the non-standard interface. It is wrong in so many ways.
These best way to print the CPU number would to be extend syslog so that when SMP is enabled it decorates each syslog output with the CPU number. This is done inside the OS and so does not require any special application interface. Then a special version of "Hello, World!" that uses syslog could safely print the CPU number at the moment that the syslog() executes.
That would show the CPU, remain POSIX compliant, and would work in PROTECTED and KERNEL builds without introducing a useless system 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] jerpelea commented on a change in pull
request #37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#discussion_r372910258
##########
File path: examples/hello/hello_main.c
##########
@@ -50,6 +54,12 @@
int main(int argc, FAR char *argv[])
{
+#ifdef CONFIG_SMP
+ uint32_t cpu = up_cpu_index();
Review comment:
@patacongo
thanks for pointing out the protected and kernel builds I will take then into consideration
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#discussion_r372894731
##########
File path: examples/hello/hello_main.c
##########
@@ -50,6 +54,12 @@
int main(int argc, FAR char *argv[])
{
+#ifdef CONFIG_SMP
+ uint32_t cpu = up_cpu_index();
Review comment:
Neither arevproper posix interfaces and neither show be called or implemted in you space. This must be reverted.
It will also break any SMP PROTECTED or kernel mode build.
You could make an exteneion to prctl(). The at least uses a Linux-like interface and will not break other builds.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580149014
@jerpelea my comment doesn't get addressed, it's very bad practice to call OS internal fucntion from user space.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580255739
"NOTE that those functions do NOT return the current CPU. They return the CPU set assigned by the setaffinity() interfaces."
You all understand that an application interface that returns the current CPU is not a usable thing. Within the OS, you can get the CPU number only in a critical section or with pre-emption disabled. You cannot do those things in applications.
So the value is unpredictable an volatile if used by applications. For example, an application running on CPU A when the interface function is called may be running on CPU B when the inteface function returns. It is absolutely useless in any real world application.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580553764
Also, I do not think hello, world! is an appropriate place for that kind of stuff. Please do not modify it.
I suggested instead that you implement your own private hello world inside of spresense/src. Please do not modify apps/examples/hello and please DO NOT implemnt any non standard OS interfaces. DO NOT implement sched_getcpu(). That must never go into the OS.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580243550
"We need to see the CPU affiliation for SMP"... maybe so (although is it volatile and not reliable in applications. But even if so, you must obey the Inviolables and the architectual principles or the OS. You cannot put OS internal calls in applications. Now can you invent non-standard OS interfaces. That can never be tolerated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] jerpelea merged pull request #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
jerpelea merged pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580517143
Thanks for all of the comments above. I'll consider to implement sched_getcpu() API.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580564661
sched_lock() is made available to applications via a syscall. That was a very bad decision that I made long ago. If pre-emption is disabled, then the task would be locked to a CPU and should be stable until sched_unlock() is called. In that case, the CPU number would remain valid thoughout that interval (but might likely change when sched_unlock() is called).
But I don't like the idea of using several non-standard interfaces to make a new non-standard interface work. That just does not feel good to me. I would prefer that we do not go this way at all, this is not in the spirit of OpenGroup.org and the Inviolable principles of the OS.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo edited a comment on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580556832
I am very opposed to all of these changes. The interfaces are useless, Hello, World! must not be modified, the interfaces are non-standard and non-process. It goes against the Inviolable principles of the OS. We cannot permit this to happen, ever.
Please do not do any of why you proposed. Please do nothing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580817657
"... but the better approach is expose this information through the offiical API, something lke sched_getcpu: http://man7.org/linux/man-pages/man3/sched_getcpu.3.html"
I am not sure exactly how official that is. It is not a Linux API, git is a non-standard GLIBC interface that requires __GNU_SOURCE be defined to be available. That is far from standard; it is not even standard for GLIBC.
sched_getcpu() is really just a wrapper around the Linux system call getcpu(): http://man7.org/linux/man-pages/man2/getcpu.2.html
It is interested to note:
"The information placed in cpu is guaranteed to be current only at the time of the call..., the kernel might change the CPU at any time. (_Normally this does not happen because the scheduler tries to minimize movements between CPUs to keep caches hot, but it is possible_.) The caller must allow for the possibility that the information returned ... is no longer current by the time the call returns.
But in a realtime system, we cannot behave that way. realtime behavior is achieved by always being responsive and not by deferring CPU context switches to get better performance and throughput. That is why NuttX will never be a good desktop OS and Linux, without extensions, will never be a good realtime OS: Baseline Linux focus on throughput, and RTOS focuses on responsiveness.
The result is that CPU switches will occur much more unpredictably and at a much higher rate than would be with Linux. This will give us realtime response, but the penalty that we pay is loss of throughput, primarily because of poorer use of data caches -- if available.
And it also means that any user-space API to get the current CPU will be essentially useless on a very busy system because it will very often not report the correct CPU that the thread is running on. How often? For low priority tasks in such a busy system, very often.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580556894
Yes, hello isn't a good place but how about apps/testing/smp/smp_main.c? It's reasonable to query the current cpu for the diagnose, actually it call up_cpu_index now!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 edited a comment on issue
#37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580149014
@jerpelea my comment doesn't get addressed, it's very bad practice to call OS internal fucntion from user space. We need avoid this as much as possible
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580250205
For pthreads, GNU supports the non-POSIX (_np) function so set affinity and also:
int pthread_getaffinity_np(pthread_t thread, size_t cpusetsize, FAR cpu_set_t *cpuset);
But there is no application interface like this for tasks. There is the internal (only) nxsched_getaffinity().
In NuttX all tasks are the same, so you could argue as Xaio Xiang did that nxsched_getaffinity() could then become a systen call, replacing the pthread_getaffinity_np() system call . sched_getaffinity() and pthread_getaffinity_np() would then be demoted to a libs/libc/pthread functions that just invoke the nxsched_getaffinity() system call (nxsched_getaffinity() would be the system call because the return values are different for the two libs/libc/pthread functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580556832
I am very opposed to all of these changes. The interfaces are useless, Hello, World! must not be modified, the interfaces are non-standard and non-process. We cannot permit this to happen.
Please do not do any of why you proposed. Please do nothing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580552876
I don't think sched_getcpu() is a good idea. It is a useless application interface and is not POSIX. I will decline that. Please do not do that. A very bad idea.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580556356
I want apps/examples/hello/ to remain a pure "Hello, World!" example. It is wrong to mix test data into the pure "Hello, World!' Please do not do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580160245
> Sorry I can't see any comment here
> We need to see the CPU affiliation for SMP
Sorry, I forget to submit my comment.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#discussion_r372934317
##########
File path: examples/hello/hello_main.c
##########
@@ -50,6 +54,12 @@
int main(int argc, FAR char *argv[])
{
+#ifdef CONFIG_SMP
+ uint32_t cpu = up_cpu_index();
Review comment:
Many of the things in boardctl() really should be moved to a new sysctl() what handles all system-level setting. All task-level setting (including CPU) should be provided by prctl() (process control). Only board level stuff should be included in boardctl().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #37:
examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580561988
Yes, it's better to extend syslog and let smp test call syslog instead of printf.
But is it reasonable to follow Linux practice if POSIX doesn't define something but very useful(don't like up_cup_index)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580557678
In addition to being just a horrible thing to the to the OS, the interface is useless. I mentioned this above. It will not tell which CPU you are running on. It can only tell which CPU it was running on when the interface called. It could be running on a different CPU after it returns. So not only is violating the POSIX interface a HORRIBLE idea, then proposed interface will not work.
I will resist this change will all of my strength. I am horrified that we are considering this. I thought we all had more respect for the portable POSIX interface and the Inviolable principles to through all that away for a useless garbage interface. We cannot do this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#discussion_r372933612
##########
File path: examples/hello/hello_main.c
##########
@@ -50,6 +54,12 @@
int main(int argc, FAR char *argv[])
{
+#ifdef CONFIG_SMP
+ uint32_t cpu = up_cpu_index();
Review comment:
I would revert sched_getcpu() as well. All OS application interfaces must be defined at opengroup.org or use an well established hook like ioctl(), prctl(), boarctl(), and maybe sysctl().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] jerpelea commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580160581
@masayuki2009 please investigate the API approach
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] jerpelea commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580155772
Sorry I can't see any comment here
We need to see the CPU affiliation for SMP
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in
pull request #37: examples: hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#discussion_r372767700
##########
File path: examples/hello/hello_main.c
##########
@@ -50,6 +54,12 @@
int main(int argc, FAR char *argv[])
{
+#ifdef CONFIG_SMP
+ uint32_t cpu = up_cpu_index();
Review comment:
application shouldn't call up_xxx API directly, it's better to implement something like:
int sched_getcpu(void);
and add this api to syscall/ list
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on issue #37: examples:
hello: Show CPU index when running in SMP mode
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #37: examples: hello: Show CPU index when running in SMP mode
URL: https://github.com/apache/incubator-nuttx-apps/pull/37#issuecomment-580563032
"But is it reasonable to follow Linux practice if POSIX doesn't define something but very useful(don't like up_cup_index)?"
I agree, but this interface is not useful and will cause bugs if anyone were to actually do anything with it. Just using it in a printf() is harmless (but might print the wrong value). Making any decisions based on the CPU would would not work because the value is volatile and cannot be used in that way.
So such an interface would be a support and maintainence problem because it is inherently buggy and returns incorrect values. Best not to support it; best not to introduce a non-standard interface that provides no value.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services