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 2021/03/01 02:30:34 UTC
[GitHub] [incubator-nuttx] masayuki2009 opened a new pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
masayuki2009 opened a new pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938
## Summary
- The code was added in Mar 2018 to stabilize the SMP kernel
- I confirmed that the code is no longer needed now.
## Impact
- SMP only
## Testing
- Tested with ostest the following configs
- esp32-devkitc:smp (QEMU), sabre-6quad:smp (QEMU)
- maix-bit:smp (QEMU), sim:smp
- spresense:smp
- Tested with nxplayer and stress test with spresense:wifi_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
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-787594702
Unrelated to your change, but shouldn't the calls to getpid() be changed to calls to gettid()? The logic implements that thread-reentrant lock. gettid() is the API that returns the thread ID, not getpid().
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-787884982
> Unrelated to your change, but shouldn't the calls to getpid() be changed to calls to gettid()? The logic implements that thread-reentrant lock. gettid() is the API that returns the thread ID, not getpid().
Yes, but we can do the change with PR: https://github.com/apache/incubator-nuttx/pull/2518
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-788559195
There are many places inovke getpid, we need time to check each place and ensure the right function get called:(.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] patacongo commented on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-788243589
>
>
> > Unrelated to your change, but shouldn't the calls to getpid() be changed to calls to gettid()? The logic implements that thread-reentrant lock. gettid() is the API that returns the thread ID, not getpid().
>
> Yes, but we can do the change with PR: #2518
Okay. Nevermind. I thought that had been merged.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] patacongo commented on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-787594702
Unrelated to your change, but shouldn't the calls to getpid() be changed to calls to gettid()?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] patacongo commented on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-788921674
>
>
> There are many places inovke getpid, we need time to check each place and ensure the right function get called:(.
Perhaps we could put the changes on a branch. I could help out some with the conversion.
However, we should wait until after the next 10.1.x release to give the changes as much time as possible to mature before 10.2.
There should be no real use of the (corrected) getpid within the OS other than at the OS API. There are several functions that take the pid as a parameter (and usually if the pid is zero, then it should use getpid() to get the pid of the caller). I suppose these should return only information for the main thread:
```
sched.h:int task_delete(pid_t pid);
sched.h:int task_restart(pid_t pid);
sched.h:int sched_setparam(pid_t pid, FAR const struct sched_param *param);
sched.h:int sched_getparam(pid_t pid, FAR struct sched_param *param);
sched.h:int sched_setscheduler(pid_t pid, int policy,
sched.h:int sched_getscheduler(pid_t pid);
sched.h:int sched_rr_get_interval(pid_t pid, FAR struct timespec *interval);
sched.h:int sched_setaffinity(pid_t pid, size_t cpusetsize,
sched.h:int sched_getaffinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask);
signal.h:int kill(pid_t pid, int signo);
spawn.h:int posix_spawnp(FAR pid_t *pid, FAR const char *path,
spawn.h:int posix_spawn(FAR pid_t *pid, FAR const char *path,
spawn.h:int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
termios.h:pid_t tcgetsid(int fd);
unistd.h:pid_t vfork(void);
unistd.h:pid_t getpid(void);
unistd.h:pid_t gettid(void);
unistd.h:pid_t getppid(void);
```
Within the OS, I think that all of the internal calls to getpid() can be converted gettid().
There is also a lot of naming of variables and structure fields that would need to be corrected.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] patacongo removed a comment on pull request #2938: libs: misc: Remove critical section in lib_filesem.c for SMP
Posted by GitBox <gi...@apache.org>.
patacongo removed a comment on pull request #2938:
URL: https://github.com/apache/incubator-nuttx/pull/2938#issuecomment-788921674
>
>
> There are many places inovke getpid, we need time to check each place and ensure the right function get called:(.
Perhaps we could put the changes on a branch. I could help out some with the conversion.
However, we should wait until after the next 10.1.x release to give the changes as much time as possible to mature before 10.2.
There should be no real use of the (corrected) getpid within the OS other than at the OS API. There are several functions that take the pid as a parameter (and usually if the pid is zero, then it should use getpid() to get the pid of the caller). I suppose these should return only information for the main thread:
```
sched.h:int task_delete(pid_t pid);
sched.h:int task_restart(pid_t pid);
sched.h:int sched_setparam(pid_t pid, FAR const struct sched_param *param);
sched.h:int sched_getparam(pid_t pid, FAR struct sched_param *param);
sched.h:int sched_setscheduler(pid_t pid, int policy,
sched.h:int sched_getscheduler(pid_t pid);
sched.h:int sched_rr_get_interval(pid_t pid, FAR struct timespec *interval);
sched.h:int sched_setaffinity(pid_t pid, size_t cpusetsize,
sched.h:int sched_getaffinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask);
signal.h:int kill(pid_t pid, int signo);
spawn.h:int posix_spawnp(FAR pid_t *pid, FAR const char *path,
spawn.h:int posix_spawn(FAR pid_t *pid, FAR const char *path,
spawn.h:int task_spawn(FAR pid_t *pid, FAR const char *name, main_t entry,
termios.h:pid_t tcgetsid(int fd);
unistd.h:pid_t vfork(void);
unistd.h:pid_t getpid(void);
unistd.h:pid_t gettid(void);
unistd.h:pid_t getppid(void);
```
Within the OS, I think that all of the internal calls to getpid() can be converted gettid().
There is also a lot of naming of variables and structure fields that would need to be corrected.
----------------------------------------------------------------
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