Skip to content

sched/sched: add nxsched_wakeup(), introduce TSTATE_SLEEPING and improve nxsched_ticksleep()#17222

Merged
anchao merged 2 commits into
apache:masterfrom
wangchdo:sched_sleeping_improve
Oct 28, 2025
Merged

sched/sched: add nxsched_wakeup(), introduce TSTATE_SLEEPING and improve nxsched_ticksleep()#17222
anchao merged 2 commits into
apache:masterfrom
wangchdo:sched_sleeping_improve

Conversation

@wangchdo
Copy link
Copy Markdown
Contributor

@wangchdo wangchdo commented Oct 21, 2025

This PR includes two commits:

commit 1:

    Add a new function nxsched_wakeup() to the scheduler,
    which allows waking up a sleeping task before its timeout.

commit 2:

    Refactor nxsched_timeout() to use nxsched_wakeup() in order to
    eliminate code duplication and improve maintainability.

Summary

Improved implementation of nxsched_ticksleep() and introduced new function nxsched_wakeup()

Impact

Improvement of newly added function nxsched_ticksleep() and introduced a new function nxsched_wakeup()

No impact to other nuttx functions

Testing

PR 17204 has already replace all Signal-based sleep implement to Scheduled sleep, So CI will verify this PR's improvement of nxsched_ticksleep() is OK

also,

on the one hand:
sleep related api test were added on sim/nsh, sabre-6quad/smp: nxsched_sleep() / nxsched_usleep() / nxsched_msleep() / nxsched_ticksleep()

on the other hand:
sched/event implementation has been modified to use sleep/wakeup pair, and the ostest including event test cases passed on board a2g-tc397-5v-tft:

image

@github-actions github-actions Bot added Area: OS Components OS Components issues Size: M The size of the change in this PR is medium labels Oct 21, 2025
@linguini1
Copy link
Copy Markdown
Contributor

I think this needs to be tested with more than just CI. It is a code change that needs runtime testing, not just building.

Also, why would we want to wake up a sleeping task before its timeout? Maybe I'm unfamiliar with the semantics of sleeping, but isn't it a requirement that the caller is never woken up prior to the timeout? Only on or after?

@linguini1 linguini1 requested a review from acassis October 21, 2025 13:18
@wangchdo
Copy link
Copy Markdown
Contributor Author

wangchdo commented Oct 21, 2025

I think this needs to be tested with more than just CI. It is a code change that needs runtime testing, not just building.

Also, why would we want to wake up a sleeping task before its timeout? Maybe I'm unfamiliar with the semantics of sleeping, but isn't it a requirement that the caller is never woken up prior to the timeout? Only on or after?

Sleep and wakeup sometimes should be in pair in terms of syncronization

For example task1 can not move on because some condition is not met and choose to sleep for a while in the simplest case, but before timeout task2 which has lower priority than task1 met the condition, then it can wakeup task1 to let task1 preempt it

sleep/wakeup is simpler and lighter than mutex, semaphore or event in this case

@wangchdo
Copy link
Copy Markdown
Contributor Author

I think this needs to be tested with more than just CI. It is a code change that needs runtime testing, not just building.

Also, why would we want to wake up a sleeping task before its timeout? Maybe I'm unfamiliar with the semantics of sleeping, but isn't it a requirement that the caller is never woken up prior to the timeout? Only on or after?

I will add test it and put the results in this PR later

@linguini1
Copy link
Copy Markdown
Contributor

linguini1 commented Oct 21, 2025

Sleep and wakeup sometimes should be in pair in terms of syncronization

For example task1 can not move on because some condition is not met and choose to sleep for a while in the simplest case, but before timeout task2 which has lower priority than task1 met the condition, then it can wakeup task1 to let task1 preempt it

sleep/wakeup is simpler and lighter than mutex, semaphore or event in this case

But this isn't what sleeping is for? If you want to wait for a condition, you block on a semaphore/mutex, not sleep (unless it's some hardware polling condition that requires sleeping).

I personally don't support the addition of the wakeup function since now all the up_delay have been replaced with this new implementation, and up_delay would never return before the timeout. We're changing the behavior all over the code. This is a sleep function, not for waiting on a condition, just sleeping. I don't see a use case this is needed either, just use a semaphore.

@linguini1
Copy link
Copy Markdown
Contributor

I personally don't support the addition of the wakeup function since now all the up_delay have been replaced with this new implementation, and up_delay would never return before the timeout. We're changing the behavior all over the code. This is a sleep function, not for waiting on a condition, just sleeping. I don't see a use case this is needed either, just use a semaphore.

I see now that even the previous implementation of nxsched_sleep could wake up the task early since it was implemented as waiting for a signal. So I do agree in that sense that this PR wouldn't necessarily change that behaviour. Although, I still think it shouldn't be possible to wake up a sleeping task before its timeout since that behaviour is not intuitive and doesn't seem correct for a delay/sleep function.

@wangchdo
Copy link
Copy Markdown
Contributor Author

Sleep and wakeup sometimes should be in pair in terms of syncronization
For example task1 can not move on because some condition is not met and choose to sleep for a while in the simplest case, but before timeout task2 which has lower priority than task1 met the condition, then it can wakeup task1 to let task1 preempt it
sleep/wakeup is simpler and lighter than mutex, semaphore or event in this case

But this isn't what sleeping is for? If you want to wait for a condition, you block on a semaphore/mutex, not sleep (unless it's some hardware pulling condition that requires sleeping).

I personally don't support the addition of the wakeup function since now all the up_delay have been replaced with this new implementation, and up_delay would never return before the timeout. We're changing the behavior all over the code. This is a sleep function, not for waiting on a condition, just sleeping. I don't see a use case this is needed either, just use a semaphore.

HI @linguini1

I used this sleep/wakeup pair to improve event implementation in #17223
could you please help to review?

@wangchdo wangchdo force-pushed the sched_sleeping_improve branch from c24f03d to aae81a3 Compare October 22, 2025 01:19
Comment thread include/nuttx/sched.h
Comment thread sched/sched/sched_wakeup.c Outdated
@wangchdo wangchdo force-pushed the sched_sleeping_improve branch from aae81a3 to 8b77e97 Compare October 22, 2025 14:21
@wangchdo wangchdo requested a review from anchao October 22, 2025 15:23
anchao
anchao previously approved these changes Oct 22, 2025
Comment thread sched/sched/sched_sleep.c Outdated
Comment thread include/nuttx/sched.h Outdated
@wangchdo wangchdo force-pushed the sched_sleeping_improve branch from 17edd89 to 45a7fb2 Compare October 26, 2025 00:23
@wangchdo wangchdo force-pushed the sched_sleeping_improve branch 2 times, most recently from c972d68 to a2a4835 Compare October 26, 2025 00:59
Comment thread include/nuttx/sched.h Outdated
Comment thread sched/init/nx_start.c Outdated
    Add a new function nxsched_wakeup() to the scheduler,
    which allows waking up a sleeping task before its timeout.

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
    Refactor nxsched_timeout() to use nxsched_wakeup() in order to
    eliminate code duplication and improve maintainability.

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
@wangchdo wangchdo force-pushed the sched_sleeping_improve branch from a2a4835 to b61e676 Compare October 27, 2025 01:18
@anchao anchao merged commit ace17ad into apache:master Oct 28, 2025
40 checks passed
@wangchdo wangchdo deleted the sched_sleeping_improve branch October 29, 2025 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components issues Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants