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/11/15 09:28:59 UTC
[GitHub] [incubator-nuttx] zyfeier opened a new pull request, #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
zyfeier opened a new pull request, #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599
## Summary
It takes about 10 cycles to obtain the task list according to the task status. In most cases, we know the task status, so we can directly delete the task from the specified task list to reduce time consuming.
## Impact
Performance:
sem_post -> sem_wait (cycle count):
before:
`171`
after:
`161`
## Testing
sabre-6quad:nsh 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] zyfeier commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
zyfeier commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1322968945
> @xiaoxiang781216 It seems working but I'd like to try the latest PR again. So please fix the conflicts.
@masayuki2009 conflicts was fixed, please check again.
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1026353019
##########
arch/arm64/src/common/arm64_unblocktask.c:
##########
@@ -43,102 +43,70 @@
* Name: up_unblock_task
*
* Description:
- * A task is currently in an inactive task list
- * but has been prepped to execute. Move the TCB to the
- * ready-to-run list, restore its context, and start execution.
+ * A task is currently in the ready-to-run list but has been prepped
+ * to execute. Restore its context, and start execution.
*
* Input Parameters:
- * tcb: Refers to the tcb to be unblocked. This tcb is
- * in one of the waiting tasks lists. It must be moved to
- * the ready-to-run list and, if it is the highest priority
- * ready to run task, executed.
+ * tcb: Refers to the head task of the ready-to-run list
+ * which will be executed.
+ * rtcb: Refers to the running task which will be blocked.
*
****************************************************************************/
-void up_unblock_task(struct tcb_s *tcb)
+void up_unblock_task(struct tcb_s *tcb, struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_BLOCKED_STATE) &&
- (tcb->task_state <= LAST_BLOCKED_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the task from the blocked task list */
-
- nxsched_remove_blocked(tcb);
-
- /* Add the task in the correct location in the prioritized
- * ready-to-run task list
- */
-
- if (nxsched_add_readytorun(tcb))
+ if (CURRENT_REGS)
{
- /* The currently active task has changed! We need to do
- * a context switch to the new task.
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
*/
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
-
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
-
- arm64_savestate(rtcb->xcp.regs);
-
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
-
- rtcb = this_task();
+ arm64_savestate(rtcb->xcp.regs);
- /* Update scheduler parameters */
-
- nxsched_resume_scheduler(rtcb);
+ /* Update scheduler parameters */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
+ nxsched_resume_scheduler(tcb);
- arm64_restorestate(rtcb->xcp.regs);
- }
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* No, then we will need to perform the user context switch */
+ arm64_restorestate(tcb->xcp.regs);
+ }
- else
- {
- struct tcb_s *nexttcb = this_task();
+ /* No, then we will need to perform the user context switch */
+ else
+ {
#ifdef CONFIG_ARCH_ADDRENV
- /* Make sure that the address environment for the previously
- * running task is closed down gracefully (data caches dump,
- * MMU flushed) and set up the address environment for the new
- * thread at the head of the ready-to-run list.
- */
+ /* Make sure that the address environment for the previously
+ * running task is closed down gracefully (data caches dump,
+ * MMU flushed) and set up the address environment for the new
+ * thread at the head of the ready-to-run list.
+ */
- group_addrenv(nexttcb);
+ group_addrenv(tcb);
Review Comment:
I'm wondering, why do we have this for ARM64, but not for ARM? Shouldn't this be executed for all archs?
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1025802011
##########
arch/xtensa/src/common/xtensa_blocktask.c:
##########
@@ -45,108 +45,69 @@
* Name: up_block_task
*
* Description:
- * The currently executing task at the head of the ready to run list must
- * be stopped. Save its context and move it to the inactive list
- * specified by task_state.
+ * The currently executing task has already removed from ready-to-run list.
+ * Save its context and switch to the next running task at the head of the
+ * ready-to-run list.
*
* Input Parameters:
- * tcb: Refers to a task in the ready-to-run list (normally the task at
- * the head of the list). It must be stopped, its context saved and
- * moved into one of the waiting task lists. If it was the task at the
- * head of the ready-to-run list, then a context switch to the new
- * ready to run task must be performed.
- * task_state: Specifies which waiting task list should hold the blocked
- * task TCB.
+ * rtcb: Reference to the running task which is different to the
+ * task (next running task) at the head of the list.
*
****************************************************************************/
-void up_block_task(struct tcb_s *tcb, tstate_t task_state)
+void up_block_task(struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
- bool switch_needed;
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_READY_TO_RUN_STATE) &&
- (tcb->task_state <= LAST_READY_TO_RUN_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the tcb task from the ready-to-run list. If we are blocking the
- * task at the head of the task list (the most likely case), then a
- * context switch to the next ready-to-run task is needed. In this case,
- * it should also be true that rtcb == tcb.
- */
-
- switch_needed = nxsched_remove_readytorun(tcb);
-
- /* Add the task to the specified blocked task list */
-
- nxsched_add_blocked(tcb, (tstate_t)task_state);
-
- /* If there are any pending tasks, then add them to the ready-to-run
- * task list now
- */
-
- if (g_pendingtasks.head)
- {
- switch_needed |= nxsched_merge_pending();
- }
-
- /* Now, perform the context switch if one is needed */
-
- if (switch_needed)
+ if (CURRENT_REGS)
{
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
+ */
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
+ xtensa_savestate(rtcb->xcp.regs);
- xtensa_savestate(rtcb->xcp.regs);
+ /* Restore the exception context of the rtcb at the (new) head
+ * of the ready-to-run task list.
+ */
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
+ rtcb = this_task();
- rtcb = this_task();
+ /* Reset scheduler parameters */
- /* Reset scheduler parameters */
+ nxsched_resume_scheduler(rtcb);
- nxsched_resume_scheduler(rtcb);
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
-
- xtensa_restorestate(rtcb->xcp.regs);
- }
+ xtensa_restorestate(rtcb->xcp.regs);
+ }
- /* No, then we will need to perform the user context switch */
+ /* No, then we will need to perform the user context switch */
- else
- {
- struct tcb_s *nexttcb = this_task();
+ else
+ {
+ struct tcb_s *nexttcb = this_task();
- /* Switch context to the context of the task at the head of the
- * ready to run list.
- */
+ /* Switch context to the context of the task at the head of the
+ * ready to run list.
+ */
- nxsched_resume_scheduler(nexttcb);
+ nxsched_resume_scheduler(nexttcb);
Review Comment:
Do we need
```
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously
* running task is closed down gracefully (data caches dump,
* MMU flushed) and set up the address environment for the new
* thread at the head of the ready-to-run list.
*/
group_addrenv(nexttcb);
#endif
```
before calling `nxsched_resume_scheduler`?
--
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 pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1322278197
@masayuki2009 do you have the result?
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1319447437
@zyfeier @xiaoxiang781216
Let me try to test some SMP use cases.
--
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 pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1318109996
@masayuki2009 @pkarashchenko this patchset try to extract the common operation from arch to sched which:
1. Avoid the same logic duplicate in each arch
2. Improve 10% sched performance
After the change, only when the current task need switch out, scheduler will call into arch 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] pkarashchenko commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1318782050
Please let me take a look. I will try to book some time slot in my calendar till Saturday
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1317797613
```
====================================================================================
Configuration/Tool: ea3131/pgnsh,CONFIG_ARM_TOOLCHAIN_GNU_EABI
------------------------------------------------------------------------------------
Cleaning...
Configuring...
Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
Building NuttX...
arm-none-eabi-ld: /github/workspace/sources/nuttx/nuttx section `.locked' will not fit in region `locked'
arm-none-eabi-ld: section .paged LMA [0000000011035000,0000000011042db7] overlaps section .locked LMA [0000000011029080,00000000110350fb]
arm-none-eabi-ld: section .ARM.exidx LMA [00000000110350fc,0000000011035103] overlaps section .paged LMA [0000000011035000,0000000011042db7]
arm-none-eabi-ld: region `locked' overflowed by 524 bytes
make[1]: *** [Makefile:174: nuttx] Error 1
make: *** [tools/Unix.mk:506: nuttx] Error 2
make: Target 'all' not remade because of 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] pkarashchenko commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1317336984
I need some more time to review 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] masayuki2009 commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1323162166
@zyfeier @xiaoxiang781216
I've just confirmed that all the tests have passed.
Let me merge this PR.
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1319299988
@xiaoxiang781216 I'm comparing platform implementations of `up_reprioritize_rtr` and observe many differences. Are you sure that new generalised code is fitting all the supported archs?
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1027104724
##########
arch/arm64/src/common/arm64_unblocktask.c:
##########
@@ -43,102 +43,70 @@
* Name: up_unblock_task
*
* Description:
- * A task is currently in an inactive task list
- * but has been prepped to execute. Move the TCB to the
- * ready-to-run list, restore its context, and start execution.
+ * A task is currently in the ready-to-run list but has been prepped
+ * to execute. Restore its context, and start execution.
*
* Input Parameters:
- * tcb: Refers to the tcb to be unblocked. This tcb is
- * in one of the waiting tasks lists. It must be moved to
- * the ready-to-run list and, if it is the highest priority
- * ready to run task, executed.
+ * tcb: Refers to the head task of the ready-to-run list
+ * which will be executed.
+ * rtcb: Refers to the running task which will be blocked.
*
****************************************************************************/
-void up_unblock_task(struct tcb_s *tcb)
+void up_unblock_task(struct tcb_s *tcb, struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_BLOCKED_STATE) &&
- (tcb->task_state <= LAST_BLOCKED_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the task from the blocked task list */
-
- nxsched_remove_blocked(tcb);
-
- /* Add the task in the correct location in the prioritized
- * ready-to-run task list
- */
-
- if (nxsched_add_readytorun(tcb))
+ if (CURRENT_REGS)
{
- /* The currently active task has changed! We need to do
- * a context switch to the new task.
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
*/
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
-
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
-
- arm64_savestate(rtcb->xcp.regs);
-
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
-
- rtcb = this_task();
+ arm64_savestate(rtcb->xcp.regs);
- /* Update scheduler parameters */
-
- nxsched_resume_scheduler(rtcb);
+ /* Update scheduler parameters */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
+ nxsched_resume_scheduler(tcb);
- arm64_restorestate(rtcb->xcp.regs);
- }
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* No, then we will need to perform the user context switch */
+ arm64_restorestate(tcb->xcp.regs);
+ }
- else
- {
- struct tcb_s *nexttcb = this_task();
+ /* No, then we will need to perform the user context switch */
+ else
+ {
#ifdef CONFIG_ARCH_ADDRENV
- /* Make sure that the address environment for the previously
- * running task is closed down gracefully (data caches dump,
- * MMU flushed) and set up the address environment for the new
- * thread at the head of the ready-to-run list.
- */
+ /* Make sure that the address environment for the previously
+ * running task is closed down gracefully (data caches dump,
+ * MMU flushed) and set up the address environment for the new
+ * thread at the head of the ready-to-run list.
+ */
- group_addrenv(nexttcb);
+ group_addrenv(tcb);
Review Comment:
But ARM64 also have it at the exactly same place in `arm64_doirq`. So why calling in unblocktask is also needed?
--
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 pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1320929086
> I do not have anything against this change. Only need to clarify `group_addrenv()` usage in general, but if overall behaviour is kept as before and change is well tested then `group_addrenv()` can be clarified separately from this PR
arm64 is fixed here: https://github.com/apache/incubator-nuttx/pull/7623. Other place need do group_addrenv in up_xxx since the rest do the context switch directly without syscall.
--
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 merged pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
masayuki2009 merged PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1315928005
```
====================================================================================
Configuration/Tool: stm32f103-minimum/sensors,CONFIG_ARM_TOOLCHAIN_GNU_EABI
------------------------------------------------------------------------------------
Cleaning...
Configuring...
Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
Building NuttX...
arm-none-eabi-ld: /github/workspace/sources/nuttx/nuttx section `.data' will not fit in region `flash'
arm-none-eabi-ld: region `flash' overflowed by 280 bytes
make[1]: *** [Makefile:174: nuttx] Error 1
make: *** [tools/Unix.mk:516: nuttx] Error 2
```
--
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] zyfeier commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
zyfeier commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1319442921
> Here is one example: <img alt="Screenshot 2022-11-18 at 00 39 34" width="1576" src="https://user-images.githubusercontent.com/17704713/202575484-495d9c23-898b-4486-bdeb-b075e8591a08.png">
@pkarashchenko up_reprioritize_rtr function was replaced by nxsched_reprioritize_rtr and up_block_task. nxsched_reprioritize_rtr function is sched related process, and up_block_task is arch special process.
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1026183408
##########
arch/xtensa/src/common/xtensa_blocktask.c:
##########
@@ -45,108 +45,69 @@
* Name: up_block_task
*
* Description:
- * The currently executing task at the head of the ready to run list must
- * be stopped. Save its context and move it to the inactive list
- * specified by task_state.
+ * The currently executing task has already removed from ready-to-run list.
+ * Save its context and switch to the next running task at the head of the
+ * ready-to-run list.
*
* Input Parameters:
- * tcb: Refers to a task in the ready-to-run list (normally the task at
- * the head of the list). It must be stopped, its context saved and
- * moved into one of the waiting task lists. If it was the task at the
- * head of the ready-to-run list, then a context switch to the new
- * ready to run task must be performed.
- * task_state: Specifies which waiting task list should hold the blocked
- * task TCB.
+ * rtcb: Reference to the running task which is different to the
+ * task (next running task) at the head of the list.
*
****************************************************************************/
-void up_block_task(struct tcb_s *tcb, tstate_t task_state)
+void up_block_task(struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
- bool switch_needed;
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_READY_TO_RUN_STATE) &&
- (tcb->task_state <= LAST_READY_TO_RUN_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the tcb task from the ready-to-run list. If we are blocking the
- * task at the head of the task list (the most likely case), then a
- * context switch to the next ready-to-run task is needed. In this case,
- * it should also be true that rtcb == tcb.
- */
-
- switch_needed = nxsched_remove_readytorun(tcb);
-
- /* Add the task to the specified blocked task list */
-
- nxsched_add_blocked(tcb, (tstate_t)task_state);
-
- /* If there are any pending tasks, then add them to the ready-to-run
- * task list now
- */
-
- if (g_pendingtasks.head)
- {
- switch_needed |= nxsched_merge_pending();
- }
-
- /* Now, perform the context switch if one is needed */
-
- if (switch_needed)
+ if (CURRENT_REGS)
{
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
+ */
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
+ xtensa_savestate(rtcb->xcp.regs);
- xtensa_savestate(rtcb->xcp.regs);
+ /* Restore the exception context of the rtcb at the (new) head
+ * of the ready-to-run task list.
+ */
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
+ rtcb = this_task();
- rtcb = this_task();
+ /* Reset scheduler parameters */
- /* Reset scheduler parameters */
+ nxsched_resume_scheduler(rtcb);
- nxsched_resume_scheduler(rtcb);
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
-
- xtensa_restorestate(rtcb->xcp.regs);
- }
+ xtensa_restorestate(rtcb->xcp.regs);
+ }
- /* No, then we will need to perform the user context switch */
+ /* No, then we will need to perform the user context switch */
- else
- {
- struct tcb_s *nexttcb = this_task();
+ else
+ {
+ struct tcb_s *nexttcb = this_task();
- /* Switch context to the context of the task at the head of the
- * ready to run list.
- */
+ /* Switch context to the context of the task at the head of the
+ * ready to run list.
+ */
- nxsched_resume_scheduler(nexttcb);
+ nxsched_resume_scheduler(nexttcb);
Review Comment:
I guess xtensa doesn't support kernel mode yet. And the origin code doens't invoke group_addrenv, it's better to not add group_addrenv in this patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1026490675
##########
arch/arm64/src/common/arm64_unblocktask.c:
##########
@@ -43,102 +43,70 @@
* Name: up_unblock_task
*
* Description:
- * A task is currently in an inactive task list
- * but has been prepped to execute. Move the TCB to the
- * ready-to-run list, restore its context, and start execution.
+ * A task is currently in the ready-to-run list but has been prepped
+ * to execute. Restore its context, and start execution.
*
* Input Parameters:
- * tcb: Refers to the tcb to be unblocked. This tcb is
- * in one of the waiting tasks lists. It must be moved to
- * the ready-to-run list and, if it is the highest priority
- * ready to run task, executed.
+ * tcb: Refers to the head task of the ready-to-run list
+ * which will be executed.
+ * rtcb: Refers to the running task which will be blocked.
*
****************************************************************************/
-void up_unblock_task(struct tcb_s *tcb)
+void up_unblock_task(struct tcb_s *tcb, struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_BLOCKED_STATE) &&
- (tcb->task_state <= LAST_BLOCKED_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the task from the blocked task list */
-
- nxsched_remove_blocked(tcb);
-
- /* Add the task in the correct location in the prioritized
- * ready-to-run task list
- */
-
- if (nxsched_add_readytorun(tcb))
+ if (CURRENT_REGS)
{
- /* The currently active task has changed! We need to do
- * a context switch to the new task.
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
*/
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
-
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
-
- arm64_savestate(rtcb->xcp.regs);
-
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
-
- rtcb = this_task();
+ arm64_savestate(rtcb->xcp.regs);
- /* Update scheduler parameters */
-
- nxsched_resume_scheduler(rtcb);
+ /* Update scheduler parameters */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
+ nxsched_resume_scheduler(tcb);
- arm64_restorestate(rtcb->xcp.regs);
- }
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* No, then we will need to perform the user context switch */
+ arm64_restorestate(tcb->xcp.regs);
+ }
- else
- {
- struct tcb_s *nexttcb = this_task();
+ /* No, then we will need to perform the user context switch */
+ else
+ {
#ifdef CONFIG_ARCH_ADDRENV
- /* Make sure that the address environment for the previously
- * running task is closed down gracefully (data caches dump,
- * MMU flushed) and set up the address environment for the new
- * thread at the head of the ready-to-run list.
- */
+ /* Make sure that the address environment for the previously
+ * running task is closed down gracefully (data caches dump,
+ * MMU flushed) and set up the address environment for the new
+ * thread at the head of the ready-to-run list.
+ */
- group_addrenv(nexttcb);
+ group_addrenv(tcb);
Review Comment:
arm do this in the common place, e.g.:
https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/armv7-a/arm_doirq.c#L80-L90
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#discussion_r1027118504
##########
arch/arm64/src/common/arm64_unblocktask.c:
##########
@@ -43,102 +43,70 @@
* Name: up_unblock_task
*
* Description:
- * A task is currently in an inactive task list
- * but has been prepped to execute. Move the TCB to the
- * ready-to-run list, restore its context, and start execution.
+ * A task is currently in the ready-to-run list but has been prepped
+ * to execute. Restore its context, and start execution.
*
* Input Parameters:
- * tcb: Refers to the tcb to be unblocked. This tcb is
- * in one of the waiting tasks lists. It must be moved to
- * the ready-to-run list and, if it is the highest priority
- * ready to run task, executed.
+ * tcb: Refers to the head task of the ready-to-run list
+ * which will be executed.
+ * rtcb: Refers to the running task which will be blocked.
*
****************************************************************************/
-void up_unblock_task(struct tcb_s *tcb)
+void up_unblock_task(struct tcb_s *tcb, struct tcb_s *rtcb)
{
- struct tcb_s *rtcb = this_task();
+ /* Update scheduler parameters */
- /* Verify that the context switch can be performed */
+ nxsched_suspend_scheduler(rtcb);
- DEBUGASSERT((tcb->task_state >= FIRST_BLOCKED_STATE) &&
- (tcb->task_state <= LAST_BLOCKED_STATE));
+ /* Are we in an interrupt handler? */
- /* Remove the task from the blocked task list */
-
- nxsched_remove_blocked(tcb);
-
- /* Add the task in the correct location in the prioritized
- * ready-to-run task list
- */
-
- if (nxsched_add_readytorun(tcb))
+ if (CURRENT_REGS)
{
- /* The currently active task has changed! We need to do
- * a context switch to the new task.
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
*/
- /* Update scheduler parameters */
-
- nxsched_suspend_scheduler(rtcb);
-
- /* Are we in an interrupt handler? */
-
- if (CURRENT_REGS)
- {
- /* Yes, then we have to do things differently.
- * Just copy the CURRENT_REGS into the OLD rtcb.
- */
-
- arm64_savestate(rtcb->xcp.regs);
-
- /* Restore the exception context of the rtcb at the (new) head
- * of the ready-to-run task list.
- */
-
- rtcb = this_task();
+ arm64_savestate(rtcb->xcp.regs);
- /* Update scheduler parameters */
-
- nxsched_resume_scheduler(rtcb);
+ /* Update scheduler parameters */
- /* Then switch contexts. Any necessary address environment
- * changes will be made when the interrupt returns.
- */
+ nxsched_resume_scheduler(tcb);
- arm64_restorestate(rtcb->xcp.regs);
- }
+ /* Then switch contexts. Any necessary address environment
+ * changes will be made when the interrupt returns.
+ */
- /* No, then we will need to perform the user context switch */
+ arm64_restorestate(tcb->xcp.regs);
+ }
- else
- {
- struct tcb_s *nexttcb = this_task();
+ /* No, then we will need to perform the user context switch */
+ else
+ {
#ifdef CONFIG_ARCH_ADDRENV
- /* Make sure that the address environment for the previously
- * running task is closed down gracefully (data caches dump,
- * MMU flushed) and set up the address environment for the new
- * thread at the head of the ready-to-run list.
- */
+ /* Make sure that the address environment for the previously
+ * running task is closed down gracefully (data caches dump,
+ * MMU flushed) and set up the address environment for the new
+ * thread at the head of the ready-to-run list.
+ */
- group_addrenv(nexttcb);
+ group_addrenv(tcb);
Review Comment:
Sorry, I point to the wrong place. Please compare arm64_syscall_switch and arm_syscall for armv7-a, you can find that group_addrenv miss in arm64_syscall_switch.
--
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 pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1320934899
> I do not have anything against this change. Only need to clarify `group_addrenv()` usage in general, but if overall behaviour is kept as before and change is well tested then `group_addrenv()` can be clarified separately from this PR
Ok, let's wait @masayuki2009 test result.
--
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 #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1319301932
Here is one example:
<img width="1576" alt="Screenshot 2022-11-18 at 00 39 34" src="https://user-images.githubusercontent.com/17704713/202575484-495d9c23-898b-4486-bdeb-b075e8591a08.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #7599: nuttx/sched: remove nxsched_remove_blocked from up_unblock_task
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7599:
URL: https://github.com/apache/incubator-nuttx/pull/7599#issuecomment-1322781435
@xiaoxiang781216
It seems working but I'd like to try the latest PR again.
So please fix the conflicts.
--
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