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/05/23 22:08:00 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request, #6318: sched/semaphore: fix priority boost restoration for priority inheritance
pkarashchenko opened a new pull request, #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318
## Summary
Attempt to fix priority boost restoration in case of priority inheritance
## Impact
## Testing
WIP
--
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] acassis commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1262678111
Crashing here too: https://pastebin.com/raw/VCGzC6kN
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r899505602
##########
sched/semaphore/sem_holder.c:
##########
@@ -495,111 +417,30 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
if (htcb->sched_priority != htcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
+ rpriority = htcb->base_priority;
Review Comment:
Done
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1271756120
Thank you for all your work on 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] hartmannathan commented on a diff in pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r885111667
##########
sched/semaphore/sem_holder.c:
##########
@@ -495,111 +417,30 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
if (htcb->sched_priority != htcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
+ rpriority = htcb->base_priority;
Review Comment:
Thanks for the explanation! Suggestion: Copy this explanation verbatim into a comment right above this line? We want this knowledge to stay with the code!!
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256555266
I'm going to try again with DEBUG_SCHED=y enabled... Let's see if I get more helpful output.
By the way I am running this on the b-g474e-dpow1:ostest configuration as of latest master (47b350a4c3262b2c5bb4acb223bf1d2543bcd2ad) with the following changes: (1) this PR applied, and (2) priority inheritance enabled.
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256331188
Yeah. You either can revert PR locally or wait my rebase
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r911326908
##########
sched/wqueue/kwork_inherit.c:
##########
@@ -173,128 +122,69 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
static void lpwork_restoreworker(pid_t wpid, uint8_t reqprio)
{
+ FAR struct semholder_s *pholder;
FAR struct tcb_s *wtcb;
-#if CONFIG_SEM_NNESTPRIO > 0
+ FAR struct tcb_s *stcb;
uint8_t wpriority;
- int index;
- int selected;
-#endif
/* Get the TCB of the low priority worker thread from the process ID. */
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
+ /* REVISIT: Priority multi-boost is not supported */
+
+ DEBUGASSERT(wtcb->boost_priority == reqprio);
+
+ /* Clear the threat boost priority */
+
+ wtcb->boost_priority = 0;
+
/* Was the priority of the worker thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
if (wtcb->sched_priority != wtcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (wtcb->npend_reprio < 1)
- {
- /* No... the worker thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the client task (reqprio)
- *
- * That latter assumption may not be true if the client's priority
- * was also boosted so that it no longer matches the wtcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* wtcb->sched_priority == reqprio && */
- wtcb->npend_reprio == 0);
-
- /* Reset the worker's priority back to the base priority. */
-
- nxsched_reprioritize(wtcb, wtcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The worker thread's
- * "boosted" priority could greater than or equal to "reqprio" (it
- * could be greater if its priority we boosted because it also holds
- * some semaphore).
+ /* We attempt to restore task priority to its base priority. If there
+ * is any task with the higher priority waiting for the semaphore
+ * held by wtcb then this value will be overwritten.
*/
- else if (wtcb->sched_priority <= reqprio)
- {
- /* The worker thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest pending priority.
- */
-
- /* Find the highest pending priority and remove it from the list */
+ wpriority = wtcb->base_priority;
- for (index = 1, selected = 0; index < wtcb->npend_reprio; index++)
- {
- if (wtcb->pend_reprios[index] > wtcb->pend_reprios[selected])
- {
- selected = index;
- }
- }
-
- /* Remove the highest priority pending priority from the list */
-
- wpriority = wtcb->pend_reprios[selected];
- index = wtcb->npend_reprio - 1;
- if (index > 0)
- {
- wtcb->pend_reprios[selected] = wtcb->pend_reprios[index];
- }
-
- wtcb->npend_reprio = index;
-
- /* And apply that priority to the thread (while retaining the
- * base_priority)
- */
+ /* Try to find the highest priority across all the tasks that are
+ * waiting for any semaphore held by wtcb. Skip search if htcb does
+ * not hold any semaphore.
+ */
- nxsched_set_priority(wtcb, wpriority);
- }
- else
+ if (wtcb->holdsem != NULL)
{
- /* The worker thread has been boosted to a higher priority than the
- * waiter task. The pending priority should be in the list (unless
- * it was lost because of list overflow or because the worker was
- * reprioritized again unbeknownst to the priority inheritance
- * logic).
- *
- * Search the list for the matching priority.
- */
-
- for (index = 0; index < wtcb->npend_reprio; index++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
+ stcb != NULL && stcb->sched_priority > wpriority;
+ stcb = stcb->flink)
{
- /* Does this pending priority match the priority of the thread
- * that just received the count?
- */
-
- if (wtcb->pend_reprios[index] == reqprio)
+ if ((stcb->waitsem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
{
- /* Yes, remove it from the list */
-
- selected = wtcb->npend_reprio - 1;
- if (selected > 0)
+ for (pholder = wtcb->holdsem; pholder != NULL;
+ pholder = pholder->tlink)
{
- wtcb->pend_reprios[index] =
- wtcb->pend_reprios[selected];
+ if (pholder->sem == stcb->waitsem &&
+ pholder->counts > 0)
+ {
+ wpriority = stcb->sched_priority;
+ break;
+ }
}
-
- wtcb->npend_reprio = selected;
- break;
}
}
}
-#else
- /* There is no alternative restore priorities, drop the priority
- * of the worker thread all the way back to the threads "base"
- * priority.
+
+ /* Apply the selected priority to the thread (hopefully back to the
Review Comment:
reminder to self: need to fix the 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.
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257190965
By the way, how many `CONFIG_SEM_PREALLOCHOLDERS` "Number of pre-allocated holders" are you using? (I used the default of 8.)
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257186809
Checking right 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.
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256326454
Oh, I misunderstood... #7159 is the PR that renamed stuff, not your re-based PR. Okay, I will wait for the new PR. Ping me when it's ready!
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r975271022
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +411,52 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
Review Comment:
Ok. I will extend the comment. This case covers the kwork thread that might have boosted priority.
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256534802
> @hartmannathan rebased
@pkarashchenko OK I will try it 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.
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] hartmannathan commented on a diff in pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r973093859
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +411,52 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
Review Comment:
Are we boosting (raising) the priority here, or lowering it?
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1137200796
> @pkarashchenko please include more details about the issue you are trying to fix
The issue is https://github.com/apache/incubator-nuttx/issues/6310
I will update the summary
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r911326090
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +411,52 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
- if (htcb->sched_priority != htcb->base_priority)
+ if (htcb->sched_priority != hpriority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The holder thread's
- * "boosted" priority could greater than or equal to
- * "stcb->sched_priority" (it could be greater if its priority was
- * boosted because it also holds another semaphore).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
- {
- /* The holder thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest priority that we have in
- * rpriority.
- */
-
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
- {
- if (htcb->pend_reprios[i] > htcb->pend_reprios[j])
- {
- j = i;
- }
- }
-
- /* Remove the highest priority pending priority from the list */
-
- rpriority = htcb->pend_reprios[j];
- i = htcb->npend_reprio - 1;
- if (i > 0)
- {
- htcb->pend_reprios[j] = htcb->pend_reprios[i];
- }
-
- htcb->npend_reprio = i;
-
- /* And apply that priority to the thread (while retaining the
- * base_priority)
- */
-
- nxsched_set_priority(htcb, rpriority);
- }
- else
+ if (htcb->holdsem != NULL)
{
- /* The holder thread has been boosted to a higher priority than the
- * waiter task. The pending priority should be in the list (unless
- * it was lost because of list overflow or because the holder
- * was reprioritized again unbeknownst to the priority inheritance
- * logic).
- *
- * Search the list for the matching priority.
- */
-
- for (i = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
+ stcb != NULL && stcb->sched_priority > hpriority;
+ stcb = stcb->flink)
{
- /* Does this pending priority match the priority of the thread
- * that just received the count?
- */
-
- if (htcb->pend_reprios[i] == stcb->sched_priority)
+ if ((stcb->waitsem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
{
- /* Yes, remove it from the list */
-
- j = htcb->npend_reprio - 1;
- if (j > 0)
+ for (pholder = htcb->holdsem; pholder != NULL;
+ pholder = pholder->tlink)
{
- htcb->pend_reprios[i] = htcb->pend_reprios[j];
+ if (pholder->sem == stcb->waitsem &&
+ pholder->counts > 0)
+ {
+ hpriority = stcb->sched_priority;
+ break;
+ }
}
-
- htcb->npend_reprio = j;
- break;
}
}
}
-#else
- /* There is no alternative restore priorities, drop the priority
- * of the holder thread all the way back to the threads "base"
- * priority.
+
+ /* Apply the selected priority to the thread (hopefully back to the
+ * threads "base" priority).
Review Comment:
reminder to self: need to fix the 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.
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256302218
> @hartmannathan do you have any test results?
I am about to test this 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.
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256310551
> @hartmannathan do you have any test results?
I'm not sure what happened here:
```
5126 git checkout master --force
5127 git pull
5128 wget https://github.com/apache/incubator-nuttx/pull/6318.patch
5129 git apply 6318.patch
5130 git status
5131 make distclean
5132 tools/configure.sh b-g474e-dpow1:nsh
5133 make menuconfig
```
(Added `CONFIG_BUILTIN`, `CONFIG_NSH_BUILTIN_APPS`, and `CONFIG_TESTING_OSTEST`.)
```
5134 make savedefconfig
5135 make
```
(Snip)
```
CC: semaphore/sem_holder.c
semaphore/sem_holder.c: In function 'nxsem_restoreholderprio':
semaphore/sem_holder.c:439:26: error: 'struct tcb_s' has no member named 'waitsem'; did you mean 'waitdog'?
if ((stcb->waitsem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
^~~~~~~
waitdog
semaphore/sem_holder.c:444:49: error: 'struct tcb_s' has no member named 'waitsem'; did you mean 'waitdog'?
if (pholder->sem == stcb->waitsem &&
^~~~~~~
waitdog
make[1]: *** [Makefile:58: sem_holder.o] Error 1
make[1]: Leaving directory '/home/nate/ramdrive/NuttX/nuttx/sched'
make: *** [tools/LibTargets.mk:59: sched/libsched.a] Error 2
```
This is strange because PR check above shows all checks passed.
What configuration are you using to test with?
--
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] hartmannathan commented on a diff in pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r978964120
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +410,55 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
- if (htcb->sched_priority != htcb->base_priority)
+ if (htcb->sched_priority != hpriority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The holder thread's
- * "boosted" priority could greater than or equal to
- * "stcb->sched_priority" (it could be greater if its priority was
- * boosted because it also holds another semaphore).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
+ if (htcb->holdsem != NULL)
{
- /* The holder thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest priority that we have in
- * rpriority.
- */
+ FAR struct tcb_s *stcb;
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
+ stcb != NULL && stcb->sched_priority > hpriority;
+ stcb = stcb->flink)
{
- if (htcb->pend_reprios[i] > htcb->pend_reprios[j])
- {
- j = i;
- }
- }
+ FAR sem_t *sem = stcb->waitobj;
Review Comment:
I haven't run any tests yet, but the compiler points out that this `sem` shadows the function parameter of the same name. Maybe rename it to avoid confusion / future errors?
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257196243
Sadly, no joy with `tm4c129e-launchpad:ostest` (+ `CONFIG_PRIORITY_INHERITANCE=y` added manually) either.
Although `b-g474e-dpow1` is STM32 and `tm4c129e-launchpad` is Tiva, they are both armv7-m. However, they seem to be crashing in different parts of the test.
I am beginning to wonder if the problem is not in priority inheritance, but in something else that is manifested because of conditions created in the priority inheritance test.
Below is the log.
```
user_main: priority inheritance test
priority_inheritance: Started
priority_inheritance: Starting lowpri_thread-1 (of 3) at 1
priority_inheritance: Set lowpri_thread-1 priority to 1
priority_inheritance: Starting lowpri_thread-2 (of 3) at 1
lowpri_thread-1: Started
priority_inheritance: Set lowpri_thread-2 priority to 1
lowpri_thread-1: initial priority: 1
priority_inheritance: Starting lowpri_thread-3 (of 3) at 1
lowpri_thread-2: Started
priority_inheritance: Set lowpri_thread-3 priority to 1
lowpri_thread-1: Waiting for the midle pri task to run
priority_inheritance: Waiting...
lowpri_thread-2: initial priority: 1
lowpri_thread-3: Started
g_middlestate: 0
lowpri_thread-2: Waiting for the midle pri task to run
lowpri_thread-3: initial priority: 1
g_highstate[0]: 0
g_middlestate: 0
lowpri_thread-3: Waiting for the midle pri task to run
I still have a count on the semaphore
g_highstate[0]: 0
g_middlestate: 0
I still have a count on the semaphore
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-1: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-2: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-3: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
priority_inheritance: Starting medpri_thread at 99
priority_inheritance: Set medpri_thread priority to 99
priority_inheritance: Waiting...
medpri_thread: Started ... I won't let go of the CPU!
priority_inheritance: Starting highpri_thread-1 (of 1) at 255
priority_inheritance: Set highpri_thread-1 priority to 255
highpri_thread-1: Started
priority_inheritance: Waiting for highpri_thread-1 to complete
highpri_thread-1: Calling sem_wait()
lowpri_thread-3: Sem count: -1, No. highpri thread: 1
highpri_thread-1: SUCCESS midpri_thread is still running!
highpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for medpri_thread to complete
medpri_thread: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-1 to complete
lowpri_thread-2: Sem count: 1, No. highpri thread: 0
lowpri_thread-2: SUCCESS the middle priority task has already exitted!
g_middlestate: 3 sem count=1
g_highstate[0]: 3
lowpri_thread-2: SUCCESS priority before sem_post: 1
lowpri_thread-1: Sem count: 2, No. highpri thread: 0
lowpri_thread-3: SUCCESS priority before sem_post: 255
lowpri_thread-2: SUCCESS final priority: 1
lowpri_thread-1: SUCCESS the middle priority task has already exitted!
lowpri_thread-3: SUCCESS final priority: 1
lowpri_thread-2: Okay... I'm done!
g_middlestate: 3 sem count=2
lowpri_thread-3: Okay... I'm done!
g_highstate[0]: 3
lowpri_thread-1: SUCCESS priority before sem_post: 1
lowpri_thread-1: SUCCESS final priority: 1
lowpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-2 to complete
priority_inheritance: Waiting for lowpri_thread-3 to complete
priority_inheritance: Restoraup_assert: Assertion failed at file:assert/lib_sta0
arm_registerdump: R0: 00000000 R1: 2000876c R2: 000466a4 R3: 2000876c
arm_registerdump: R4: 00000000 R5: 00000000 R6: 00000000 FP: 200114b0
arm_registerdump: R8: 00000000 SB: 00000000 SL: 00000000 R11: 00000000
arm_registerdump: IP: 00000000 SP: 20011478 LR: 000169bf PC: 0001698c
arm_registerdump: xPSR: 61000000 PRIMASK: 00000000 CONTROL: 00000000
arm_registerdump: EXC_RETURN: fffffff9
arm_dump_stack: User Stack:
arm_dump_stack: sp: 200114b0
arm_dump_stack: base: 20011260
arm_dump_stack: size: 000003c0
arm_stackdump: 200114a0: 000100c2 00016419 00000001 00000000 200114c0 20012bb0 4
arm_stackdump: 200114c0: 00000000 0001650d 00000039 20012c30 00000039 000466a8 4
arm_stackdump: 200114e0: fffffff5 00011c37 00000039 000466a8 00000001 000466a4 3
arm_stackdump: 20011500: deadbeef 000466a4 00000000 0002a113 00000000 20011594 0
arm_stackdump: 20011520: 20010000 00000003 0004fc28 200115f4 00000002 fffffff5 b
arm_stackdump: 20011540: 00000000 00000000 200115f4 00000003 0000df00 00000000 4
arm_stackdump: 20011560: 00016868 000466a4 20010a9c 0000ab25 200115f4 00000003 0
arm_stackdump: 20011580: 00000000 000466a4 00000034 000016cb 20011e7c 00000003 4
arm_stackdump: 200115a0: 000000c8 000005cb 00000000 00000000 deadbeef 00000000 3
arm_stackdump: 200115c0: deadbeef 2001168c 00000002 000466a4 00000000 00001c5b 0
arm_stackdump: 200115e0: 2001168c 00000003 2001df00 00000000 40000000 20011640 0
arm_stackdump: 20011600: 00000000 00000000 00000000 00000000 00000000 00000000 9
arm_showtasks: PID PRI STACK USED FILLED COMMAND
arm_dump_task: 0 0 1000 880 88.0%! Idle Task
arm_dump_task: 1 224 1992 492 24.6% hpwork 0x200000a0
arm_dump_task: 2 100 1992 492 24.6% lpwork 0x200000b4
arm_dump_task: 3 100 2000 1772 88.6%! nsh_main
arm_dump_task: 4 100 1984 888 44.7% Telnet daemon 0x200
arm_dump_task: 5 80 2024 1096 54.1% netinit 0
arm_dump_task: 6 100 2008 1180 58.7% ostest
arm_dump_task: 7 100 8112 1444 17.8% ostest Arg1 Arg2 A4
up_assert: Assertion failed at file:armv7-m/arm_hardfault.c line: 173 task: Tas0
arm_registerdump: R0: dc006160 R1: ffffffff R2: 200113d8 R3: dc006160
arm_registerdump: R4: 00000000 R5: 00000000 R6: 00000000 FP: 200114b0
arm_registerdump: R8: 00000000 SB: 00000000 SL: 00000000 R11: 00000000
arm_registerdump: IP: 00000000 SP: 200112b8 LR: 00012c6b PC: 000143a4
arm_registerdump: xPSR: a1000000 PRIMASK: 00000001 CONTROL: 00000000
arm_registerdump: EXC_RETURN: fffffff9
arm_dump_stack: User Stack:
arm_dump_stack: sp: 20011168
arm_dump_stack: base: 20011260
arm_dump_stack: size: 000003c0
arm_dump_stack: ERROR: User Stack pointer is not within the stack
arm_stackdump: 20011260: 0000000a 20000160 20012bb0 200112b8 00000001 00000000 0
arm_stackdump: 20011280: 200114b0 00000000 00000000 00000000 00000000 fffffff9 f
arm_stackdump: 200112a0: 200113d8 dc006160 00000000 00012c6b 000143a4 a1000000 0
arm_stackdump: 200112c0: dc006160 000466a4 01000000 00012c6b 00000038 20000000 8
arm_stackdump: 200112e0: 00046b43 00000000 00000000 200113ac 01730383 000466a4 d
arm_stackdump: 20011300: 00000000 00000000 dc006160 00000000 00000001 000466a4 0
arm_stackdump: 20011320: 00000010 20000160 20012bb0 000466a4 00000000 000466a4 1
arm_stackdump: 20011340: 200113d0 00046630 00000010 200113d0 00046b44 2001137c 4
arm_stackdump: 20011360: 00000000 000132b1 200113d4 200113a8 00046b44 200113d4 c
arm_stackdump: 20011380: 00000000 000466a4 00000058 0001228b 00001420 00046b40 f
arm_stackdump: 200113a0: 200113d4 200113ac 00000004 00013e35 00013da9 00013f61 f
arm_stackdump: 200113c0: 0000000c 000466a4 000003c0 000161df 00046b40 dc006160 0
arm_stackdump: 200113e0: 000005a4 00000011 00000008 00000020 2000e640 2001141c 4
arm_stackdump: 20011400: 00000000 20011a40 000003e8 20011e2c 00000033 000003c0 0
arm_stackdump: 20011420: 0466a420 46189b20 9a034b05 429a681b f010d001 b005fab2 0
arm_stackdump: 20011440: 02092020 012bb020 2061ac20 200466a4 00000000 00000000 4
arm_stackdump: 20011460: 00046b7c 00000000 00000000 00000000 200114b0 000291af 5
arm_stackdump: 20011480: 00001490 00040000 00000008 000466a4 00046cac 000162a5 4
arm_stackdump: 200114a0: 000100c2 0001641d 00000001 00000000 200114c0 20012bb0 4
arm_stackdump: 200114c0: 00000000 0001650d 00000039 20012c30 00000039 000466a8 4
arm_stackdump: 200114e0: fffffff5 00011c37 00000039 000466a8 00000001 000466a4 3
arm_stackdump: 20011500: deadbeef 000466a4 00000000 0002a113 00000000 20011594 0
arm_stackdump: 20011520: 20010000 00000003 0004fc28 200115f4 00000002 fffffff5 b
arm_stackdump: 20011540: 00000000 00000000 200115f4 00000003 0000df00 00000000 4
arm_stackdump: 20011560: 00016868 000466a4 20010a9c 0000ab25 200115f4 00000003 0
arm_stackdump: 20011580: 00000000 000466a4 00000034 000016cb 20011e7c 00000003 4
arm_stackdump: 200115a0: 000000c8 000005cb 00000000 00000000 deadbeef 00000000 3
arm_stackdump: 200115c0: deadbeef 2001168c 00000002 000466a4 00000000 00001c5b 0
arm_stackdump: 200115e0: 2001168c 00000003 2001df00 00000000 40000000 20011640 0
arm_stackdump: 20011600: 00000000 00000000 00000000 00000000 00000000 00000000 9
arm_showtasks: PID PRI STACK USED FILLED COMMAND
arm_dump_task: 0 0 1000 880 88.0%! Idle Task
arm_dump_task: 1 224 1992 492 24.6% hpwork 0x200000a0
arm_dump_task: 2 100 1992 492 24.6% lpwork 0x200000b4
arm_dump_task: 3 100 2000 1772 88.6%! nsh_main
arm_dump_task: 4 100 1984 888 44.7% Telnet daemon 0x200
arm_dump_task: 5 80 2024 1096 54.1% netinit 0
arm_dump_task: 6 100 2008 1180 58.7% ostest
arm_dump_task: 7 100 8112 1444 17.8% ostest Arg1 Arg2 A4
```
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257196601
When you test with stm32f4, if the test passes, it will be a good idea to compare defconfig and see if there are any differences that give a clue...
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256953958
@hartmannathan I just ran ostest on same70-qmtech board without any issues. Here is the log [ostest-same70-qmtech.txt](https://github.com/apache/incubator-nuttx/files/9638641/ostest-same70-qmtech.txt)
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256553354
Hmm... `ostest` is still crashing when priority inheritance is enabled. See below...
Now, it is possible that the crash is not caused by priority inheritance, but by something else that is triggered under the conditions produced by `ostest`.
I am not sure, because:
When priority inheritance is disabled, this part of `ostest` does not run at all.
I was not able to pinpoint the cause of the hard faults in the debugger.
What I do know is:
A project I was working on recently would hard-fault at seemingly random times. Since I turned off priority inheritance, I haven't seen any more hard-faults there.
As I wrote here: https://lists.apache.org/thread/qsyc453vc8stwd54rjxfdqjw1y5zq2fb it seems like the hard faults I was seeing were related to an unexpected ISR. I tried to identify which ISR that was, but I couldn't quite pinpoint it.
This is the output from my `ostest` run just now, with this PR applied:
```
user_main: priority inheritance test
priority_inheritance: Started
priority_inheritance: Starting lowpri_thread-1 (of 3) at 1
priority_inheritance: Set lowpri_thread-1 priority to 1
priority_inheritance: Starting lowpri_thread-2 (of 3) at 1
lowpri_thread-1: Started
priority_inheritance: Set lowpri_thread-2 priority to 1
lowpri_thread-1: initial priority: 1
priority_inheritance: Starting lowpri_thread-3 (of 3) at 1
lowpri_thread-2: Started
priority_inheritance: Set lowpri_thread-3 priority to 1
lowpri_thread-1: Waiting for the midle pri task to run
priority_inheritance: Waiting...
lowpri_thread-2: initial priority: 1
lowpri_thread-3: Started
g_middlestate: 0
lowpri_thread-2: Waiting for the midle pri task to run
lowpri_thread-3: initial priority: 1
g_highstate[0]: 0
g_middlestate: 0
lowpri_thread-3: Waiting for the midle pri task to run
I still have a count on the semaphore
g_highstate[0]: 0
g_middlestate: 0
I still have a count on the semaphore
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-1: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-2: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-3: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
priority_inheritance: Starting medpri_thread at 99
priority_inheritance: Set medpri_thread priority to 99
priority_inheritance: Waiting...
medpri_thread: Started ... I won't let go of the CPU!
priority_inheritance: Starting highpri_thread-1 (of 1) at 255
priority_inheritance: Set highpri_thread-1 priority to 255
highpri_thread-1: Started
priority_inheritance: Waiting for highpri_thread-1 to complete
highpri_thread-1: Calling sem_wait()
lowpri_thread-3: Sem count: -1, No. highpri thread: 1
highpri_thread-1: SUCCESS midpri_thread is still running!
highpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for medpri_thread to complete
medpri_thread: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-1 to complete
lowpri_thread-2: Sem count: 1, No. highpri thread: 0
lowpri_thread-2: SUCCESS the middle priority task has already exitted!
g_middlestate: 3 sem count=1
g_highstate[0]: 3
lowpri_thread-1: Sem count: 1, No. highpri thread: 0
lowpri_thread-3: SUCCESS priority before sem_post: 255
lowpri_thread-2: SUCCESS priority before sem_post: 1
lowpri_thread-1: SUCCESS the middle priority task has already exitted!
lowpri_thread-3: SUCCESS final priority: 1
lowpri_thread-2: SUCCESS final priority: 1
g_middlestate: 3 sem count=1
lowpri_thread-3: Okay... I'm done!
lowpri_thread-2: Okay... I'm done!
g_highstate[0]: 3
lowpri_thread-1: SUCCESS priority before sem_post: 1
lowpri_thread-1: SUCCESS final priority: 1
lowpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-2 to complete
priority_inheritance: Waiting for lowpri_thread-3 to complete
priority_inheritancpriority_inheritance: Waiting for Task-0 to complete
priority_inheritance: FF�]�� Started, waiting 0 uS to take count
priority_inheritance: Started, waiting 20000 uS to take count
priority_inheritance: Task0 priority was:200 is:220
priority_inheritance: (null) Posted
priority_inheritance: Task0 priority was:220 is:200
priority_inheritance: Waiting for Task-1 to complete
up_assert: Assertion failed at file:armv7-m/arm_hardfault.c line: 173 task: Tas2
arm_registerdump: R0: 000000e2 R1: 0800310d R2: 00000072 R3: 14110800
arm_registerdump: R4: 00000000 R5: 00000000 R6: 00000000 FP: 00000000
arm_registerdump: R8: 00000000 SB: 00000000 SL: 00000000 R11: 00000000
arm_registerdump: IP: 00008000 SP: 20008cc8 LR: 08010d77 PC: 080117e0
arm_registerdump: xPSR: 21000000 BASEPRI: 000000f0 CONTROL: 00000000
arm_registerdump: EXC_RETURN: ffffffe9
arm_dump_stack: User Stack:
arm_dump_stack: sp: 20008a88
arm_dump_stack: base: 200089d0
arm_dump_stack: size: 000003c0
arm_stackdump: 20008a80: 00000001 20008a90 20008a88 20008610 00000000 0800a1bd c
arm_stackdump: 20008aa0: 000000ad 08023db8 20008610 08007341 000000ad 08023db8 3
arm_stackdump: 20008ac0: 40004808 00000000 20008bf4 00000003 20008704 00008200 1
arm_stackdump: 20008ae0: 000000f0 40000000 080030a1 08002d1f 20008bf4 00000003 3
arm_stackdump: 20008b00: 00000000 08001875 00000080 08001849 00000000 00000003 b
arm_stackdump: 20008b20: 00000000 00000000 009a1d1f ffffffe9 00000000 00000000 0
arm_stackdump: 20008b40: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008b60: 00000000 00000000 00000000 00000000 00000002 20008688 8
arm_stackdump: 20008b80: 00008000 0800a28f 0800a1f4 01000000 00000000 00000000 0
arm_stackdump: 20008ba0: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008bc0: 00000000 00000000 20008610 20008704 00000000 20000acc 4
arm_stackdump: 20008be0: 20008688 20008808 20000a7c 20001c70 20008804 20008cc8 0
arm_stackdump: 20008c00: 00000000 00000000 00000000 00000000 00000000 00000000 9
arm_stackdump: 20008c20: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008c40: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008c60: 000000e2 0800310d 00000072 14110800 00008000 08010d77 0
arm_stackdump: 20008c80: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008ca0: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008cc0: 00000000 200086d8 f002bb08 000000e2 14110800 00000000 7
arm_stackdump: 20008ce0: f002bb08 20008704 00000007 00000000 f0008f35 0800466b 0
arm_stackdump: 20008d00: 200086c0 080046cb 00000000 20008610 00000000 200086c0 5
arm_stackdump: 20008d20: 000000dc 0001ecf7 00000000 20008610 000000dc 0801557d 0
arm_stackdump: 20008d40: 200089a8 20008610 00000000 08008c69 00004e20 00000000 1
arm_stackdump: 20008d60: 00000000 200089a8 00000004 0801ed21 00000000 08003db7 0
arm_showtasks: PID PRI STACK COMMAND
arm_dump_task: 0 0 1000 Idle Task
arm_dump_task: 1 224 1992 hpwork 0x20000094
arm_dump_task: 2 100 1992 lpwork 0x200000a8
arm_dump_task: 3 100 2000 nsh_main
arm_dump_task: 4 100 2008 ostest
arm_dump_task: 5 100 8112 ostest Arg1 Arg2 Arg3 Arg4
arm_dump_task: 89 210 960 Task1
arm_dump_task: 90 220 960 Task2 � � � hD`������hD
```
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257195775
I use default. Unfortunately I do not have the same bord as you. I just got stm32f4 based board, but need to solder UART pins. Will test with that board on upcoming week.
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1264577524
The PR is here for quite a long time, so waiting one more week should not be a problem unless other reviewers want to speedup merging of a solution.
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1271755517
@pkarashchenko Finally got back and was able to test. Happy to report the test **passes** on tm4c129e-launchpad:ostest with priority inheritance enabled!
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r911326478
##########
sched/wqueue/kwork_inherit.c:
##########
@@ -66,91 +66,40 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
-#if CONFIG_SEM_NNESTPRIO > 0
+ /* REVISIT: Priority multi-boost is not supported */
+
+ DEBUGASSERT(wtcb->boost_priority == 0);
+
/* If the priority of the client thread that is greater than the base
* priority of the worker thread, then we may need to adjust the worker
* thread's priority now or later to that priority.
*/
if (reqprio > wtcb->base_priority)
{
- /* If the new priority is greater than the current, possibly already
- * boosted priority of the worker thread, then we will have to raise
- * the worker thread's priority now.
+ /* Save boost priority value as it might be needed in case of multiple
+ * re-prioritisations happen, then the priority of the thread can't go
+ * below the boost priority value until priority boost is canceled
*/
- if (reqprio > wtcb->sched_priority)
- {
- /* If the current priority of worker thread has already been
- * boosted, then add the boost priority to the list of restoration
- * priorities. When the higher priority waiter thread gets its
- * count, then we need to revert the worker thread to this saved
- * priority (not to its base priority).
- */
-
- if (wtcb->sched_priority > wtcb->base_priority)
- {
- /* Save the current, boosted priority of the worker thread. */
-
- if (wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO)
- {
- wtcb->pend_reprios[wtcb->npend_reprio] =
- wtcb->sched_priority;
- wtcb->npend_reprio++;
- }
- else
- {
- serr("ERROR: CONFIG_SEM_NNESTPRIO exceeded\n");
- DEBUGASSERT(wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO);
- }
- }
+ wtcb->boost_priority = reqprio;
- /* Raise the priority of the worker. This cannot cause a context
- * switch because we have preemption disabled. The worker thread
- * may be marked "pending" and the switch may occur during
- * sched_unblock() processing.
- */
+ /* If the priority of the client thread that is less than of equal to
+ * the priority of the worker thread, then do nothing because the
+ * thread is already running at a sufficient priority.
+ */
- nxsched_set_priority(wtcb, reqprio);
- }
- else
+ if (reqprio > wtcb->sched_priority)
{
- /* The new priority is above the base priority of the worker,
- * but not as high as its current working priority. Just put it
- * in the list of pending restoration priorities so that when the
- * higher priority thread gets its count, we can revert to this
- * saved priority and not to the base priority.
+ /* Raise the priority of the worker thread. This cannot cause
Review Comment:
reminder to self: need to fix the 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.
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256555403
Ok. Let me run ostest on samv7 based board. I ran it initially when submitted PR, but can be that things changed from that time. I will notify you when ostest with priority inheritance will pass on my end
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1257190633
No joy. :-(
This still gets a hardfault on `b-g474e-dpow1:ostest` (`CONFIG_PRIORITY_INHERITANCE=y` needs to be enabled manually).
Log:
```
user_main: priority inheritance test
priority_inheritance: Started
priority_inheritance: Starting lowpri_thread-1 (of 3) at 1
priority_inheritance: Set lowpri_thread-1 priority to 1
priority_inheritance: Starting lowpri_thread-2 (of 3) at 1
lowpri_thread-1: Started
priority_inheritance: Set lowpri_thread-2 priority to 1
lowpri_thread-1: initial priority: 1
priority_inheritance: Starting lowpri_thread-3 (of 3) at 1
lowpri_thread-2: Started
priority_inheritance: Set lowpri_thread-3 priority to 1
lowpri_thread-1: Waiting for the midle pri task to run
priority_inheritance: Waiting...
lowpri_thread-2: initial priority: 1
lowpri_thread-3: Started
g_middlestate: 0
lowpri_thread-2: Waiting for the midle pri task to run
lowpri_thread-3: initial priority: 1
g_highstate[0]: 0
g_middlestate: 0
lowpri_thread-3: Waiting for the midle pri task to run
I still have a count on the semaphore
g_highstate[0]: 0
g_middlestate: 0
I still have a count on the semaphore
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-1: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-2: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
lowpri_thread-3: Waiting for the midle pri task to run
g_middlestate: 0
g_highstate[0]: 0
I still have a count on the semaphore
priority_inheritance: Starting medpri_thread at 99
priority_inheritance: Set medpri_thread priority to 99
priority_inheritance: Waiting...
medpri_thread: Started ... I won't let go of the CPU!
priority_inheritance: Starting highpri_thread-1 (of 1) at 255
priority_inheritance: Set highpri_thread-1 priority to 255
highpri_thread-1: Started
priority_inheritance: Waiting for highpri_thread-1 to complete
highpri_thread-1: Calling sem_wait()
lowpri_thread-3: Sem count: -1, No. highpri thread: 1
highpri_thread-1: SUCCESS midpri_thread is still running!
highpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for medpri_thread to complete
medpri_thread: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-1 to complete
lowpri_thread-2: Sem count: 1, No. highpri thread: 0
lowpri_thread-2: SUCCESS the middle priority task has already exitted!
g_middlestate: 3 sem count=1
g_highstate[0]: 3
lowpri_thread-1: Sem count: 1, No. highpri thread: 0
lowpri_thread-3: SUCCESS priority before sem_post: 255
lowpri_thread-2: SUCCESS priority before sem_post: 1
lowpri_thread-1: SUCCESS the middle priority task has already exitted!
lowpri_thread-3: SUCCESS final priority: 1
lowpri_thread-2: SUCCESS final priority: 1
g_middlestate: 3 sem count=1
lowpri_thread-3: Okay... I'm done!
lowpri_thread-2: Okay... I'm done!
g_highstate[0]: 3
lowpri_thread-1: SUCCESS priority before sem_post: 1
lowpri_thread-1: SUCCESS final priority: 1
lowpri_thread-1: Okay... I'm done!
priority_inheritance: Waiting for lowpri_thread-2 to complete
priority_inheritance: Waiting for lowpri_thread-3 to complete
priority_inheritancpriority_inheritance: Waiting for Task-0 to complete
priority_inheritance: FF�]�� Started, waiting 0 uS to take count
priority_inheritance: Started, waiting 20000 uS to take count
priority_inheritance: Task0 priority was:200 is:220
priority_inheritance: � Posted
priority_inheritance: Task0 priority was:220 is:200
priority_inheritance: Waiting for Task-1 to complete
up_assert: Assertion failed at file:armv7-m/arm_hardfault.c line: 173 task: Tas2
arm_registerdump: R0: 000000e2 R1: 0800310d R2: 00000072 R3: 14110800
arm_registerdump: R4: 00000000 R5: 00000000 R6: 00000000 FP: 00000000
arm_registerdump: R8: 00000000 SB: 00000000 SL: 00000000 R11: 00000000
arm_registerdump: IP: 00008000 SP: 20008cc8 LR: 08010d77 PC: 080117e0
arm_registerdump: xPSR: 21000000 BASEPRI: 000000f0 CONTROL: 00000000
arm_registerdump: EXC_RETURN: ffffffe9
arm_dump_stack: User Stack:
arm_dump_stack: sp: 20008a88
arm_dump_stack: base: 200089d0
arm_dump_stack: size: 000003c0
arm_stackdump: 20008a80: 00000001 20008a90 20008a88 20008610 00000000 0800a1bd c
arm_stackdump: 20008aa0: 000000ad 08023db4 20008610 08007341 000000ad 08023db4 3
arm_stackdump: 20008ac0: 40004808 00000000 20008bf4 00000003 20008704 00008200 1
arm_stackdump: 20008ae0: 000000f0 40000000 080030a1 08002d1f 20008bf4 00000003 3
arm_stackdump: 20008b00: 00000000 08001875 00000080 08001849 00000000 00000003 b
arm_stackdump: 20008b20: 00000000 00000000 009a1d1f ffffffe9 00000000 00000000 0
arm_stackdump: 20008b40: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008b60: 00000000 00000000 00000000 00000000 00000002 20008688 8
arm_stackdump: 20008b80: 00008000 0800a28f 0800a1f4 01000000 00000000 00000000 0
arm_stackdump: 20008ba0: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008bc0: 00000000 00000000 20008610 20008704 00000000 20000acc 4
arm_stackdump: 20008be0: 20008688 20008808 20000a7c 20001c70 20008804 20008cc8 0
arm_stackdump: 20008c00: 00000000 00000000 00000000 00000000 00000000 00000000 9
arm_stackdump: 20008c20: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008c40: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008c60: 000000e2 0800310d 00000072 14110800 00008000 08010d77 0
arm_stackdump: 20008c80: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008ca0: 00000000 00000000 00000000 00000000 00000000 00000000 0
arm_stackdump: 20008cc0: 00000000 200086d8 f002bb08 000000e2 14110800 00000000 7
arm_stackdump: 20008ce0: f002bb08 20008704 00000007 00000000 f0008f35 0800466b 0
arm_stackdump: 20008d00: 200086c0 080046cb 00000000 20008610 00000000 200086c0 5
arm_stackdump: 20008d20: 000000dc 0001ecf7 00000000 20008610 000000dc 0801557d 0
arm_stackdump: 20008d40: 200089a8 20008610 00000000 08008c69 00004e20 00000000 1
arm_stackdump: 20008d60: 00000000 200089a8 00000004 0801ed21 00000000 08003db7 0
arm_showtasks: PID PRI STACK COMMAND
arm_dump_task: 0 0 1000 Idle Task
arm_dump_task: 1 224 1992 hpwork 0x20000094
arm_dump_task: 2 100 1992 lpwork 0x200000a8
arm_dump_task: 3 100 2000 nsh_main
arm_dump_task: 4 100 2008 ostest
arm_dump_task: 5 100 8112 ostest Arg1 Arg2 Arg3 Arg4
arm_dump_task: 89 210 960 Task1 �0
arm_dump_task: 90 220 960 Task2 � � � hD`������hD
```
Let me try with `tm4c129e-launchpad` also...
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1263758632
Finally I was managed to catch the crash with next config. I was able to identify that if `CONFIG_DEBUG_FULLOPT=y` the crash is not reproduced. @acassis @hartmannathan could you please check on your end?
[defconfig_stm32disco.txt](https://github.com/apache/incubator-nuttx/files/9686073/defconfig_stm32disco.txt)
--
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] masayuki2009 commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1264579341
>@masayuki2009 @davids5 maybe you have some boards and can run ostest with priority inheritance enabled?
The latest PR now works with `qemu-intel64:ostest`
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1262661720
@masayuki2009 @davids5 maybe you have some boards and can run ostest with priority inheritance enabled?
--
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] acassis commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1137199465
@pkarashchenko please include more details about the issue you are trying to fix
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256516952
@hartmannathan rebased
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256323384
> #7159 here is PR with rename
Doh! Thank you! I will test that right 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.
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] davids5 commented on a diff in pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r974494229
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +411,52 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
Review Comment:
This is getting the holder's priority that can be boosted or at base. The test is the next bit.
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r983864632
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +410,55 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
- if (htcb->sched_priority != htcb->base_priority)
+ if (htcb->sched_priority != hpriority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The holder thread's
- * "boosted" priority could greater than or equal to
- * "stcb->sched_priority" (it could be greater if its priority was
- * boosted because it also holds another semaphore).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
+ if (htcb->holdsem != NULL)
{
- /* The holder thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest priority that we have in
- * rpriority.
- */
+ FAR struct tcb_s *stcb;
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
let's rebase to the master
##########
sched/wqueue/kwork_inherit.c:
##########
@@ -174,127 +123,72 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
static void lpwork_restoreworker(pid_t wpid, uint8_t reqprio)
{
FAR struct tcb_s *wtcb;
-#if CONFIG_SEM_NNESTPRIO > 0
- uint8_t wpriority;
- int index;
- int selected;
-#endif
/* Get the TCB of the low priority worker thread from the process ID. */
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
+ /* REVISIT: Priority multi-boost is not supported */
+
+ DEBUGASSERT(wtcb->boost_priority == reqprio);
+
+ /* Clear the threat boost priority */
+
+ wtcb->boost_priority = 0;
+
/* Was the priority of the worker thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
if (wtcb->sched_priority != wtcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (wtcb->npend_reprio < 1)
- {
- /* No... the worker thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the client task (reqprio)
- *
- * That latter assumption may not be true if the client's priority
- * was also boosted so that it no longer matches the wtcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* wtcb->sched_priority == reqprio && */
- wtcb->npend_reprio == 0);
-
- /* Reset the worker's priority back to the base priority. */
+ uint8_t wpriority;
- nxsched_reprioritize(wtcb, wtcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The worker thread's
- * "boosted" priority could greater than or equal to "reqprio" (it
- * could be greater if its priority we boosted because it also holds
- * some semaphore).
+ /* We attempt to restore task priority to its base priority. If there
+ * is any task with the higher priority waiting for the semaphore
+ * held by wtcb then this value will be overwritten.
*/
- else if (wtcb->sched_priority <= reqprio)
- {
- /* The worker thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest pending priority.
- */
+ wpriority = wtcb->base_priority;
- /* Find the highest pending priority and remove it from the list */
-
- for (index = 1, selected = 0; index < wtcb->npend_reprio; index++)
- {
- if (wtcb->pend_reprios[index] > wtcb->pend_reprios[selected])
- {
- selected = index;
- }
- }
-
- /* Remove the highest priority pending priority from the list */
-
- wpriority = wtcb->pend_reprios[selected];
- index = wtcb->npend_reprio - 1;
- if (index > 0)
- {
- wtcb->pend_reprios[selected] = wtcb->pend_reprios[index];
- }
-
- wtcb->npend_reprio = index;
-
- /* And apply that priority to the thread (while retaining the
- * base_priority)
- */
+ /* Try to find the highest priority across all the tasks that are
+ * waiting for any semaphore held by wtcb. Skip search if htcb does
+ * not hold any semaphore.
+ */
- nxsched_set_priority(wtcb, wpriority);
- }
- else
+ if (wtcb->holdsem != NULL)
{
- /* The worker thread has been boosted to a higher priority than the
- * waiter task. The pending priority should be in the list (unless
- * it was lost because of list overflow or because the worker was
- * reprioritized again unbeknownst to the priority inheritance
- * logic).
- *
- * Search the list for the matching priority.
- */
+ FAR struct tcb_s *stcb;
- for (index = 0; index < wtcb->npend_reprio; index++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
ditto
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1263684641
@hartmannathan I just successfully ran ostest on stm32f4discovery board. Here is a `defconfig` that I used. Also run log is attached.
[defconfig.txt](https://github.com/apache/incubator-nuttx/files/9685530/defconfig.txt)
[stm32f4discovery.log](https://github.com/apache/incubator-nuttx/files/9685546/stm32f4discovery.log)
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1264062453
@hartmannathan @acassis here is a fix of the crash https://github.com/apache/incubator-nuttx-apps/pull/1333
Actually log from Alan helped as it was containing string `assert/lib_sta` that lead me to `libs/libc/assert/lib_stackchk.c` and `__stack_chk_fail`
--
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] hartmannathan commented on pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1264517958
> @hartmannathan @acassis here is a fix of the crash [apache/incubator-nuttx-apps#1333](https://github.com/apache/incubator-nuttx-apps/pull/1333)
> Actually log from Alan helped as it was containing string `assert/lib_sta` that lead me to `libs/libc/assert/lib_stackchk.c` and `__stack_chk_fail`
Kudos for catching that! I am currently traveling and expect to be back at the lab on Thursday or Friday. Unfortunately that means I will not be able to test it because I don't have real hardware, and only occasional connectivity. I don't know whether you'd like to wait that long to merge it; if you and @acassis feel comfortable with it now, then feel free to merge it and I'll test it anyway when I get back just to make sure...
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r884885991
##########
sched/semaphore/sem_holder.c:
##########
@@ -495,111 +417,30 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
if (htcb->sched_priority != htcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
+ rpriority = htcb->base_priority;
Review Comment:
@hartmannathan here the priority is restored. The task base priority is taken as a basis and if there is a task with higher priority waiting on the semaphore with priority inheritance enabled, then the priority of the higher priority task is taken. The loop below iterates all the tasks and picks the highest priority or if there is no higher priority, then task base_priority is applied (restored).
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r978966096
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +410,55 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
- if (htcb->sched_priority != htcb->base_priority)
+ if (htcb->sched_priority != hpriority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The holder thread's
- * "boosted" priority could greater than or equal to
- * "stcb->sched_priority" (it could be greater if its priority was
- * boosted because it also holds another semaphore).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
+ if (htcb->holdsem != NULL)
{
- /* The holder thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest priority that we have in
- * rpriority.
- */
+ FAR struct tcb_s *stcb;
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
+ stcb != NULL && stcb->sched_priority > hpriority;
+ stcb = stcb->flink)
{
- if (htcb->pend_reprios[i] > htcb->pend_reprios[j])
- {
- j = i;
- }
- }
+ FAR sem_t *sem = stcb->waitobj;
Review Comment:
fixed
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256319894
https://github.com/apache/incubator-nuttx/pull/7159 here is PR with rename
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1256318814
> > @hartmannathan do you have any test results?
>
> I'm not sure what happened here:
>
> ```
> 5126 git checkout master --force
> 5127 git pull
> 5128 wget https://github.com/apache/incubator-nuttx/pull/6318.patch
> 5129 git apply 6318.patch
> 5130 git status
> 5131 make distclean
> 5132 tools/configure.sh b-g474e-dpow1:nsh
> 5133 make menuconfig
> ```
>
> (Added `CONFIG_BUILTIN`, `CONFIG_NSH_BUILTIN_APPS`, and `CONFIG_TESTING_OSTEST`.)
>
> ```
> 5134 make savedefconfig
> 5135 make
> ```
>
> (Snip)
>
> ```
> CC: semaphore/sem_holder.c
> semaphore/sem_holder.c: In function 'nxsem_restoreholderprio':
> semaphore/sem_holder.c:439:26: error: 'struct tcb_s' has no member named 'waitsem'; did you mean 'waitdog'?
> if ((stcb->waitsem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
> ^~~~~~~
> waitdog
> semaphore/sem_holder.c:444:49: error: 'struct tcb_s' has no member named 'waitsem'; did you mean 'waitdog'?
> if (pholder->sem == stcb->waitsem &&
> ^~~~~~~
> waitdog
> make[1]: *** [Makefile:58: sem_holder.o] Error 1
> make[1]: Leaving directory '/home/nate/ramdrive/NuttX/nuttx/sched'
> make: *** [tools/LibTargets.mk:59: sched/libsched.a] Error 2
> ```
>
> This is strange because PR check above shows all checks passed.
>
> What configuration are you using to test with?
Yeah... Recent changes on mainline removed mqwait and renamed semwait to generic void wait obj. I will rebase the changes and fix compilation
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#issuecomment-1255878739
@hartmannathan do you have any test results?
--
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 #6318: sched/semaphore: fix priority boost restoration for priority inheritance
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r983891888
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +410,55 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
+ /* We attempt to restore thread priority to its base priority. If
+ * there is any thread with the higher priority waiting for the
+ * semaphore held by htcb then this value will be overwritten.
+ */
+
+ hpriority = htcb->boost_priority > htcb->base_priority ?
+ htcb->boost_priority : htcb->base_priority;
+
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
- if (htcb->sched_priority != htcb->base_priority)
+ if (htcb->sched_priority != hpriority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (htcb->npend_reprio < 1)
- {
- /* No... the holder thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the task that just got the semaphore
- * (stcb->sched_priority)
- *
- * That latter assumption may not be true if the stcb's priority
- * was also boosted so that it no longer matches the htcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The holder thread's
- * "boosted" priority could greater than or equal to
- * "stcb->sched_priority" (it could be greater if its priority was
- * boosted because it also holds another semaphore).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
+ if (htcb->holdsem != NULL)
{
- /* The holder thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest priority that we have in
- * rpriority.
- */
+ FAR struct tcb_s *stcb;
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
Done
##########
sched/wqueue/kwork_inherit.c:
##########
@@ -174,127 +123,72 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
static void lpwork_restoreworker(pid_t wpid, uint8_t reqprio)
{
FAR struct tcb_s *wtcb;
-#if CONFIG_SEM_NNESTPRIO > 0
- uint8_t wpriority;
- int index;
- int selected;
-#endif
/* Get the TCB of the low priority worker thread from the process ID. */
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
+ /* REVISIT: Priority multi-boost is not supported */
+
+ DEBUGASSERT(wtcb->boost_priority == reqprio);
+
+ /* Clear the threat boost priority */
+
+ wtcb->boost_priority = 0;
+
/* Was the priority of the worker thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
if (wtcb->sched_priority != wtcb->base_priority)
{
-#if CONFIG_SEM_NNESTPRIO > 0
- /* Are there other, pending priority levels to revert to? */
-
- if (wtcb->npend_reprio < 1)
- {
- /* No... the worker thread has only been boosted once.
- * npend_reprio should be 0 and the boosted priority should be the
- * priority of the client task (reqprio)
- *
- * That latter assumption may not be true if the client's priority
- * was also boosted so that it no longer matches the wtcb's
- * sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
- * that we do not have a proper record of the reprioritizations).
- */
-
- DEBUGASSERT(/* wtcb->sched_priority == reqprio && */
- wtcb->npend_reprio == 0);
-
- /* Reset the worker's priority back to the base priority. */
+ uint8_t wpriority;
- nxsched_reprioritize(wtcb, wtcb->base_priority);
- }
-
- /* There are multiple pending priority levels. The worker thread's
- * "boosted" priority could greater than or equal to "reqprio" (it
- * could be greater if its priority we boosted because it also holds
- * some semaphore).
+ /* We attempt to restore task priority to its base priority. If there
+ * is any task with the higher priority waiting for the semaphore
+ * held by wtcb then this value will be overwritten.
*/
- else if (wtcb->sched_priority <= reqprio)
- {
- /* The worker thread has been boosted to the same priority as the
- * waiter thread that just received the count. We will simply
- * reprioritize to the next highest pending priority.
- */
+ wpriority = wtcb->base_priority;
- /* Find the highest pending priority and remove it from the list */
-
- for (index = 1, selected = 0; index < wtcb->npend_reprio; index++)
- {
- if (wtcb->pend_reprios[index] > wtcb->pend_reprios[selected])
- {
- selected = index;
- }
- }
-
- /* Remove the highest priority pending priority from the list */
-
- wpriority = wtcb->pend_reprios[selected];
- index = wtcb->npend_reprio - 1;
- if (index > 0)
- {
- wtcb->pend_reprios[selected] = wtcb->pend_reprios[index];
- }
-
- wtcb->npend_reprio = index;
-
- /* And apply that priority to the thread (while retaining the
- * base_priority)
- */
+ /* Try to find the highest priority across all the tasks that are
+ * waiting for any semaphore held by wtcb. Skip search if htcb does
+ * not hold any semaphore.
+ */
- nxsched_set_priority(wtcb, wpriority);
- }
- else
+ if (wtcb->holdsem != NULL)
{
- /* The worker thread has been boosted to a higher priority than the
- * waiter task. The pending priority should be in the list (unless
- * it was lost because of list overflow or because the worker was
- * reprioritized again unbeknownst to the priority inheritance
- * logic).
- *
- * Search the list for the matching priority.
- */
+ FAR struct tcb_s *stcb;
- for (index = 0; index < wtcb->npend_reprio; index++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
Done
--
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