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

Draft terminology related to API behavior definitions #20959

Closed
wants to merge 18 commits into from

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Nov 24, 2019

This PR contains an updated version of proposed terminology related to #18970, hopefully making it easier to comment though review comments.

The formatted document can be read here.

The content is in markdown because that's most convenient for me. Assuming we reach consensus we can figure out how it should be integrated into the standard documentation.

This is an evolving document used to come to consensus on the set of
defined terms necessary to precisely document the expected behavior of
API calls in various contexts.

It's in Markdown rather than reStructured Text because that's most
convenient for the editor.

This commit archives the original proposal in issue zephyrproject-rtos#18970.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
See mbolivar comment at
zephyrproject-rtos#18970 (comment)
and subsequent discussion.

Add Background section to reference and summarize key concepts from
existing documentation, particularly for thread state and priority and
execution context.

Replaced "blocking" with "yielding" for precision and consistency with
Zephyr thread terminology.

Add Commentary subsections that give a bit of explanation for why
something is important or how concepts relate to each other.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot pabigot added area: API Changes to public APIs DNM This PR should not be merged (Do Not Merge) labels Nov 24, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 24, 2019

Some checks failed. Please fix and resubmit.

Codeowners issues

New files added that are not covered in CODEOWNERS:

zephyr-api.csv

Please add one or more entries in the CODEOWNERS file to cover those files

Gitlint issues

Commit 5101abb61c:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: remove implied relation between -ok tags"

Commit 90d66ea129:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: isr-ok and pre-kernel-ok tweaks"

Commit 361a3c0f86:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: update table and implicit attribute relations"

Commit 75ca6e1798:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: wording tweaks"

Commit b1c85f5c1f:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: more revisions"

Commit fb043136af:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: add file listing Zephyr public functions and attributes"

Commit a90c63a638:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: refactor to isolate proposed attributes"

Commit 889e03a1ce:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: miscellaneous cleanups"

Commit 63eeeb6433:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: restore missing modifier"

Commit 45a4d506df:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: move background below definitions"

Commit 250b3d45b5:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: define contexts and contextual restrictions"

Commit f224da0765:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: tweak cooperative-safe"

Commit 5416c5aa2a:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: updates to unready, reschedule-safe"

Commit 7cab23a18c:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: minor clarifications"

Commit 36a92dff80:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: more refinements to thread/scheduler terms and behavior"

Commit c1409e3594:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: refinements to thread and function/operation terminology"

Commit 6c85575d37:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: api18970: modifications per discussion 2019-11-19"

Commit f0458907b4:
1: T5 Title contains the word 'wip' (case-insensitive): "WIP: doc: definitions for precisely defining API behavior"

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

doc/api18970.md Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
@pabigot
Copy link
Collaborator Author

pabigot commented Dec 1, 2019

Updated with the following edits. Some previous comments have been marked "resolved".

Added more reference to existing Zephyr documentation on thread terminology and behavior, and updated definitions to be more consistent and refined.

Replaced "yield" with "suspendable" when speaking of functions, and gave it a more precise definition based on cooperative thread behavior. Added "no-wait" as a way to control suspension behavior. Address "thread-safe" using these concepts.

Defined operation as distinct from function to allow precise characterization of asynchronous functions. Comment on why we should not use "blocking" as a precise term.

doc/api18970.md Outdated Show resolved Hide resolved
Added more reference to existing Zephyr documentation on thread
terminology and behavior, and updated definitions to be more
consistent and refined.

Replaced "yield" with "suspendable" when speaking of functions, and
gave it a more precise definition based on cooperative thread
behavior.  Added "no-wait" as a way to control suspension behavior.
Address "thread-safe" using these concepts.

Defined operation as distinct from function to allow precise
characterization of asynchronous functions.  Comment on why we should
not use "blocking" as a precise term.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Document what it means for a thread to become unready.

Introduce the concept of a reschedule point as that term is used
within the kernel.  Note existence of meta-irq threads and that ISRs
include a reschedule point.  Replace "suspendable" by a finer-grained
characterization of "rescheduling", accompanied by "reschedule-safe"
and "context-switching" that promise specific behavior of reschedule
points in the common case of cooperative threads.  Define "no-wait"
and "isr-callable" in terms of reschedule concepts.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Grammar and style edits, and remove outdated concern about intentional
breakage of the cooperative concept.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on board with the synchronous, asynchronous, and context-switching definitions.

The "rescheduling" term reads a bit strangely, but it makes sense and I think we should go with its definition. Choosing an odd word actually probably helps avoid people using the term loosely.

I feel the same way about "no-wait" as I do "rescheduling".

I would like to reach agreement on the isr-callable issue.

doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
Acknowledge that a thread may be made unready externally.  Use "made
unready" as a technical term where appropriate.

Define "context switch" in terms of the behavior at a reschedule
point.

Tighten up the definition of rescheduling.  Replace "reschedule-safe"
with "cooperative-safe" and its inverse "cooperative-unsafe".  Define
no-wait in terms of controllable behavior of a cooperative-unsafe
function.

Update definition of "isr-callable".

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
Copy link
Collaborator Author

pabigot commented Dec 5, 2019

Summary of recent updates:

Document what it means for a thread to become unready. Acknowledge that a thread may be made unready externally. Use "made unready" as a technical term where appropriate.

Introduce the concept of a reschedule point as that term is used within the kernel. Note existence of meta-irq threads and that ISRs include a reschedule point. Define "context switch" in terms of the behavior at a reschedule point. Replace "suspendable" by a finer-grained characterization of "rescheduling", accompanied by "cooperative-safe" (vs unsafe) and "context-switching" that promise specific behavior of reschedule points in the common case of cooperative threads. Define "no-wait" and "isr-callable" in terms of reschedule concepts.

"make unready" doesn't cover k_yield(), so use "transition from
Running" when trying to characterize the behavior that could cause a
cooperative thread to context switch.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Based on the answer to issue 21341 identify functions that cannot be
invoked from interrupt (or pre-kernel) context as those that could
cause the invoking thread to "sleep".  Define "sleep" as a voluntary
transition to a non-running active thread state.  Discard the entire
concept of "cooperative-safe" as it is fragile and not actually
relevant to this case.

Generalize the concept of no-wait from preventing a transition to
Waiting to preventing an attempt to sleep.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Address the concerns that too much of this is repeating or refining
material that belongs elsewhere by moving the supporting material
after the definitions.  Accept that nobody can understand the
definitions without understanding the supporting material.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
Copy link
Collaborator Author

pabigot commented Dec 12, 2019

Summary of recent updates:

Based on the answer to issue 21341 identify functions that cannot be invoked from interrupt (or pre-kernel) context as those that could cause the invoking thread to "sleep". Define "sleep" as a voluntary transition to a non-running active thread state. Discard the entire concept of "cooperative-safe" as it is fragile and not actually relevant.

Generalize the concept of no-wait from preventing a transition to Waiting to preventing an attempt to sleep.

Functions can be called from an ISR if they do not cause their invoking thread to sleep.

Functions are no-wait if they can be prevented from causing their invoking thread to sleep.

no-wait means not blocking.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
Typos, improved wording.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
This effort has identified three attributes that together define
callability and behavior related to thread and interrupt context:
reschedule, sleeps, and no-wait.

A fourth attribute specifies whether the function is decoupled from
the operation it initiates: async

A fifth attribute is identified from work on PR 21389 relating to user
mode programs: supervisor

Name these attributes, and try to describe them in simple standalone
terms in the absence of the full context required to understand why
they're the ones we need.  Some of that context will come from
refactoring and submitting a generalized async notification feature
along with resource management services prototyped in 21090.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
@mbolivar
Copy link
Contributor

@pabigot I think I understand you on all the definitions above. I just have some wording questions to check that and then I think these terms are good and would be useful to have in the official docs.

Extracted from PR 21389 is the existing documentation on behavior
attributes, which is limited to identifying calling context (Threads
and ISRs) and privileges (Supervisor Mode).  Functions declared in
kernel.h that do not have this information have been added.

Whether the function is a syscall or a macro is indicated by Type.

The Threads, ISRs, and Supervisor (Mode) are from 21389.

A few functions have been extended with the attributes under
consideration.  Presumably anything that is Threads but not ISRs is
sleep in the terminology here.  reschedule is not identified by 21389,
nor is no-wait except when certain functions are marked as callable
from ISRs.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Remove async as an API attribute.  It will be obvious from the
parameters that a function is async.

Remove the extension of no-wait to affect behavior of async.  This was
yet another result of considering "blocking" to imply "sleeps".  There
is no implicit problem with invoking async functions from interrupts,
as long as initialization of the operation cannot sleep.

Add isr-ok which is necessary for generic API that may be called from
interrupts but that invokes sleep functions internally at points where
a no-wait path may not be selected.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Fill in "no*" for empty cells in the example table, indicating that
the absence is a consequence of the value of another attribute rather
than an implementation decision.

Add pre-kernel-ok as another attribute that extends isr-ok to
explicitly support safe invocation from pre-kernel initialization
stages.

Add async as another attribute in preparation for precise description
of asynchronous APIs.

Clarify distinction between isr-ok and no-wait, and relation between
no-wait and async.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Make more things "yes*" as a consequent of other attribute values.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Looks like pre-kernel-ok is new. It makes sense but I wonder if we should relax the definition a little bit, to give future changes some room to breathe.

doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
doc/api18970.md Show resolved Hide resolved
doc/api18970.md Outdated Show resolved Hide resolved
Also a slight rewording to hint at the fact that meta-IRQ threads are
"cooperative" threads that preempt other threads.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
pre-kernel-ok should not be described in a way that suggests it's
coupled to an explicitly isr-ok function.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@mbolivar
Copy link
Contributor

mbolivar commented Jan 6, 2020

I am +1 on these terms as of f2474c6. Thanks!

@pabigot
Copy link
Collaborator Author

pabigot commented Jan 25, 2020

Replaced by #21678

@pabigot pabigot closed this Jan 25, 2020
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: Documentation DNM This PR should not be merged (Do Not Merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants