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 2022/10/25 12:32:33 UTC
[GitHub] [incubator-nuttx] XinStellaris opened a new pull request, #7426: mutex:move mutex holder to struct mutex_s.
XinStellaris opened a new pull request, #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426
Signed-off-by: 田昕 <ti...@xiaomi.com>
## Summary
We move the mutex holder to struct mutex_s so that we can trace who holds the mutex when deadlock happens.
We also print possible mutex holder in sched_dumpstack.
## Impact
mutex_t and related code.
## Testing
Tested on ESP32C3
--
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] [incubator-nuttx] GUIDINGLI commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004436815
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
+ {
+ mutex = (FAR mutex_t *)tcb->waitobj;
+ syslog(LOG_EMERG, "STATE:WAIT_SEM|%2d, maybe held by %2d\n", tid,
Review Comment:
should not be %2d, thread id may over than 1000
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
Review Comment:
move to line 56
--
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] [incubator-nuttx] GUIDINGLI commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004436815
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
+ {
+ mutex = (FAR mutex_t *)tcb->waitobj;
+ syslog(LOG_EMERG, "STATE:WAIT_SEM|%2d, maybe held by %2d\n", tid,
Review Comment:
should not be %2d, thread id may over than 1000
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1005188835
##########
include/nuttx/mutex.h:
##########
@@ -522,11 +523,11 @@ static inline int nxrmutex_breaklock(FAR rmutex_t *rmutex,
pid_t tid = gettid();
int ret = -EPERM;
- if (rmutex->holder == tid)
+ if (rmutex->mutex.holder == tid)
Review Comment:
there is no count in struct mutex_s.
I don't see the need to break lock.
--
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] [incubator-nuttx] XinStellaris commented on pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#issuecomment-1290475970
@xiaoxiang781216 @liguiding please review
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1008717062
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
An other approach could add a new flag in sem_t::flags to indicate whether is mutex or sem
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1005189882
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
how about we design an internal function like:
int _nxsem_wait(FAR sem_t *sem, bool is_mutex)
Then we can differentiate mutex and sem.
After that, we can add a new TSTATE_WAIT_MUTEX, then we know if a task is waiting on mutex, and we can print the mutex holder in such case.
--
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] [incubator-nuttx] GUIDINGLI commented on pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#issuecomment-1290499226
> I am not sure whether we should print mutex holder in sched_dumpstack. It may be helpful when we lack coredump or gdb.
In a real device and there is no JTAG (mainly for the device just sold out), this method will be helpful for debugging
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004483651
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
Review Comment:
cmd_ps seems a good place to go.
What do you mean by crash dump? board_crashdump?
--
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] [incubator-nuttx] XinStellaris commented on pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#issuecomment-1293114155
@anjiahao1 please review 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#issuecomment-1290501958
This change reminded me https://github.com/apache/incubator-nuttx/pull/6376#discussion_r893228330 discussion.
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004464049
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
Review Comment:
why not move to ps and crash dump?
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004573267
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -53,8 +53,28 @@
void sched_dumpstack(pid_t tid)
{
+ FAR struct tcb_s *tcb = NULL;
int size = DUMP_DEPTH;
int skip;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
+ {
+ mutex = (FAR mutex_t *)tcb->waitobj;
Review Comment:
let's split the change to new patch.
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -53,8 +53,28 @@
void sched_dumpstack(pid_t tid)
{
+ FAR struct tcb_s *tcb = NULL;
int size = DUMP_DEPTH;
int skip;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
+ {
+ mutex = (FAR mutex_t *)tcb->waitobj;
Review Comment:
let's split the dump to new patch.
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004578059
##########
include/nuttx/mutex.h:
##########
@@ -522,11 +523,11 @@ static inline int nxrmutex_breaklock(FAR rmutex_t *rmutex,
pid_t tid = gettid();
int ret = -EPERM;
- if (rmutex->holder == tid)
+ if (rmutex->mutex.holder == tid)
{
*count = rmutex->count;
rmutex->count = 0;
- rmutex->holder = NXRMUTEX_NO_HOLDER;
+ rmutex->mutex.holder = NXRMUTEX_NO_HOLDER;
Review Comment:
remove
##########
include/nuttx/mutex.h:
##########
@@ -458,7 +461,7 @@ static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
static inline bool nxrmutex_is_hold(FAR rmutex_t *rmutex)
Review Comment:
add nxmutex_is_hold
##########
include/nuttx/mutex.h:
##########
@@ -557,11 +558,11 @@ static inline int nxrmutex_restorelock(FAR rmutex_t *rmutex,
pid_t tid = gettid();
int ret;
- DEBUGASSERT(rmutex->holder != tid);
+ DEBUGASSERT(rmutex->mutex.holder != tid);
Review Comment:
add nxmutex_restorelock
##########
include/nuttx/mutex.h:
##########
@@ -522,11 +523,11 @@ static inline int nxrmutex_breaklock(FAR rmutex_t *rmutex,
pid_t tid = gettid();
int ret = -EPERM;
- if (rmutex->holder == tid)
+ if (rmutex->mutex.holder == tid)
Review Comment:
add nxmutex_breaklock?
##########
include/nuttx/mutex.h:
##########
@@ -484,15 +487,13 @@ static inline bool nxrmutex_is_hold(FAR rmutex_t *rmutex)
static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
{
- pid_t tid = gettid();
int ret = OK;
- DEBUGASSERT(rmutex->holder == tid);
+ DEBUGASSERT(rmutex->mutex.holder == gettid());
DEBUGASSERT(rmutex->count > 0);
if (rmutex->count-- == 1)
{
- rmutex->holder = NXRMUTEX_NO_HOLDER;
ret = nxmutex_unlock(&rmutex->mutex);
Review Comment:
add DEBUGASSERT to nxmutex_unlock
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004569154
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
Review Comment:
e.g. arm_dumpstate
--
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] [incubator-nuttx] XinStellaris commented on pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#issuecomment-1290484908
I am not sure whether we should print mutex holder in sched_dumpstack.
It may be helpful when we lack coredump or gdb.
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004487107
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
As a matter of fact, I don't know how to tell the difference.
There is no way to differentiate waiting on a sem or on a mutex.
So I just print possible mutex holder anyway.
What's your suggestion?
--
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] XinStellaris closed pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by "XinStellaris (via GitHub)" <gi...@apache.org>.
XinStellaris closed pull request #7426: mutex:move mutex holder to struct mutex_s.
URL: https://github.com/apache/nuttx/pull/7426
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1005262042
##########
include/nuttx/mutex.h:
##########
@@ -522,11 +523,11 @@ static inline int nxrmutex_breaklock(FAR rmutex_t *rmutex,
pid_t tid = gettid();
int ret = -EPERM;
- if (rmutex->holder == tid)
+ if (rmutex->mutex.holder == tid)
Review Comment:
break is used to help caller leave the lock temp without pre knowledge whether the lock is held before. so, it's useful for no recurve case.
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004465123
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
not all semaphore have holder field.
--
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] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004495297
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
let's check the wait object. If semaphore has `PRIOINHERIT_FLAGS_ENABLE` flag then most probably it is a mutex and we can upcast it to mutex_t. That is one of the possible variants
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7426: mutex:move mutex holder to struct mutex_s.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7426:
URL: https://github.com/apache/incubator-nuttx/pull/7426#discussion_r1004570297
##########
libs/libc/sched/sched_dumpstack.c:
##########
@@ -55,6 +55,26 @@ void sched_dumpstack(pid_t tid)
{
int size = DUMP_DEPTH;
int skip;
+ FAR struct tcb_s *tcb = NULL;
+ FAR mutex_t *mutex = NULL;
+
+ /* If task is in TSTATE_WAIT_SEM, it may be waiting for a mutex,
+ * then print mutex holder to help debugging.
+ * However, if it is not a mutex, an invalid holder is printed.
+ * I think printing invalid value is acceptable since this is
+ * for debugging only.
+ */
+
+ if (tid >= 0)
+ {
+ tcb = nxsched_get_tcb(tid);
+ if (tcb != NULL && tcb->task_state == TSTATE_WAIT_SEM)
Review Comment:
But it can't handle the project which doesn't enable priority inheritance.
--
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