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

Use “thread” rather than “process” #122

Merged
merged 7 commits into from
Oct 18, 2022

Conversation

rsandifo-arm
Copy link
Contributor

@rsandifo-arm rsandifo-arm commented Dec 16, 2021

The AAPCS64 uses “process” as a cover-all term for “thread
or process”:

The AAPCS64 applies to a single thread of execution or
process (hereafter referred to as a process).

However, SME adds a register called TPIDR2_EL0 that forms an
important part of the SME PCS. The “T” in TPIDR2_EL0 stands
for “thread”, so it seemed incongruous to use “process” when
describing its role in the PCS.

This preliminary patch therefore tries to avoid treating
“thread” and “process” as synonymous. Since the exact
nature of threads and processes depends on the platform,
the patch tries to give a flexible definition.

Two later parts of the PCS refer specifically to processes:

  • “Universal stack constraints”, when describing the parts of stack
    memory that can be safely accessed. These restrictions apply to
    the program generally (wrt a given thread's stack and SP), so the
    patch uses “conforming program” instead of “thread”. This is also
    the term used in:

    A conforming program must only execute instructions that are in
    areas of memory designated to contain code.

  • The section on interworking. These restrictions specifically apply
    at the process level rather than just the thread level, so the patch
    leaves the wording unchanged.

@rearnsha
Copy link

Whilst there is room for improvement in the wording here, I don't entirely agree with this change. Conceptually, at least (it may be different in environments where memory is flat-mapped with a single page mapping shared by all processes), a process consists of one or more threads of execution. Threads may either execute concurrently (on multiple compute elements in the system) or serially (being switched under the control of an operating system).

With the exception of the stack and thread-local storage the memory is managed and shared across all threads of the process. I don't think it makes sense to say that the heap is managed by a thread - it's managed across the entire process.

Stack and thread-local data are /normally/ private to the thread, but a thread may export the addresses of certain objects to other threads, provided that it can guarantee that the contents of those objects exists and is stable at the times that other threads might access it.

@rearnsha
Copy link

Of course, there is also 'shared memory': memory that is shared by two or more processes. How such sharing is done is, I think, beyond the scope of the AAPCS and into the realms of platform-specific behaviour.

@rsandifo-arm
Copy link
Contributor Author

Whilst there is room for improvement in the wording here, I don't entirely agree with this change. Conceptually, at least (it may be different in environments where memory is flat-mapped with a single page mapping shared by all processes), a process consists of one or more threads of execution. Threads may either execute concurrently (on multiple compute elements in the system) or serially (being switched under the control of an operating system).

With the exception of the stack and thread-local storage the memory is managed and shared across all threads of the process. I don't think it makes sense to say that the heap is managed by a thread - it's managed across the entire process.

Stack and thread-local data are /normally/ private to the thread, but a thread may export the addresses of certain objects to other threads, provided that it can guarantee that the contents of those objects exists and is stable at the times that other threads might access it.

Yeah, the new wording was trying to capture that (but clearly failed :-)). The claim wasn't that the thread “owns” these areas of memory in any sense, or that they're private to the thread. It was just that a thread (potentially) has access to these areas of memory. That's why it says:

Each thread must have its own stack, but it can share other categories of memory with other threads in a process.
(A platform might allow a thread to access the stacks of other threads, perhaps by treating them as part of the heap.)

The point with the second sentence is that, if thread T1 shares a stack object S with thread T2, S is not part of stack memory from T2's point of view. The distinction is important because of the later requirements about which stack addresses a thread/process is allowed to access. If we said that S was in “stack memory” even from T2's point of view, an access to S would be out of bounds (and so invalid) for T2.

So it feels like we might be saying the same thing, but in different ways.

Like you say, there are a few possible variations, and I don't think it's really up to the PCS to define exactly which memory is shared between threads in a process or between processes in a running system, beyond the requirement that the area of memory designated to be stack memory is specific to each thread. E.g. there's no reason in principle to exclude FDPIC-like models, where code is shared between processes.

@rsandifo-arm rsandifo-arm force-pushed the threads-and-processes branch 2 times, most recently from 8fb21a8 to bd3c780 Compare May 13, 2022 12:40
@rsandifo-arm
Copy link
Contributor Author

Hi @rearnsha. I've (very belatedly) tried to address your concerns in the updated version of the patch. How does this version look?

aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
The AAPCS64 uses “process” as a cover-all term for “thread
or process”:

  The AAPCS64 applies to a single thread of execution or
  process (hereafter referred to as a process).

However, SME adds a register called TPIDR2_EL0 that forms an
important part of the SME PCS.  The “T” in TPIDR2_EL0 stands
for “thread”, so it seemed incongruous to use “process” when
describing its role in the PCS.

This preliminary patch therefore tries to avoid treating
“thread” and “process” as synonymous.  Since the exact
nature of threads and processes depends on the platform,
the patch tries to give a flexible definition.

Two later parts of the PCS refer specifically to processes:

- “Universal stack constraints”, when describing the parts of stack
  memory that can be safely accessed.  These restrictions apply to
  the program generally (wrt a given thread's stack and SP), so the
  patch uses “conforming program” instead of “thread”.  This is also
  the term used in:

    A conforming program must only execute instructions that are in
    areas of memory designated to contain code.

- The section on interworking.  These restrictions specifically apply
  at the process level rather than just the thread level, so the patch
  leaves the wording unchanged.
@rsandifo-arm
Copy link
Contributor Author

I've rebased onto #79 and tweaked the wording around the stack a bit more.

@stuij
Copy link
Member

stuij commented Aug 11, 2022

@rearnsha, are you now happy with this pull request, after the latest changes of @rsandifo-arm?

operating system. If the platform supports multiple processes but has
no separate concept of threads, “thread” is synonymous with “process”.
If a platform has no concurrency or preemption then it will generally
have a single “thread” and “process” that executes all instructions.

Choose a reason for hiding this comment

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

Avoid using quotation marks for emphasis - see Writing Guide https://confluence.arm.com/display/TCOM/Avoid+formatting+for+emphasis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't really intending to use quotes for emphasis here. This paragraph is trying to define what the ABI means by the terms “thread” and “process” (sometimes in circumstances where the concept of thread and process might seem unnatural).

Choose a reason for hiding this comment

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

In this case - I would just use the quotes in the first instance of the term in in the paragraph.

Copy link
Contributor Author

@rsandifo-arm rsandifo-arm Oct 6, 2022

Choose a reason for hiding this comment

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

Dropping the quotes later would lead to things like:

thread and process generally have their usual meanings for that operating system

which might be quite hard to parse. It would be less clear that the text is referring to the word/term “thread” rather than to an actual thread.

Perhaps this just shows that I should reword the whole pararaph :-)

Choose a reason for hiding this comment

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

:)

Copy link

Choose a reason for hiding this comment

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

Elsewhere in this document we've used capitalization for defined terms, for example 'Composite Type' (although after multiple rounds of editing that's perhaps become a bit inconsistent).
Another alternative would be to simply move the definitions into the glossary of terms at the start of the document and then take the definitions as read at this point. That would only work in this case if the terms are used consistently throughout the document and if the definitions can be kept fairly succinct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather keep the paragraph here. But I've tried to reword it to avoid excessive quoting.

aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
Copy link

@sallyarmneale sallyarmneale left a comment

Choose a reason for hiding this comment

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

A few minor changes. Always use might instead of may. It's more easily understood by non-native speakers.

@rsandifo-arm
Copy link
Contributor Author

A few minor changes. Always use might instead of may. It's more easily understood by non-native speakers.

Thanks for the reviews!

I was originally trying to keep the existing wording as much as possible, because I was worried about over-editorialising. But a lot of the suggestions seem uncontroversial, so I've pushed an update with them included.

The “may” vs. “might” thing is trickier though. I think we need to take an action to go through the doc and make a coordinated attempt to remove uses of “may”. It isn't always obvious what the right replacement would be. I get the impression that the doc is being deliberately vague about whether it's describing a state of affairs that holds naturally/inherently, or whether it's prescribing what platforms can and can't do.

For example, I think this:

The address space may consist of one or more disjoint regions

was probably intended to be a concession: platforms are allowed to create disjoint memory regions. Similarly, I think:

No region may span address zero

is supposed to prohibit any attempt to create regions that span address zero. I'm not sure “might” would be a suitable replacement.

@rearnsha
Copy link

rearnsha commented Oct 6, 2022

"May" is the correct term here. In standards documents like these it always has the specific meaning of "permitted but not required". Using another word or term would just be distracting.

aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
Comment on lines 1018 to 1020
- A conforming program must not access (for reading or writing) the
inactive region of S. This is true regardless of whether the access
is performed by T or by some other thread.
Copy link

Choose a reason for hiding this comment

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

I thought we had agreed that reads from below the stack, solely for the purposes of stack probing should be permitted, provided the probe is less than 64k below the current stack pointer (page size with 64-k pages) and provided the stack pointer is then immediately adjusted to the point of the probe (or lower). Any data read from the probed location has unspecified contents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but I think the idea was to tackle that as a separate PR.

aapcs64/aapcs64.rst Outdated Show resolved Hide resolved
limit to the base as the stack shrinks. In practice, an application might
not be able to determine the value of either the base or the limit.

In the description below, the base, limit and SP for a thread T are

Choose a reason for hiding this comment

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

Rather than 'SP' here, I think it would be better to say 'current stack extent'. SP is the register where this is stored, as stated above.

Copy link

@rearnsha rearnsha left a comment

Choose a reason for hiding this comment

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

I think this is pretty much OK now. Just one minor change suggested.

@stuij stuij merged commit 9210606 into ARM-software:main Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants