Skip to content

[!] sched/event: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT#17244

Merged
acassis merged 7 commits into
apache:masterfrom
wangchdo:improve_event_new_1024
Oct 31, 2025
Merged

[!] sched/event: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT#17244
acassis merged 7 commits into
apache:masterfrom
wangchdo:improve_event_new_1024

Conversation

@wangchdo
Copy link
Copy Markdown
Contributor

@wangchdo wangchdo commented Oct 24, 2025

Summary

This PR contains breaking changes for event implementation to improve its performance, safety and make its api cleaner and easier to use

Change summary:

Refactors the event module by removing its dependency on semaphores and separate wait objects, and introduces a new task state TSTATE_WAIT_EVENT to simplify scheduling and improve maintainability.

Details:

1. Remove semaphore dependency
Reason:
- Semaphore objects consume more memory than necessary for event synchronization.
- Semaphore interfaces are relatively complex, involving global locks and logic that exceeds
the needs of the event mechanism.
Benefit:
- Simplifies the event module and reduces runtime and memory overhead.
2. Remove wait object dependency
Reason:
- Wait objects introduce additional memory usage.
- The current design either uses a local wait object in the waiting task (which is unsafe because the posting task also accesses it) or requires users to define global wait objects and call event_tickwait_wait(). This leads to complicated and error-prone usage.
- By removing wait objects, the event module can be implemented more cleanly.
Benefit:
- Simplifies API usage.
- Improves safety and code maintainability.

3. Introduce TSTATE_WAIT_EVENT and move the scheduling list to the event object
Reason:
- Makes the event module implementation more concise.
- Allows the scheduler to handle tasks blocked on events more flexibly in special cases (e.g., task deletion).
Benefit:
- Improves modularity and better integrates event handling with the scheduler.

Impact

This change streamlines the event mechanism by eliminating unnecessary abstractions, reducing memory footprint, and improving the clarity of both the implementation and external API, no impact to other nuttx functions

Testing

ostest including nxevent test cases passed on board arm/fvp-armv8r-aarch32
ostest.log

ostest include nxevent test cases passed on board tricore/a2g-tc397-5v-tft
tricore_a2g-tc397-5v-tft_ostest.log

ostest include nxevent test cases passed on board riscv/rv-virt board
riscv_rv-virt_ostest.log

@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 24, 2025
@linguini1 linguini1 marked this pull request as draft October 24, 2025 15:10
@linguini1
Copy link
Copy Markdown
Contributor

Just converting to draft while test logs are pending.

@wangchdo wangchdo marked this pull request as ready for review October 24, 2025 15:12
@wangchdo wangchdo marked this pull request as draft October 24, 2025 15:12
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from 611f0a4 to 5f775d0 Compare October 25, 2025 07:40
@wangchdo wangchdo marked this pull request as ready for review October 25, 2025 07:48
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch 2 times, most recently from 57cbed7 to b498f94 Compare October 25, 2025 10:06
@wangchdo wangchdo marked this pull request as draft October 25, 2025 10:06
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from b498f94 to 7d941c5 Compare October 25, 2025 10:52
@wangchdo wangchdo marked this pull request as ready for review October 25, 2025 10:56
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from 7d941c5 to 55fdb23 Compare October 25, 2025 10:58
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch 2 times, most recently from 406dd90 to 4b2ef05 Compare October 26, 2025 01:09
@github-actions github-actions Bot added the Size: L The size of the change in this PR is large label Oct 26, 2025
@wangchdo
Copy link
Copy Markdown
Contributor Author

wangchdo commented Oct 27, 2025

@acassis I’ve uploaded the test logs for the ARM, RISC-V, and TriCore architectures in the PR description. Please check and let me know if any additional testing is needed.

acassis
acassis previously approved these changes Oct 27, 2025
@xiaoxiang781216 xiaoxiang781216 added the breaking change This change requires a mitigation entry in the release notes. label Oct 27, 2025
@wangchdo
Copy link
Copy Markdown
Contributor Author

wangchdo commented Oct 29, 2025

you need fix apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ too.

Hi @xiaoxiang781216 Do you think this PR can be merged after apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ are fixed?

@xiaoxiang781216
Copy link
Copy Markdown
Contributor

you need fix apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ too.

Hi @xiaoxiang781216 Do you think this PR can be merged after apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ are fixed?

apps/nshlib could, but other need be part of this pr.

@linguini1
Copy link
Copy Markdown
Contributor

Commit messages look good!

@linguini1 linguini1 changed the title sched/event: breaking change!: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT sched/event!: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT Oct 29, 2025
@wangchdo
Copy link
Copy Markdown
Contributor Author

wangchdo commented Oct 30, 2025

you need fix apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ too.

Hi @xiaoxiang781216 Do you think this PR can be merged after apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ are fixed?

apps/nshlib could, but other need be part of this pr.

Hi @xiaoxiang781216

I’ve uploaded a fix for tools/pynuttx/nxgdb and sched_get_stateinfo.c/g_statenames. Could you please help review it?

Also, I didn’t find anything in procfs/ that needs to be updated. I believe the new state doesn’t affect procfs/, but could you please help double-check that?
Besides, I think no changes are needed in apps/nshlib/ either — as long as sched_get_stateinfo.c/g_statenames is updated, everything should work correctly. Please check the test code and ps output below for confirmation.

test code:


static int nx_event_task(int argc, FAR char **argv)
{
  nxevent_t event;

  nxevent_init(&event, 0);

  nxevent_wait(&event, 0x01, NXEVENT_WAIT_ALL);
  return NULL;
}

int main(int argc, FAR char *argv[])
{
  int pid;

  pid = nxthread_create("event wait task", TCB_FLAG_TTYPE_KERNEL,
                        100,
                        NULL, 1024,
                        nx_event_task, NULL, NULL);
  return 0;
}

ps output

image

@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from eca0575 to 6fe3834 Compare October 30, 2025 01:58
@xiaoxiang781216
Copy link
Copy Markdown
Contributor

you need fix apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ too.

Hi @xiaoxiang781216 Do you think this PR can be merged after apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ are fixed?

apps/nshlib could, but other need be part of this pr.

Hi @xiaoxiang781216

I’ve uploaded a fix for tools/pynuttx/nxgdb and sched_get_stateinfo.c/g_statenames. Could you please help review it?

Also, I didn’t find anything in procfs/ that needs to be updated. I believe the new state doesn’t affect procfs/, but could you please help double-check that?

The latest code call nxsched_get_stateinfo to convert enum to string in all place, and your patch already update nxsched_get_stateinfo, so the problem is fixed.

Besides, I think no changes are needed in apps/nshlib/ either — as long as sched_get_stateinfo.c/g_statenames is updated, everything should work correctly. Please check the test code and ps output below for confirmation.

you are right.

@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from 6fe3834 to 6cf7c0e Compare October 30, 2025 04:39
@github-actions github-actions Bot added the Area: Documentation Improvements or additions to documentation label Oct 30, 2025
@jerpelea jerpelea changed the title sched/event!: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT [!] sched/event!: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT Oct 30, 2025
@jerpelea jerpelea changed the title [!] sched/event!: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT [!] sched/event: removed event_tickwait_wait() and added TSTATE_WAIT_EVENT Oct 30, 2025
@jerpelea
Copy link
Copy Markdown
Contributor

jerpelea commented Oct 30, 2025

I updated PR title with the proper marker
please do the same for the breaking commit

Thanks
Alin

…n event

    The current event implementation uses semaphores for wait and post
    operations. Since semaphores are relatively heavy-weight and intended
    for resource-based synchronization, this is suboptimal.

    So this patch replaced the semaphore-based mechanism with direct
    scheduler operations to improve performance and reduce memory footprint.

    This patch also introduce a new task state TSTATE_WAIT_EVENT to indicate
    the task is waiting for a event.

    BREAKING CHANGE:  This commit introduced a new task state TSTATE_WAIT_EVENT
    so apps/nshlib/, procfs/ and tools/pynuttx/nxgdb/ are needed to be updated accordingly.

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
    If the thread is blocked waiting on a event, then the
    thread must be unblocked from the evnet to handle
    the task cancellation.

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
   Implement nxevent_timeout with nxevent_wait_irq to remove
   code duplication

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
    The current event implementation relies on a dedicated wait object,
    which introduces several issues:
        1. Increases memory usage
        2. Complicates the event logic
        3. Makes the API design less clean, as it requires a separate
                nxevent_tickwait_wait() function

    This patch removes the event’s dependency on the wait object
    and eliminates the nxevent_tickwait_wait() API accordingly.

    BREAKING CHANGE: this commit removed the nxevent_tickwait_wait function
    so the related docs should be updated accordingly

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
   This patch add new taskstate entry in tools/pynuttx/nxgdb/thread.py
   and in sched_get_stateinfo.c/g_statenames for the new added task
   state: TSTATE_WAIT_EVENT

   test log is:
     nsh> ps
       PID  PPID GROUP PRI POLICY   TYPE    NPX STATE    EVENT     SIGMASK            STACK    USED FILLED COMMAND
         0     0     0   0 FIFO     Kthread   - Ready              0000000000000000 0001008 0000196  19.4%  Idle_Task
         1     0     0 192 FIFO     Kthread   - Waiting  Semaphore 0000000000000000 0001984 0000016   0.8%  hpwork 0x70000100 0x7000014c
         2     0     2 100 FIFO     Task      - Running            0000000000000000 0002008 0000740  36.8%  nsh_main
         4     0     0 100 FIFO     Kthread   - Waiting  Event     0000000000000000 0000984 0000012   1.2%  event_wait_task

Signed-off-by: Chengdong Wang wangchengdong@lixiang.com
    Remove the nxevent_tickwait_wait() description from events.rst,
    since this API was removed during the event module refactoring.

Signed-off-by: Chengdong Wang <wangchengdong@lixiang.com>
@wangchdo wangchdo force-pushed the improve_event_new_1024 branch from 22dc06c to 58cb3e2 Compare October 30, 2025 14:37
@wangchdo
Copy link
Copy Markdown
Contributor Author

I updated PR title with the proper marker please do the same for the breaking commit

Thanks Alin

@jerpelea

Done, please check:

image

@wangchdo
Copy link
Copy Markdown
Contributor Author

Please help to check if this PR can be merged @acassis @xiaoxiang781216

@acassis acassis merged commit 5a6242a into apache:master Oct 31, 2025
41 checks passed
@acassis
Copy link
Copy Markdown
Contributor

acassis commented Oct 31, 2025

@wangchdo I think now the nuttx-apps need to be updated

@wangchdo
Copy link
Copy Markdown
Contributor Author

wangchdo commented Nov 1, 2025

@wangchdo I think now the nuttx-apps need to be updated

Hi @acassis nuttx-apps is not needed to be updated anymore according to my analysis, and @xiaoxiang781216 has helped to confirm it

Please check the conversation i had with @xiaoxiang781216 above for the details of my analysis

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Documentation Improvements or additions to documentation Area: OS Components OS Components issues Area: Tooling breaking change This change requires a mitigation entry in the release notes. Size: L The size of the change in this PR is large Size: M The size of the change in this PR is medium State: Needs more testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants