Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deep review and redesign of API for work queue functionality #27356

Closed
pabigot opened this issue Aug 3, 2020 · 1 comment · Fixed by #29618
Closed

deep review and redesign of API for work queue functionality #27356

pabigot opened this issue Aug 3, 2020 · 1 comment · Fixed by #29618
Labels
area: API Changes to public APIs area: Kernel area: Workqueues Workqueues Enhancement Changes/Updates/Additions to existing features

Comments

@pabigot
Copy link
Collaborator

pabigot commented Aug 3, 2020

As requested by @andrewboie here and rephrasing this comment:

As demonstrated in #23132 although k_delayed_work_cancel() is documented to return -EALREADY if the work item has been completed, it actually returns it if the work item has already been submitted.

I'm skeptical of the whole k_work implementation. I'm seeing at least nine distinct states for a work item that is not cancelled:

  • never submitted
  • delayed on timeout
  • timeout expired, not yet added to queue (in z_clock_announce())
  • marked pending but not on queue (by k_submit_to_queue)
  • on queue
  • removed from queue (by z_work_q_main)
  • cleared pending (by z_work_q_main)
  • invoking handler (by z_work_q_main)
  • complete

Most of these can't be inferred by inspecting the state.

Somebody should take the time to go through the whole k_work API and implementation and revise it so the documentation accurately describes the behavior and the functionality is useful. By "useful" I mean that after an attempt to cancel you should always be able to tell whether the item was:

  • submitted but not yet committed to processing (delayed or on queue): can be cancelled, 0
  • processing (including committed to processing, not yet in handler): cannot be cancelled, now -EINVAL preferably -EINPROGRESS
  • complete (which may be indistinguishable from unsubmitted): too late to cancel, now -EALREADY preferably -EINVAL

This capability should be present whether the item is a standard work item, a delayed work item, or a pollable work item.

@pabigot pabigot added Enhancement Changes/Updates/Additions to existing features area: Kernel area: API Changes to public APIs labels Aug 3, 2020
@pabigot
Copy link
Collaborator Author

pabigot commented Aug 4, 2020

@andrewboie @andyross I plan to start on this later this week. If it's something you'd like done in-house at Intel instead please reply here.

@nashif nashif added the area: Workqueues Workqueues label Aug 14, 2020
@pabigot pabigot added this to v2.5 - Feature Freeze in Release Plan Dec 1, 2020
@nashif nashif moved this from v2.5 - Feature Freeze to v2.6 - LTS2 in Release Plan Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Kernel area: Workqueues Workqueues Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants