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 2023/01/18 13:33:43 UTC
[GitHub] [nuttx] xiaoxiang781216 opened a new pull request, #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
xiaoxiang781216 opened a new pull request, #8182:
URL: https://github.com/apache/nuttx/pull/8182
## Summary
- sched/semaphore: Change SEM_PRIO_MUTEX to SEM_TYPE_MUTEX
- sim/usrsocktest: Enable CONFIG_DEBUG_ASSERTIONS
Fix regression report here: https://github.com/apache/nuttx/pull/8164
## Impact
mutex
## Testing
Pass CI
--
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
[GitHub] [nuttx] anchao commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1407388242
Hi @masayuki2009 san,
Please review this PR again, I restored the mutex holder changes, and fixed the issues your mentioned on config spresense:rndis_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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] masayuki2009 commented on pull request #8182: Add the holder for mutex
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1408380187
@xiaoxiang781216
I will try the latest PR tomorrow.
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8182: Add the holder for mutex
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#discussion_r1090897946
##########
boards/sim/sim/sim/configs/usrsocktest/defconfig:
##########
@@ -13,6 +13,8 @@ CONFIG_ARCH_CHIP="sim"
CONFIG_ARCH_SIM=y
CONFIG_BOARDCTL_POWEROFF=y
CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_SYMBOLS=y
Review Comment:
Here is the patch: https://github.com/apache/nuttx/pull/8364
--
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
[GitHub] [nuttx] masayuki2009 merged pull request #8182: Add the holder for mutex
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 merged PR #8182:
URL: https://github.com/apache/nuttx/pull/8182
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8182: Add the holder for mutex
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1409680278
Thanks, @masayuki2009 .
--
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
[GitHub] [nuttx] masayuki2009 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1387097263
Hmm, spresense:rndis_smp and spresesnse:wifi_smp are still unstable.
--
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
[GitHub] [nuttx] masayuki2009 commented on a diff in pull request #8182: Add the holder for mutex
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on code in PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#discussion_r1090885862
##########
boards/sim/sim/sim/configs/usrsocktest/defconfig:
##########
@@ -13,6 +13,8 @@ CONFIG_ARCH_CHIP="sim"
CONFIG_ARCH_SIM=y
CONFIG_BOARDCTL_POWEROFF=y
CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_SYMBOLS=y
Review Comment:
@xiaoxiang781216
Should we add these configs to `./boards/sim/sim/sim/configs/citest/defconfig` instead of this file?
--
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
[GitHub] [nuttx] masayuki2009 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1387221181
>Ok, let's revert https://github.com/apache/nuttx/pull/8164 utils we found why spresense:rndis_smp stop working.
Revert PR is https://github.com/apache/nuttx/pull/8181
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8182: Add the holder for mutex
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#discussion_r1090892898
##########
boards/sim/sim/sim/configs/usrsocktest/defconfig:
##########
@@ -13,6 +13,8 @@ CONFIG_ARCH_CHIP="sim"
CONFIG_ARCH_SIM=y
CONFIG_BOARDCTL_POWEROFF=y
CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_SYMBOLS=y
Review Comment:
let me create a new patch for ./boards/sim/sim/sim/configs/citest/defconfig.
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1407388184
@masayuki2009 @anchao found the root cause, could you try this patch again?
<img width="474" alt="image" src="https://user-images.githubusercontent.com/18066964/215266192-bb741a9d-4b5d-4293-b9de-0280a13d90d0.png">
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1407399131
let's squash the fix into the main patch? @anchao
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1408157702
@masayuki2009 could you verify this patch again? 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
[GitHub] [nuttx] masayuki2009 commented on pull request #8182: Add the holder for mutex
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1409676369
@xiaoxiang781216
>Hmm, spresense:rndis_smp and spresesnse:wifi_smp are still unstable.
I confirmed that the latest PR is stable.
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1387107751
> Hmm, spresense:rndis_smp and spresesnse:wifi_smp are still unstable.
Ok, let's revert #8164 utils we found why spresense:rndis_smp stop working.
--
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
[GitHub] [nuttx] anchao commented on pull request #8182: sched/mutex: Return the error code correctly in nxmutex_timedlock
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #8182:
URL: https://github.com/apache/nuttx/pull/8182#issuecomment-1407414787
> let's squash the fix into the main patch? @anchao
Ok, let me keep the last commit message here:
```
nuttx/mutex: fix race condition of holder update
CPU0 | CPU1
|
static inline int |
nxmutex_unlock(FAR mutex_t *mutex) |
{ |
... |
--> 1. CPU0 unlock the mutex |
|
ret = _SEM_POST(&mutex->sem); |
... |
|--> 2. CPU1 take the mutex before no holder update.
|
| static inline int nxmutex_lock(FAR mutex_t *mutex)
| {
| ...
| for (; ; )
| {
| /* Take the semaphore (perhaps waiting) */
|
| ret = _SEM_WAIT(&mutex->sem);
| if (ret >= 0)
| {
|------------> 3. CPU1 update the holder to current thread
| mutex->holder = gettid();
| break;
| }
| }
| ...
| }
... |
--> 4. CPU0 clean the mutex holder wrongly |
|
mutex->holder = NXMUTEX_NO_HOLDER; |
|
return ret; |
} |
|
| static inline int nxmutex_unlock(FAR mutex_t *mutex)
| {
|----> 5. CPU1 unlock the invaild hold, assert
|
| DEBUGASSERT(nxmutex_is_hold(mutex));
```
```
[CPU1] [ 3] _assert: Current Version: NuttX 10.4.0 https://github.com/apache/nuttx/commit/9bdd022b6cfd55d2d233221aaa611404c60167b7-dirty Jan 28 2023 18:31:34 arm
[CPU1] [ 3] _assert: Assertion failed: nxmutex_is_hold(mutex) at file: nuttx/include/nuttx/mutex.h:349 task(CPU1): lpwork 0xd00c79d
```
Signed-off-by: chao an <an...@xiaomi.com>
--
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