2023-02-14 19:54:59

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

Signed-off-by: Mathieu Desnoyers <[email protected]>
---
man2/rseq.2 | 465 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 465 insertions(+)
create mode 100644 man2/rseq.2

diff --git a/man2/rseq.2 b/man2/rseq.2
new file mode 100644
index 000000000..ad3215d1c
--- /dev/null
+++ b/man2/rseq.2
@@ -0,0 +1,465 @@
+.\" Copyright 2015-2023 Mathieu Desnoyers <[email protected]>
+.\"
+.\" SPDX-License-Identifier: Linux-man-pages-copyleft
+.\"
+.TH rseq 2 (date) "Linux man-pages (unreleased)"
+.SH NAME
+rseq \- restartable sequences system call
+.SH LIBRARY
+Standard C library
+.RI ( libc ", " \-lc )
+.SH SYNOPSIS
+.nf
+.PP
+.BR "#include <linux/rseq.h>" " /* Definition of " RSEQ_* " constants */"
+.BR "#include <sys/syscall.h>" " /* Definition of " SYS_* " constants */"
+.B #include <unistd.h>
+.PP
+.BI "int syscall(SYS_rseq, struct rseq *" rseq ", uint32_t " rseq_len ,
+.BI " int " flags ", uint32_t " sig );
+.fi
+.PP
+.IR Note :
+glibc provides no wrapper for
+.BR rseq (),
+necessitating the use of
+.BR syscall (2).
+.SH DESCRIPTION
+The
+.BR rseq ()
+ABI accelerates specific user-space operations by registering a
+per-thread data structure shared between kernel and user-space.
+This data structure can be read from or written to by user-space to skip
+otherwise expensive system calls.
+.PP
+A restartable sequence is a sequence of instructions
+guaranteed to be executed atomically with respect to
+other threads and signal handlers on the current CPU.
+If its execution does not complete atomically,
+the kernel changes the execution flow by jumping to an abort handler
+defined by user-space for that restartable sequence.
+.PP
+Using restartable sequences requires to register a
+.BR rseq ()
+ABI per-thread data structure
+.RB ( "struct rseq" )
+through the
+.BR rseq ()
+system call.
+Only one
+.BR rseq ()
+ABI can be registered per thread, so user-space libraries and
+applications must follow a user-space ABI defining how to share this
+resource.
+The ABI defining how to share this resource between applications and
+libraries is defined by the C library.
+Allocation of the per-thread
+.BR rseq ()
+ABI and its registration to the kernel is handled by glibc since version
+2.35.
+.PP
+The
+.BR rseq ()
+ABI per-thread data structure contains a
+.I rseq_cs
+field which points to the currently executing critical section.
+For each thread, a single rseq critical section can run at any given
+point.
+Each critical section need to be implemented in assembly.
+.PP
+The
+.BR rseq ()
+ABI accelerates user-space operations on per-cpu data by defining a
+shared data structure ABI between each user-space thread and the kernel.
+.PP
+It allows user-space to perform update operations on per-cpu data
+without requiring heavy-weight atomic operations.
+.PP
+The term CPU used in this documentation refers to a hardware execution
+context.
+For instance, each CPU number returned by
+.BR sched_getcpu ()
+is a CPU.
+The current CPU means to the CPU on which the registered thread is
+running.
+.PP
+Restartable sequences are atomic with respect to preemption (making it
+atomic with respect to other threads running on the same CPU),
+as well as signal delivery (user-space execution contexts nested over
+the same thread).
+They either complete atomically with respect to preemption on the
+current CPU and signal delivery, or they are aborted.
+.PP
+Restartable sequences are suited for update operations on per-cpu data.
+.PP
+Restartable sequences can be used on data structures shared between threads
+within a process,
+and on data structures shared between threads across different
+processes.
+.PP
+Some examples of operations that can be accelerated or improved by this ABI:
+.IP \(bu 3
+Memory allocator per-cpu free-lists,
+.IP \(bu 3
+Querying the current CPU number,
+.IP \(bu 3
+Incrementing per-CPU counters,
+.IP \(bu 3
+Modifying data protected by per-CPU spinlocks,
+.IP \(bu 3
+Inserting/removing elements in per-CPU linked-lists,
+.IP \(bu 3
+Writing/reading per-CPU ring buffers content.
+.IP \(bu 3
+Accurately reading performance monitoring unit counters with respect to
+thread migration.
+.PP
+Restartable sequences must not perform system calls.
+Doing so may result in termination of the process by a segmentation
+fault.
+.PP
+The
+.I rseq
+argument is a pointer to the thread-local
+.B struct rseq
+to be shared between kernel and user-space.
+.PP
+The structure
+.B struct rseq
+is an extensible structure.
+Additional feature fields can be added in future kernel versions.
+Its layout is as follows:
+.TP
+.B Structure alignment
+This structure is aligned on either 32-byte boundary,
+or on the alignment value returned by
+.IR getauxval ()
+invoked with
+.B AT_RSEQ_ALIGN
+if the structure size differs from 32 bytes.
+.TP
+.B Structure size
+This structure size needs to be at least 32 bytes.
+It can be either 32 bytes,
+or it needs to be large enough to hold the result of
+.IR getauxval ()
+invoked with
+.BR AT_RSEQ_FEATURE_SIZE .
+Its size is passed as parameter to the
+.BR rseq ()
+system call.
+.in +4n
+.IP
+.EX
+#include <linux/rseq.h>
+
+struct rseq {
+ __u32 cpu_id_start;
+ __u32 cpu_id;
+ union {
+ /* ... */
+ } rseq_cs;
+ __u32 flags;
+ __u32 node_id;
+ __u32 mm_cid;
+} __attribute__((aligned(32)));
+.EE
+.in
+.TP
+.B Fields
+.RS
+.TP
+.I cpu_id_start
+Always-updated value of the CPU number on which the registered thread is
+running.
+Its value is guaranteed to always be a possible CPU number,
+even when
+.BR rseq ()
+is not registered.
+Its value should always be confirmed by reading the cpu_id field before
+user-space performs any side-effect
+(e.g. storing to memory).
+.IP
+This field is always guaranteed to hold a valid CPU number in the range
+[ 0 .. nr_possible_cpus - 1 ].
+It can therefore be loaded by user-space
+and used as an offset in per-cpu data structures
+without having to check whether its value is within the valid bounds
+compared to the number of possible CPUs in the system.
+.IP
+Initialized by user-space to a possible CPU number (e.g., 0),
+updated by the kernel for threads registered with
+.BR rseq ().
+.IP
+For user-space applications executed on a kernel without
+.BR rseq ()
+support,
+the cpu_id_start field stays initialized at 0,
+which is indeed a valid CPU number.
+It is therefore valid to use it as an offset in per-cpu data structures,
+and only validate whether it's actually the current CPU number by
+comparing it with the cpu_id field within the rseq critical section.
+If the kernel does not provide
+.BR rseq ()
+support, that cpu_id field stays initialized at -1,
+so the comparison always fails, as intended.
+.IP
+This field should only be read by the thread which registered this data
+structure.
+Aligned on 32-bit.
+.IP
+It is up to user space to implement a fall-back mechanism for scenarios where
+.BR rseq ()
+is not available.
+.TP
+.I cpu_id
+Always-updated value of the CPU number on which the registered thread is
+running.
+Initialized by user-space to -1,
+updated by the kernel for threads registered with
+.BR rseq ().
+.IP
+This field should only be read by the thread which registered this data
+structure.
+Aligned on 32-bit.
+.TP
+.I rseq_cs
+The rseq_cs field is a pointer to a
+.BR "struct rseq_cs" .
+Is is NULL when no rseq assembly block critical section is active for
+the registered thread.
+Setting it to point to a critical section descriptor
+.RB ( "struct rseq_cs")
+marks the beginning of the critical section.
+.IP
+Initialized by user-space to NULL.
+.IP
+Updated by user-space, which sets the address of the currently
+active rseq_cs at the beginning of assembly instruction sequence
+block,
+and set to NULL by the kernel when it restarts an assembly instruction
+sequence block,
+as well as when the kernel detects that it is preempting or delivering a
+signal outside of the range targeted by the rseq_cs.
+Also needs to be set to NULL by user-space before reclaiming memory that
+contains the targeted
+.BR "struct rseq_cs" .
+.IP
+Read and set by the kernel.
+.IP
+This field should only be updated by the thread which registered this
+data structure.
+Aligned on 64-bit.
+.TP
+.I flags
+Flags indicating the restart behavior for the registered thread.
+This is mainly used for debugging purposes.
+Can be a combination of:
+.RS
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
+Inhibit instruction sequence block restart on preemption for this
+thread.
+This flag is deprecated since Linux 6.1.
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
+Inhibit instruction sequence block restart on signal delivery for this
+thread.
+This flag is deprecated since Linux 6.1.
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
+Inhibit instruction sequence block restart on migration for this thread.
+This flag is deprecated since Linux 6.1.
+.RE
+.IP
+Initialized by user-space, used by the kernel.
+.TP
+.I node_id
+Always-updated value of the current NUMA node ID.
+.IP
+Initialized by user-space to 0.
+.IP
+Updated by the kernel.
+Read by user-space with single-copy atomicity semantics.
+This field should only be read by the thread which registered
+this data structure.
+Aligned on 32-bit.
+.TP
+.I mm_cid
+Contains the current thread's concurrency ID
+(allocated uniquely within a memory map).
+.IP
+Updated by the kernel.
+Read by user-space with single-copy atomicity semantics.
+This field should only be read by the thread which registered this data
+structure.
+Aligned on 32-bit.
+.IP
+This concurrency ID is within the possible cpus range,
+and is temporarily (and uniquely) assigned while threads are actively
+running within a memory map.
+If a memory map has fewer threads than cores,
+or is limited to run on few cores concurrently through sched affinity or
+cgroup cpusets,
+the concurrency IDs will be values close to 0,
+thus allowing efficient use of user-space memory for per-cpu data
+structures.
+.RE
+.PP
+The layout of
+.B struct rseq_cs
+version 0 is as follows:
+.TP
+.B Structure alignment
+This structure is aligned on 32-byte boundary.
+.TP
+.B Structure size
+This structure has a fixed size of 32 bytes.
+.in +4n
+.IP
+.EX
+#include <linux/rseq.h>
+
+struct rseq_cs {
+ __u32 version;
+ __u32 flags;
+ __u64 start_ip;
+ __u64 post_commit_offset;
+ __u64 abort_ip;
+} __attribute__((aligned(32)));
+.EE
+.in
+.TP
+.B Fields
+.RS
+.TP
+.I version
+Version of this structure.
+Should be initialized to 0.
+.TP
+.I flags
+.RS
+Flags indicating the restart behavior of this structure.
+Can be a combination of:
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
+Inhibit instruction sequence block restart on preemption for this
+critical section.
+This flag is deprecated since Linux 6.1.
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
+Inhibit instruction sequence block restart on signal delivery for this
+critical section.
+This flag is deprecated since Linux 6.1.
+.TP
+.B RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
+Inhibit instruction sequence block restart on migration for this
+critical section.
+This flag is deprecated since Linux 6.1.
+.RE
+.TP
+.I start_ip
+Instruction pointer address of the first instruction of the sequence of
+consecutive assembly instructions.
+.TP
+.I post_commit_offset
+Offset (from start_ip address) of the address after the last instruction
+of the sequence of consecutive assembly instructions.
+.TP
+.I abort_ip
+Instruction pointer address where to move the execution flow in case of
+abort of the sequence of consecutive assembly instructions.
+.RE
+.PP
+The
+.I rseq_len
+argument is the size of the
+.B struct rseq
+to register.
+.PP
+The
+.I flags
+argument is 0 for registration, and
+.B RSEQ_FLAG_UNREGISTER
+for unregistration.
+.PP
+The
+.I sig
+argument is the 32-bit signature to be expected before the abort
+handler code.
+.PP
+A single library per process should keep the
+.B struct rseq
+in a per-thread data structure.
+The
+.I cpu_id
+field should be initialized to -1, and the
+.I cpu_id_start
+field should be initialized to a possible CPU value (typically 0).
+.PP
+Each thread is responsible for registering and unregistering its
+.BR "struct rseq" .
+No more than one
+.B struct rseq
+address can be registered per thread at a given time.
+.PP
+Reclaim of
+.B struct rseq
+object's memory must only be done after either an explicit rseq
+unregistration is performed or after the thread exits.
+.PP
+In a typical usage scenario, the thread registering the
+.B struct rseq
+will be performing loads and stores from/to that structure.
+It is however also allowed to read that structure from other threads.
+The
+.B struct rseq
+field updates performed by the kernel provide relaxed atomicity
+semantics (atomic store, without memory ordering),
+which guarantee that other threads performing relaxed atomic reads
+(atomic load, without memory ordering) of the cpu number fields will
+always observe a consistent value.
+.SH RETURN VALUE
+A return value of 0 indicates success.
+On error, \-1 is returned, and
+.I errno
+is set appropriately.
+.SH ERRORS
+.TP
+.B EINVAL
+Either
+.I flags
+contains an invalid value, or
+.I rseq
+contains an address which is not appropriately aligned, or
+.I rseq_len
+contains an incorrect size.
+.TP
+.B ENOSYS
+The
+.BR rseq ()
+system call is not implemented by this kernel.
+.TP
+.B EFAULT
+.I rseq
+is an invalid address.
+.TP
+.B EBUSY
+Restartable sequence is already registered for this thread.
+.TP
+.B EPERM
+The
+.I sig
+argument on unregistration does not match the signature received
+on registration.
+.SH VERSIONS
+The
+.BR rseq ()
+system call was added in Linux 4.18.
+.SH STANDARDS
+.BR rseq ()
+is Linux-specific.
+.SH SEE ALSO
+.BR sched_getcpu (3) ,
+.BR membarrier (2) ,
+.BR getauxval (3)
--
2.30.2



2023-02-14 22:29:56

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

Hi Mathieu and Branden,

On 2/14/23 20:54, Mathieu Desnoyers wrote:
> Signed-off-by: Mathieu Desnoyers <[email protected]>
> ---
> man2/rseq.2 | 465 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 465 insertions(+)
> create mode 100644 man2/rseq.2
>
> diff --git a/man2/rseq.2 b/man2/rseq.2
> new file mode 100644
> index 000000000..ad3215d1c
> --- /dev/null
> +++ b/man2/rseq.2
> @@ -0,0 +1,465 @@
> +.\" Copyright 2015-2023 Mathieu Desnoyers <[email protected]>
> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.\"
> +.TH rseq 2 (date) "Linux man-pages (unreleased)"
> +.SH NAME
> +rseq \- restartable sequences system call
> +.SH LIBRARY
> +Standard C library
> +.RI ( libc ", " \-lc )
> +.SH SYNOPSIS
> +.nf
> +.PP
> +.BR "#include <linux/rseq.h>" " /* Definition of " RSEQ_* " constants */"
> +.BR "#include <sys/syscall.h>" " /* Definition of " SYS_* " constants */"
> +.B #include <unistd.h>
> +.PP
> +.BI "int syscall(SYS_rseq, struct rseq *" rseq ", uint32_t " rseq_len ,
> +.BI " int " flags ", uint32_t " sig );
> +.fi
> +.PP
> +.IR Note :
> +glibc provides no wrapper for
> +.BR rseq (),
> +necessitating the use of
> +.BR syscall (2).
> +.SH DESCRIPTION
> +The
> +.BR rseq ()
> +ABI accelerates specific user-space operations by registering a
> +per-thread data structure shared between kernel and user-space.

This last 'user-space' is not adjectivated, so it should go without
a hyphen, according to common English rules.

> +This data structure can be read from or written to by user-space to skip

same here

> +otherwise expensive system calls.
> +.PP
> +A restartable sequence is a sequence of instructions
> +guaranteed to be executed atomically with respect to
> +other threads and signal handlers on the current CPU.
> +If its execution does not complete atomically,
> +the kernel changes the execution flow by jumping to an abort handler
> +defined by user-space for that restartable sequence.

same here

> +.PP
> +Using restartable sequences requires to register a
> +.BR rseq ()
> +ABI per-thread data structure
> +.RB ( "struct rseq" )

We format types in italics, so this should be '.RI'.

> +through the
> +.BR rseq ()
> +system call.
> +Only one
> +.BR rseq ()
> +ABI can be registered per thread, so user-space libraries and
> +applications must follow a user-space ABI defining how to share this
> +resource.

Please use semantic newlines. See man-pages(7):

Use semantic newlines
In the source of a manual page, new sentences should be started on new
lines, long sentences should be split into lines at clause breaks (com‐
mas, semicolons, colons, and so on), and long clauses should be split
at phrase boundaries. This convention, sometimes known as "semantic
newlines", makes it easier to see the effect of patches, which often
operate at the level of individual sentences, clauses, or phrases.

In the above lines, that would mean breaking after the comma, and not leaving resource in a line of its own.

> +The ABI defining how to share this resource between applications and
> +libraries is defined by the C library.
> +Allocation of the per-thread
> +.BR rseq ()
> +ABI and its registration to the kernel is handled by glibc since version
> +2.35.
> +.PP
> +The
> +.BR rseq ()
> +ABI per-thread data structure contains a
> +.I rseq_cs
> +field which points to the currently executing critical section.

currently-executing should probably use a hyphen
(if I understood the line correctly).
However, you might want to reorder the phrase to remove the need for that.

See an interesting discussion in the groff@ mailing list:
<https://lists.gnu.org/archive/html/groff/2022-10/msg00015.html>

> +For each thread, a single rseq critical section can run at any given
> +point.
> +Each critical section need to be implemented in assembly.

needs?

> +.PP
> +The
> +.BR rseq ()
> +ABI accelerates user-space operations on per-cpu data by defining a
> +shared data structure ABI between each user-space thread and the kernel.
> +.PP
> +It allows user-space to perform update operations on per-cpu data
> +without requiring heavy-weight atomic operations.
> +.PP
> +The term CPU used in this documentation refers to a hardware execution
> +context.
> +For instance, each CPU number returned by
> +.BR sched_getcpu ()
> +is a CPU.
> +The current CPU means to the CPU on which the registered thread is
> +running.
> +.PP
> +Restartable sequences are atomic with respect to preemption (making it
> +atomic with respect to other threads running on the same CPU),
> +as well as signal delivery (user-space execution contexts nested over
> +the same thread).
> +They either complete atomically with respect to preemption on the
> +current CPU and signal delivery, or they are aborted.
> +.PP
> +Restartable sequences are suited for update operations on per-cpu data.
> +.PP
> +Restartable sequences can be used on data structures shared between threads
> +within a process,
> +and on data structures shared between threads across different
> +processes.
> +.PP
> +Some examples of operations that can be accelerated or improved by this ABI:
> +.IP \(bu 3

Please use \[bu]

We changed that recently:
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=36f73ba37945c7ff4aa2d8383f831519a38e3f27>

> +Memory allocator per-cpu free-lists,
> +.IP \(bu 3
> +Querying the current CPU number,
> +.IP \(bu 3
> +Incrementing per-CPU counters,
> +.IP \(bu 3
> +Modifying data protected by per-CPU spinlocks,
> +.IP \(bu 3
> +Inserting/removing elements in per-CPU linked-lists,
> +.IP \(bu 3
> +Writing/reading per-CPU ring buffers content.
> +.IP \(bu 3
> +Accurately reading performance monitoring unit counters with respect to
> +thread migration.
> +.PP
> +Restartable sequences must not perform system calls.
> +Doing so may result in termination of the process by a segmentation
> +fault.
> +.PP
> +The
> +.I rseq
> +argument is a pointer to the thread-local
> +.B struct rseq
> +to be shared between kernel and user-space.
> +.PP
> +The structure
> +.B struct rseq
> +is an extensible structure.
> +Additional feature fields can be added in future kernel versions.
> +Its layout is as follows:
> +.TP
> +.B Structure alignment

Let's remove the bold here. It's not necessary for marking a constant or something that needs bold. And the indentation is already making it stand out, so bold is a bit too much aggressive to the reader.

> +This structure is aligned on either 32-byte boundary,
> +or on the alignment value returned by
> +.IR getauxval ()
> +invoked with
> +.B AT_RSEQ_ALIGN
> +if the structure size differs from 32 bytes.
> +.TP
> +.B Structure size
> +This structure size needs to be at least 32 bytes.
> +It can be either 32 bytes,
> +or it needs to be large enough to hold the result of
> +.IR getauxval ()
> +invoked with
> +.BR AT_RSEQ_FEATURE_SIZE .

Maybe it's simpler to use getauxval(AT_RSEQ_FEATURE_SIZE) as an expression?

If you decide to do so, it should go in italics (see man-pages(7)).

Complete commands should, if long, be written as an indented line on
their own, with a blank line before and after the command, for example

man 7 man-pages

If the command is short, then it can be included inline in the text, in
italic format, for example, man 7 man‐pages. In this case, it may be
worth using nonbreaking spaces (\[ti]) at suitable places in the com‐
mand. Command options should be written in italics (e.g., -l).

Expressions, if not written on a separate indented line, should be
specified in italics. Again, the use of nonbreaking spaces may be ap‐
propriate if the expression is inlined with normal text.


> +Its size is passed as parameter to the
> +.BR rseq ()
> +system call.
> +.in +4n
> +.IP
> +.EX
> +#include <linux/rseq.h>
> +
> +struct rseq {
> + __u32 cpu_id_start;
> + __u32 cpu_id;
> + union {
> + /* ... */
> + } rseq_cs;
> + __u32 flags;
> + __u32 node_id;
> + __u32 mm_cid;
> +} __attribute__((aligned(32)));
> +.EE
> +.in
> +.TP
> +.B Fields
> +.RS
> +.TP
> +.I cpu_id_start
> +Always-updated value of the CPU number on which the registered thread is
> +running.
> +Its value is guaranteed to always be a possible CPU number,
> +even when
> +.BR rseq ()
> +is not registered.
> +Its value should always be confirmed by reading the cpu_id field before

cpu_id should be formatted (.I).

> +user-space performs any side-effect
> +(e.g. storing to memory).
> +.IP
> +This field is always guaranteed to hold a valid CPU number in the range
> +[ 0 .. nr_possible_cpus - 1 ].

Please use interval notation:
[0, nr_possible_cpus)
or
[0, nr_possible_cpus - 1]
whichever looks better to you.

We did some consistency fix recently:
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>

Also, do we have a more standard way of saying nr_possible_cpus?
Should we say nproc?

> +It can therefore be loaded by user-space
> +and used as an offset in per-cpu data structures
> +without having to check whether its value is within the valid bounds
> +compared to the number of possible CPUs in the system.
> +.IP
> +Initialized by user-space to a possible CPU number (e.g., 0),
> +updated by the kernel for threads registered with
> +.BR rseq ().
> +.IP
> +For user-space applications executed on a kernel without
> +.BR rseq ()
> +support,
> +the cpu_id_start field stays initialized at 0,
> +which is indeed a valid CPU number.
> +It is therefore valid to use it as an offset in per-cpu data structures,
> +and only validate whether it's actually the current CPU number by
> +comparing it with the cpu_id field within the rseq critical section.
> +If the kernel does not provide
> +.BR rseq ()
> +support, that cpu_id field stays initialized at -1,
> +so the comparison always fails, as intended.
> +.IP
> +This field should only be read by the thread which registered this data
> +structure.
> +Aligned on 32-bit.
> +.IP
> +It is up to user space to implement a fall-back mechanism for scenarios where
> +.BR rseq ()
> +is not available.
> +.TP
> +.I cpu_id
> +Always-updated value of the CPU number on which the registered thread is
> +running.
> +Initialized by user-space to -1,
> +updated by the kernel for threads registered with
> +.BR rseq ().
> +.IP
> +This field should only be read by the thread which registered this data
> +structure.
> +Aligned on 32-bit.
> +.TP
> +.I rseq_cs
> +The rseq_cs field is a pointer to a
> +.BR "struct rseq_cs" .
> +Is is NULL when no rseq assembly block critical section is active for

"It is"

> +the registered thread.
> +Setting it to point to a critical section descriptor
> +.RB ( "struct rseq_cs")
> +marks the beginning of the critical section.
> +.IP
> +Initialized by user-space to NULL.
> +.IP
> +Updated by user-space, which sets the address of the currently
> +active rseq_cs at the beginning of assembly instruction sequence
> +block,
> +and set to NULL by the kernel when it restarts an assembly instruction
> +sequence block,
> +as well as when the kernel detects that it is preempting or delivering a
> +signal outside of the range targeted by the rseq_cs.
> +Also needs to be set to NULL by user-space before reclaiming memory that
> +contains the targeted
> +.BR "struct rseq_cs" .
> +.IP
> +Read and set by the kernel.
> +.IP
> +This field should only be updated by the thread which registered this
> +data structure.
> +Aligned on 64-bit.
> +.TP
> +.I flags
> +Flags indicating the restart behavior for the registered thread.
> +This is mainly used for debugging purposes.
> +Can be a combination of:
> +.RS
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
> +Inhibit instruction sequence block restart on preemption for this
> +thread.
> +This flag is deprecated since Linux 6.1.
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
> +Inhibit instruction sequence block restart on signal delivery for this
> +thread.
> +This flag is deprecated since Linux 6.1.
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
> +Inhibit instruction sequence block restart on migration for this thread.
> +This flag is deprecated since Linux 6.1.
> +.RE
> +.IP
> +Initialized by user-space, used by the kernel.
> +.TP
> +.I node_id
> +Always-updated value of the current NUMA node ID.
> +.IP
> +Initialized by user-space to 0.
> +.IP
> +Updated by the kernel.
> +Read by user-space with single-copy atomicity semantics.
> +This field should only be read by the thread which registered
> +this data structure.
> +Aligned on 32-bit.
> +.TP
> +.I mm_cid
> +Contains the current thread's concurrency ID
> +(allocated uniquely within a memory map).
> +.IP
> +Updated by the kernel.
> +Read by user-space with single-copy atomicity semantics.
> +This field should only be read by the thread which registered this data
> +structure.
> +Aligned on 32-bit.
> +.IP
> +This concurrency ID is within the possible cpus range,
> +and is temporarily (and uniquely) assigned while threads are actively
> +running within a memory map.
> +If a memory map has fewer threads than cores,
> +or is limited to run on few cores concurrently through sched affinity or
> +cgroup cpusets,
> +the concurrency IDs will be values close to 0,
> +thus allowing efficient use of user-space memory for per-cpu data
> +structures.
> +.RE
> +.PP
> +The layout of
> +.B struct rseq_cs
> +version 0 is as follows:
> +.TP
> +.B Structure alignment
> +This structure is aligned on 32-byte boundary.
> +.TP
> +.B Structure size
> +This structure has a fixed size of 32 bytes.
> +.in +4n
> +.IP
> +.EX
> +#include <linux/rseq.h>
> +
> +struct rseq_cs {
> + __u32 version;
> + __u32 flags;
> + __u64 start_ip;
> + __u64 post_commit_offset;
> + __u64 abort_ip;
> +} __attribute__((aligned(32)));
> +.EE
> +.in
> +.TP
> +.B Fields

Branden, IIRC, this seems to be the reason why I didn't want .RS for indenting code examples. It doesn't fit nicely right after TP.

Why is there a blank line? I'm not even sure that's reasonable. Is it a (minor) bug in man(7)? (FWIW, mandoc(1) is consistent with groff(1).)

> +.RS
> +.TP
> +.I version
> +Version of this structure.
> +Should be initialized to 0.
> +.TP
> +.I flags
> +.RS
> +Flags indicating the restart behavior of this structure.
> +Can be a combination of:
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
> +Inhibit instruction sequence block restart on preemption for this
> +critical section.
> +This flag is deprecated since Linux 6.1.
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
> +Inhibit instruction sequence block restart on signal delivery for this
> +critical section.
> +This flag is deprecated since Linux 6.1.
> +.TP
> +.B RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
> +Inhibit instruction sequence block restart on migration for this
> +critical section.
> +This flag is deprecated since Linux 6.1.
> +.RE
> +.TP
> +.I start_ip
> +Instruction pointer address of the first instruction of the sequence of
> +consecutive assembly instructions.
> +.TP
> +.I post_commit_offset
> +Offset (from start_ip address) of the address after the last instruction
> +of the sequence of consecutive assembly instructions.
> +.TP
> +.I abort_ip
> +Instruction pointer address where to move the execution flow in case of
> +abort of the sequence of consecutive assembly instructions.
> +.RE
> +.PP
> +The
> +.I rseq_len
> +argument is the size of the
> +.B struct rseq
> +to register.
> +.PP
> +The
> +.I flags
> +argument is 0 for registration, and
> +.B RSEQ_FLAG_UNREGISTER
> +for unregistration.
> +.PP
> +The
> +.I sig
> +argument is the 32-bit signature to be expected before the abort
> +handler code.
> +.PP
> +A single library per process should keep the
> +.B struct rseq
> +in a per-thread data structure.
> +The
> +.I cpu_id
> +field should be initialized to -1, and the
> +.I cpu_id_start
> +field should be initialized to a possible CPU value (typically 0).
> +.PP
> +Each thread is responsible for registering and unregistering its
> +.BR "struct rseq" .
> +No more than one
> +.B struct rseq
> +address can be registered per thread at a given time.
> +.PP
> +Reclaim of
> +.B struct rseq
> +object's memory must only be done after either an explicit rseq
> +unregistration is performed or after the thread exits.
> +.PP
> +In a typical usage scenario, the thread registering the
> +.B struct rseq
> +will be performing loads and stores from/to that structure.
> +It is however also allowed to read that structure from other threads.
> +The
> +.B struct rseq
> +field updates performed by the kernel provide relaxed atomicity
> +semantics (atomic store, without memory ordering),
> +which guarantee that other threads performing relaxed atomic reads
> +(atomic load, without memory ordering) of the cpu number fields will
> +always observe a consistent value.
> +.SH RETURN VALUE
> +A return value of 0 indicates success.
> +On error, \-1 is returned, and
> +.I errno
> +is set appropriately.

Michael tried to add some consistency to the RETURN VALUE (and EXIT STATUS) sections.
Please have a look at `git show 30b0d8d97e5e^..31e9088393e4`.

> +.SH ERRORS
> +.TP
> +.B EINVAL
> +Either
> +.I flags
> +contains an invalid value, or
> +.I rseq
> +contains an address which is not appropriately aligned, or
> +.I rseq_len
> +contains an incorrect size.
> +.TP
> +.B ENOSYS
> +The
> +.BR rseq ()
> +system call is not implemented by this kernel.
> +.TP
> +.B EFAULT
> +.I rseq
> +is an invalid address.
> +.TP
> +.B EBUSY
> +Restartable sequence is already registered for this thread.
> +.TP
> +.B EPERM
> +The
> +.I sig
> +argument on unregistration does not match the signature received
> +on registration.
> +.SH VERSIONS
> +The
> +.BR rseq ()
> +system call was added in Linux 4.18.
> +.SH STANDARDS
> +.BR rseq ()
> +is Linux-specific.
> +.SH SEE ALSO
> +.BR sched_getcpu (3) ,
> +.BR membarrier (2) ,
> +.BR getauxval (3)

Cheers,

Alex

--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


Attachments:
OpenPGP_signature (833.00 B)
OpenPGP digital signature

2023-02-15 01:21:15

by G. Branden Robinson

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

[CC list violently trimmed; for those who remain, this is mostly man
page style issues]

At 2023-02-14T23:29:37+0100, Alejandro Colomar wrote:
> On 2/14/23 20:54, Mathieu Desnoyers wrote:
> > +per-thread data structure shared between kernel and user-space.
>
> This last 'user-space' is not adjectivated, so it should go without
> a hyphen, according to common English rules.

+1

Also I like your coinage. "Adjectivated yeast" is reflexive and
tautological!

> > +.RB ( "struct rseq" )
>
> We format types in italics, so this should be '.RI'.

+1

> > +Only one
> > +.BR rseq ()
> > +ABI can be registered per thread, so user-space libraries and
> > +applications must follow a user-space ABI defining how to share this
> > +resource.
>
> Please use semantic newlines. See man-pages(7):
>
> Use semantic newlines
> In the source of a manual page, new sentences should be started on new
> lines, long sentences should be split into lines at clause breaks (com‐
> mas, semicolons, colons, and so on), and long clauses should be split
> at phrase boundaries. This convention, sometimes known as "semantic
> newlines", makes it easier to see the effect of patches, which often
> operate at the level of individual sentences, clauses, or phrases.

I think I've said this before, but, strictly, commas in particular can
separate things that are not clauses. Clauses have subjects and
predicates.

Might it be better to say simply:

Start each sentence on a new line. Split long sentences where
punctuated by commas, semicolons, and colons.

With this there is not even any need to discuss "phrase boundaries".

> In the above lines, that would mean breaking after the comma,
> and not leaving resource in a line of its own.

The latter is inevitably going to happen from time to time simply due to
sentence length and structure and the line length used by one's text
editor. I don't think an "orphan word" (what typographers call this) is
symptomatic of anything in *roff source when filling is enabled.

> > +The ABI defining how to share this resource between applications and
> > +libraries is defined by the C library.
> > +Allocation of the per-thread
> > +.BR rseq ()
> > +ABI and its registration to the kernel is handled by glibc since version
> > +2.35.
> > +.PP
> > +The
> > +.BR rseq ()
> > +ABI per-thread data structure contains a
> > +.I rseq_cs
> > +field which points to the currently executing critical section.
>
> currently-executing should probably use a hyphen
> (if I understood the line correctly).

This is not the case, according to some style authorities. Dave Kemper
convinced me of this on the groff list.

Here is one resource.

https://www.editorgroup.com/blog/to-hyphenate-or-not-to-hyphenate/

> See an interesting discussion in the groff@ mailing list:
> <https://lists.gnu.org/archive/html/groff/2022-10/msg00015.html>

That's not _squarely_ on point, as none of "block", "device", or "based"
is an adverb. "Currently" is.

> > +For each thread, a single rseq critical section can run at any given
> > +point.
> > +Each critical section need to be implemented in assembly.
>
> needs?

+1

> > +.TP
> > +.B Structure alignment
>
> Let's remove the bold here. It's not necessary for marking a constant
> or something that needs bold. And the indentation is already making
> it stand out, so bold is a bit too much aggressive to the reader.

I agree; if it wouldn't be styled in running text, it doesn't need
styling as a paragraph tag; it already stands out by dint of its
placement as a tag.

> > +Its value should always be confirmed by reading the cpu_id field before
>
> cpu_id should be formatted (.I).

+1

> > +user-space performs any side-effect
> > +(e.g. storing to memory).
> > +.IP
> > +This field is always guaranteed to hold a valid CPU number in the range
> > +[ 0 .. nr_possible_cpus - 1 ].
>
> Please use interval notation:
> [0, nr_possible_cpus)
> or
> [0, nr_possible_cpus - 1]
> whichever looks better to you.
>
> We did some consistency fix recently:
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>
> Also, do we have a more standard way of saying nr_possible_cpus?
> Should we say nproc?

Apropos of a separate discussion we had weeks ago, Alex, I remembered
where I saw "nitems" as a variable name.

_UNIX Programming_, second edition, by Kernighan and Ritchie!

https://wolfram.schneider.org/bsd/7thEdManVol2/uprog/uprog.pdf

Plop _that_ down on the desk of the person who claimed it was a stupid
variable name that no good programmer would ever use.

(I think appeals to authority are just fine as long as one is being
mean. ;-P And as regards variable naming, Kernighan is a _legitimate_
authority: a subject matter expert with multiple well-regarded books on
coding style to his credit. Ritchie's strengths were more esoteric,
enough that he put up specimens of his own youthful hubris as, I think,
an effort to discourage his many admirers from copying his mistakes as
slavishly as his successes[1]--apart from their humor value.)

> Branden, IIRC, this seems to be the reason why I didn't want .RS for
> indenting code examples. It doesn't fit nicely right after TP.
>
> Why is there a blank line? I'm not even sure that's reasonable. Is
> it a (minor) bug in man(7)? (FWIW, mandoc(1) is consistent with
> groff(1).)

Right, I'll take this up in the separate thread you started for it on
the groff list.

Regards,
Branden

[1] https://www.bell-labs.com/usr/dmr/www/odd.html

See particularly "Comments I do feel guilty about".


Attachments:
(No filename) (5.52 kB)
signature.asc (833.00 B)
Download all attachments

2023-02-15 01:52:47

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

Hi Branden,

On 2/15/23 02:20, G. Branden Robinson wrote:
> [CC list violently trimmed; for those who remain, this is mostly man
> page style issues]

Ironically, you trimmed linux-man@ :D

>
> At 2023-02-14T23:29:37+0100, Alejandro Colomar wrote:
>> On 2/14/23 20:54, Mathieu Desnoyers wrote:
>>> +per-thread data structure shared between kernel and user-space.
>>
>> This last 'user-space' is not adjectivated, so it should go without
>> a hyphen, according to common English rules.
>
> +1
>
> Also I like your coinage. "Adjectivated yeast" is reflexive and
> tautological!

:D

I didn't even think about it. "adjetivado" is an actual word in Spanish,
so I just guessed that it would exist in English. But yes, it's a nice
word for this case :)

>
>>> +.RB ( "struct rseq" )
>>
>> We format types in italics, so this should be '.RI'.
>
> +1
>
>>> +Only one
>>> +.BR rseq ()
>>> +ABI can be registered per thread, so user-space libraries and
>>> +applications must follow a user-space ABI defining how to share this
>>> +resource.
>>
>> Please use semantic newlines. See man-pages(7):
>>
>> Use semantic newlines
>> In the source of a manual page, new sentences should be started on new
>> lines, long sentences should be split into lines at clause breaks (com‐
>> mas, semicolons, colons, and so on), and long clauses should be split
>> at phrase boundaries. This convention, sometimes known as "semantic
>> newlines", makes it easier to see the effect of patches, which often
>> operate at the level of individual sentences, clauses, or phrases.
>
> I think I've said this before, but, strictly, commas in particular can
> separate things that are not clauses. Clauses have subjects and
> predicates.
>
> Might it be better to say simply:
>
> Start each sentence on a new line. Split long sentences where
> punctuated by commas, semicolons, and colons.
>
> With this there is not even any need to discuss "phrase boundaries".

I still disagree with that;
punctuation is not enough
(see below).

>
>> In the above lines, that would mean breaking after the comma,
>> and not leaving resource in a line of its own.
>
> The latter is inevitably going to happen from time to time simply due to
> sentence length and structure and the line length used by one's text
> editor.

That's if you only break at sentence and clause boundaries,
and then adjust at 80 columns...

> I don't think an "orphan word" (what typographers call this) is
> symptomatic of anything in *roff source when filling is enabled.

but if you also break at phrase boundaries
(when/where convenient)
you'll find that you get rid of those "orphan words",
because you'll never have to adjust at 80 columns
(or rather,
you'll try to find a way to break it
that also happens to be a phrase boundary
because breaking at phrase boundaries is _better_
than breaking at random points which may happen to be
in the middle of a compound noun).

>
>>> +The ABI defining how to share this resource between applications and
>>> +libraries is defined by the C library.
>>> +Allocation of the per-thread
>>> +.BR rseq ()
>>> +ABI and its registration to the kernel is handled by glibc since version
>>> +2.35.
>>> +.PP
>>> +The
>>> +.BR rseq ()
>>> +ABI per-thread data structure contains a
>>> +.I rseq_cs
>>> +field which points to the currently executing critical section.
>>
>> currently-executing should probably use a hyphen
>> (if I understood the line correctly).
>
> This is not the case, according to some style authorities. Dave Kemper
> convinced me of this on the groff list.
>
> Here is one resource.
>
> https://www.editorgroup.com/blog/to-hyphenate-or-not-to-hyphenate/

Hmm, interesting. I didn't think about it, but it makes sense
to not hyphenate here.

>
>> See an interesting discussion in the groff@ mailing list:
>> <https://lists.gnu.org/archive/html/groff/2022-10/msg00015.html>
>
> That's not _squarely_ on point, as none of "block", "device", or "based"
> is an adverb. "Currently" is.

You're right.

>
>>> +For each thread, a single rseq critical section can run at any given
>>> +point.
>>> +Each critical section need to be implemented in assembly.
>>
>> needs?
>
> +1
>
>>> +.TP
>>> +.B Structure alignment
>>
>> Let's remove the bold here. It's not necessary for marking a constant
>> or something that needs bold. And the indentation is already making
>> it stand out, so bold is a bit too much aggressive to the reader.
>
> I agree; if it wouldn't be styled in running text, it doesn't need
> styling as a paragraph tag; it already stands out by dint of its
> placement as a tag.
>
>>> +Its value should always be confirmed by reading the cpu_id field before
>>
>> cpu_id should be formatted (.I).
>
> +1
>
>>> +user-space performs any side-effect
>>> +(e.g. storing to memory).
>>> +.IP
>>> +This field is always guaranteed to hold a valid CPU number in the range
>>> +[ 0 .. nr_possible_cpus - 1 ].
>>
>> Please use interval notation:
>> [0, nr_possible_cpus)
>> or
>> [0, nr_possible_cpus - 1]
>> whichever looks better to you.
>>
>> We did some consistency fix recently:
>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>>
>> Also, do we have a more standard way of saying nr_possible_cpus?
>> Should we say nproc?
>
> Apropos of a separate discussion we had weeks ago, Alex, I remembered
> where I saw "nitems" as a variable name.
>
> _UNIX Programming_, second edition, by Kernighan and Ritchie!
>
> https://wolfram.schneider.org/bsd/7thEdManVol2/uprog/uprog.pdf
>
> Plop _that_ down on the desk of the person who claimed it was a stupid
> variable name that no good programmer would ever use.

ROFL. I'm bookmarking this!

>
> (I think appeals to authority are just fine as long as one is being
> mean. ;-P

Indeed ;-P

> And as regards variable naming, Kernighan is a _legitimate_
> authority: a subject matter expert with multiple well-regarded books on
> coding style to his credit. Ritchie's strengths were more esoteric,
> enough that he put up specimens of his own youthful hubris as, I think,
> an effort to discourage his many admirers from copying his mistakes as
> slavishly as his successes[1]--apart from their humor value.)
>
>> Branden, IIRC, this seems to be the reason why I didn't want .RS for
>> indenting code examples. It doesn't fit nicely right after TP.
>>
>> Why is there a blank line? I'm not even sure that's reasonable. Is
>> it a (minor) bug in man(7)? (FWIW, mandoc(1) is consistent with
>> groff(1).)
>
> Right, I'll take this up in the separate thread you started for it on
> the groff list.

Sure.

>
> Regards,
> Branden
>
> [1] https://www.bell-labs.com/usr/dmr/www/odd.html
>
> See particularly "Comments I do feel guilty about".

Yep, that page is always funny :)

Cheers,

Alex


--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


Attachments:
OpenPGP_signature (833.00 B)
OpenPGP digital signature

2023-02-15 02:21:59

by G. Branden Robinson

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

At 2023-02-15T02:52:03+0100, Alejandro Colomar wrote:
> On 2/15/23 02:20, G. Branden Robinson wrote:
> > [CC list violently trimmed; for those who remain, this is mostly man
> > page style issues]
>
> Ironically, you trimmed linux-man@ :D

I didn't! It wasn't present in the mail to which I repled.

This did puzzle me. I guess it was an oversight. You might want to
re-send that message of yours, and/or Mathieu's, if it lacked it too.

Or maybe it doesn't matter because lore.kernel.org finds all. I just
used it to track down an exchange between Michael Kerrisk and me that
GMail refused to find even though it was in my inbox. It showed me only
one thread, didn't highlight the specific message that it thought
matched, and showed me the _wrong_ thread on top of everything else.
The word "constraint" was in the thread I wanted, not in the one I
didn't, and even when I quoted it I was served up an incorrect match.

Clearly their AI efforts are going swimmingly.

Regards,
Branden


Attachments:
(No filename) (994.00 B)
signature.asc (833.00 B)
Download all attachments

2023-02-15 03:08:23

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

Hi Branden,

On 2/15/23 03:21, G. Branden Robinson wrote:
> At 2023-02-15T02:52:03+0100, Alejandro Colomar wrote:
>> On 2/15/23 02:20, G. Branden Robinson wrote:
>>> [CC list violently trimmed; for those who remain, this is mostly man
>>> page style issues]
>>
>> Ironically, you trimmed linux-man@ :D
>
> I didn't! It wasn't present in the mail to which I repled.

Hmm, you're right, Mathieu didn't CC linux-man@. I guessed somewhere
in that big list it would be there, but it wasn't. Thanks for CCing it.

>
> This did puzzle me. I guess it was an oversight. You might want to
> re-send that message of yours, and/or Mathieu's, if it lacked it too.
>
> Or maybe it doesn't matter because lore.kernel.org finds all. I just
> used it to track down an exchange between Michael Kerrisk and me that
> GMail refused to find even though it was in my inbox. It showed me only
> one thread, didn't highlight the specific message that it thought
> matched, and showed me the _wrong_ thread on top of everything else.
> The word "constraint" was in the thread I wanted, not in the one I
> didn't, and even when I quoted it I was served up an incorrect match.

Which reminds me that I hate searching in the groff@ archives. It's not
because of the search engine, but because of the thread view. You are
artificially restricted to a given month, and you can't see entire threads
in the search engine. Is there anything similar to lore for groff@?
Other GNU projects can now be searched at <https://inbox.sourceware.org/>
such as <https://inbox.sourceware.org/libc-alpha/>, but groff@ isn't there
:(

Cheers,

Alex

>
> Clearly their AI efforts are going swimmingly.>
> Regards,
> Branden

--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


Attachments:
OpenPGP_signature (833.00 B)
OpenPGP digital signature

2023-02-15 16:44:34

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

On 2023-02-14 22:07, Alejandro Colomar wrote:
> Hi Branden,
>
> On 2/15/23 03:21, G. Branden Robinson wrote:
>> At 2023-02-15T02:52:03+0100, Alejandro Colomar wrote:
>>> On 2/15/23 02:20, G. Branden Robinson wrote:
>>>> [CC list violently trimmed; for those who remain, this is mostly man
>>>> page style issues]
>>>
>>> Ironically, you trimmed linux-man@ :D
>>
>> I didn't! It wasn't present in the mail to which I repled.
>
> Hmm, you're right, Mathieu didn't CC linux-man@. I guessed somewhere
> in that big list it would be there, but it wasn't. Thanks for CCing it.
>
>>
>> This did puzzle me. I guess it was an oversight. You might want to
>> re-send that message of yours, and/or Mathieu's, if it lacked it too.

My bad. The list I used was from a script I last updated when I was
trying to get Michael's attention a few years ago. I've trimmed it and
included linux-man@.

I will use that new email list for my next post taking your comments
into account.

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


2023-02-15 17:09:28

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

On 2023-02-14 20:20, G. Branden Robinson wrote:
> [CC list violently trimmed; for those who remain, this is mostly man
> page style issues]

[ Gently added linux-man to CC list. ;-) ]

>
> At 2023-02-14T23:29:37+0100, Alejandro Colomar wrote:
>> On 2/14/23 20:54, Mathieu Desnoyers wrote:
>>> +per-thread data structure shared between kernel and user-space.
>>
>> This last 'user-space' is not adjectivated, so it should go without
>> a hyphen, according to common English rules.
>
> +1

done


>
> Also I like your coinage. "Adjectivated yeast" is reflexive and
> tautological!
>
>>> +.RB ( "struct rseq" )
>>
>> We format types in italics, so this should be '.RI'.
>
> +1

OK, so it's italics for both types and arguments.

I will replace all the bold markers for "struct rseq" and "struct
rseq_cs" to italic in the description (but not in the synopsis section
and not in the code snippets).

>
>>> +Only one
>>> +.BR rseq ()
>>> +ABI can be registered per thread, so user-space libraries and
>>> +applications must follow a user-space ABI defining how to share this
>>> +resource.
>>
>> Please use semantic newlines. See man-pages(7):
>>
>> Use semantic newlines
>> In the source of a manual page, new sentences should be started on new
>> lines, long sentences should be split into lines at clause breaks (com‐
>> mas, semicolons, colons, and so on), and long clauses should be split
>> at phrase boundaries. This convention, sometimes known as "semantic
>> newlines", makes it easier to see the effect of patches, which often
>> operate at the level of individual sentences, clauses, or phrases.
>
> I think I've said this before, but, strictly, commas in particular can
> separate things that are not clauses. Clauses have subjects and
> predicates.
>
> Might it be better to say simply:
>
> Start each sentence on a new line. Split long sentences where
> punctuated by commas, semicolons, and colons.
>
> With this there is not even any need to discuss "phrase boundaries".
>

I've modified to:

Only one
.BR rseq ()
ABI can be registered per thread,
so user-space libraries and applications must follow a user-space ABI
defining how to share this resource.

Hopefully that's correct.


>> In the above lines, that would mean breaking after the comma,
>> and not leaving resource in a line of its own.
>
> The latter is inevitably going to happen from time to time simply due to
> sentence length and structure and the line length used by one's text
> editor. I don't think an "orphan word" (what typographers call this) is
> symptomatic of anything in *roff source when filling is enabled.
>
>>> +The ABI defining how to share this resource between applications and
>>> +libraries is defined by the C library.
>>> +Allocation of the per-thread
>>> +.BR rseq ()
>>> +ABI and its registration to the kernel is handled by glibc since version
>>> +2.35.
>>> +.PP
>>> +The
>>> +.BR rseq ()
>>> +ABI per-thread data structure contains a
>>> +.I rseq_cs
>>> +field which points to the currently executing critical section.
>>
>> currently-executing should probably use a hyphen
>> (if I understood the line correctly).
>
> This is not the case, according to some style authorities. Dave Kemper
> convinced me of this on the groff list.
>
> Here is one resource.
>
> https://www.editorgroup.com/blog/to-hyphenate-or-not-to-hyphenate/
>
>> See an interesting discussion in the groff@ mailing list:
>> <https://lists.gnu.org/archive/html/groff/2022-10/msg00015.html>
>
> That's not _squarely_ on point, as none of "block", "device", or "based"
> is an adverb. "Currently" is.

Leaving unchanged based on this discussion.

>
>>> +For each thread, a single rseq critical section can run at any given
>>> +point.
>>> +Each critical section need to be implemented in assembly.
>>
>> needs?
>
> +1

done.

>
>>> +.TP
>>> +.B Structure alignment
>>
>> Let's remove the bold here. It's not necessary for marking a constant
>> or something that needs bold. And the indentation is already making
>> it stand out, so bold is a bit too much aggressive to the reader.
>
> I agree; if it wouldn't be styled in running text, it doesn't need
> styling as a paragraph tag; it already stands out by dint of its
> placement as a tag.
>
>>> +Its value should always be confirmed by reading the cpu_id field before
>>
>> cpu_id should be formatted (.I).
>
> +1

done

>
>>> +user-space performs any side-effect
>>> +(e.g. storing to memory).
>>> +.IP
>>> +This field is always guaranteed to hold a valid CPU number in the range
>>> +[ 0 .. nr_possible_cpus - 1 ].
>>
>> Please use interval notation:
>> [0, nr_possible_cpus)
>> or
>> [0, nr_possible_cpus - 1]
>> whichever looks better to you.
>>
>> We did some consistency fix recently:
>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>>
>> Also, do we have a more standard way of saying nr_possible_cpus?
>> Should we say nproc?

nproc(1) means:

Print the number of processing units available to the current
process, which may be less than the number of online processors

Which is the number of cpus currently available (AFAIU the result of the
cpuset and sched affinity).

What I really mean here is the maximum value for possible cpus which can
be hotplugged into the system. So it's not the maximum number of
possible CPUs per se, but rather the maximum enabled bit in the possible
CPUs mask.

Note that we could express this differently as well: rather than saying
that it guarantees a value in the range [0, nr_possible_cpus - 1], we
could say that the values are guaranteed to be part of the possible cpus
mask, which would actually more accurate in case the possible cpus mask
has a hole (it tends to happen with things like lxc containers nowadays).

Do you agree that we should favor expressing this in terms of belonging
to the possible cpumask set rather than a range starting from 0 ?

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


2023-02-15 17:12:59

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

On 2023-02-15 12:09, Mathieu Desnoyers wrote:
> On 2023-02-14 20:20, G. Branden Robinson wrote:
[...]
>>
>>>> +user-space performs any side-effect
>>>> +(e.g. storing to memory).
>>>> +.IP
>>>> +This field is always guaranteed to hold a valid CPU number in the
>>>> range
>>>> +[ 0 ..  nr_possible_cpus - 1 ].
>>>
>>> Please use interval notation:
>>>     [0, nr_possible_cpus)
>>> or
>>>     [0, nr_possible_cpus - 1]
>>> whichever looks better to you.
>>>
>>> We did some consistency fix recently:
>>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>>>
>>> Also, do we have a more standard way of saying nr_possible_cpus?
>>> Should we say nproc?
>
> nproc(1) means:
>
>        Print  the number of processing units available to the current
>        process, which may be less than the number of online processors
>
> Which is the number of cpus currently available (AFAIU the result of the
> cpuset and sched affinity).
>
> What I really mean here is the maximum value for possible cpus which can
> be hotplugged into the system. So it's not the maximum number of
> possible CPUs per se, but rather the maximum enabled bit in the possible
> CPUs mask.
>
> Note that we could express this differently as well: rather than saying
> that it guarantees a value in the range [0, nr_possible_cpus - 1], we
> could say that the values are guaranteed to be part of the possible cpus
> mask, which would actually more accurate in case the possible cpus mask
> has a hole (it tends to happen with things like lxc containers nowadays).
>
> Do you agree that we should favor expressing this in terms of belonging
> to the possible cpumask set rather than a range starting from 0 ?

Actually, the field may contain the value 0 even if 0 is not part of the
possible cpumask. So forget what I just said about being guaranteed to
be part of the possible cpus mask.

Thoughts ?

Thanks,

Mathieu

>
> Thanks,
>
> Mathieu
>

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


2023-02-15 17:17:18

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

Hi Mathieu,

On 2/15/23 18:09, Mathieu Desnoyers wrote:
> On 2023-02-14 20:20, G. Branden Robinson wrote:
>> [CC list violently trimmed; for those who remain, this is mostly man
>> page style issues]
>
> [ Gently added linux-man to CC list. ;-) ]

:-)

>
>>
>> At 2023-02-14T23:29:37+0100, Alejandro Colomar wrote:
>>> On 2/14/23 20:54, Mathieu Desnoyers wrote:
>>>> +per-thread data structure shared between kernel and user-space.
>>>
>>> This last 'user-space' is not adjectivated, so it should go without
>>> a hyphen, according to common English rules.
>>
>> +1
>
> done
>
>
>>
>> Also I like your coinage. "Adjectivated yeast" is reflexive and
>> tautological!
>>
>>>> +.RB ( "struct rseq" )
>>>
>>> We format types in italics, so this should be '.RI'.
>>
>> +1
>
> OK, so it's italics for both types and arguments.
>
> I will replace all the bold markers for "struct rseq" and "struct
> rseq_cs" to italic in the description (but not in the synopsis section
> and not in the code snippets).
>
>>
>>>> +Only one
>>>> +.BR rseq ()
>>>> +ABI can be registered per thread, so user-space libraries and
>>>> +applications must follow a user-space ABI defining how to share this
>>>> +resource.
>>>
>>> Please use semantic newlines. See man-pages(7):
>>>
>>> Use semantic newlines
>>> In the source of a manual page, new sentences should be started on new
>>> lines, long sentences should be split into lines at clause breaks (com‐
>>> mas, semicolons, colons, and so on), and long clauses should be split
>>> at phrase boundaries. This convention, sometimes known as "semantic
>>> newlines", makes it easier to see the effect of patches, which often
>>> operate at the level of individual sentences, clauses, or phrases.
>>
>> I think I've said this before, but, strictly, commas in particular can
>> separate things that are not clauses. Clauses have subjects and
>> predicates.
>>
>> Might it be better to say simply:
>>
>> Start each sentence on a new line. Split long sentences where
>> punctuated by commas, semicolons, and colons.
>>
>> With this there is not even any need to discuss "phrase boundaries".
>>
>
> I've modified to:
>
> Only one
> .BR rseq ()
> ABI can be registered per thread,
> so user-space libraries and applications must follow a user-space ABI
> defining how to share this resource.
>
> Hopefully that's correct.

Yes, that's good.

>
>
>>> In the above lines, that would mean breaking after the comma,
>>> and not leaving resource in a line of its own.
>>
>> The latter is inevitably going to happen from time to time simply due to
>> sentence length and structure and the line length used by one's text
>> editor. I don't think an "orphan word" (what typographers call this) is
>> symptomatic of anything in *roff source when filling is enabled.
>>
>>>> +The ABI defining how to share this resource between applications and
>>>> +libraries is defined by the C library.
>>>> +Allocation of the per-thread
>>>> +.BR rseq ()
>>>> +ABI and its registration to the kernel is handled by glibc since version
>>>> +2.35.
>>>> +.PP
>>>> +The
>>>> +.BR rseq ()
>>>> +ABI per-thread data structure contains a
>>>> +.I rseq_cs
>>>> +field which points to the currently executing critical section.
>>>
>>> currently-executing should probably use a hyphen
>>> (if I understood the line correctly).
>>
>> This is not the case, according to some style authorities. Dave Kemper
>> convinced me of this on the groff list.
>>
>> Here is one resource.
>>
>> https://www.editorgroup.com/blog/to-hyphenate-or-not-to-hyphenate/
>>
>>> See an interesting discussion in the groff@ mailing list:
>>> <https://lists.gnu.org/archive/html/groff/2022-10/msg00015.html>
>>
>> That's not _squarely_ on point, as none of "block", "device", or "based"
>> is an adverb. "Currently" is.
>
> Leaving unchanged based on this discussion.
>
>>
>>>> +For each thread, a single rseq critical section can run at any given
>>>> +point.
>>>> +Each critical section need to be implemented in assembly.
>>>
>>> needs?
>>
>> +1
>
> done.
>
>>
>>>> +.TP
>>>> +.B Structure alignment
>>>
>>> Let's remove the bold here. It's not necessary for marking a constant
>>> or something that needs bold. And the indentation is already making
>>> it stand out, so bold is a bit too much aggressive to the reader.
>>
>> I agree; if it wouldn't be styled in running text, it doesn't need
>> styling as a paragraph tag; it already stands out by dint of its
>> placement as a tag.
>>
>>>> +Its value should always be confirmed by reading the cpu_id field before
>>>
>>> cpu_id should be formatted (.I).
>>
>> +1
>
> done
>
>>
>>>> +user-space performs any side-effect
>>>> +(e.g. storing to memory).
>>>> +.IP
>>>> +This field is always guaranteed to hold a valid CPU number in the range
>>>> +[ 0 .. nr_possible_cpus - 1 ].
>>>
>>> Please use interval notation:
>>> [0, nr_possible_cpus)
>>> or
>>> [0, nr_possible_cpus - 1]
>>> whichever looks better to you.
>>>
>>> We did some consistency fix recently:
>>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>>>
>>> Also, do we have a more standard way of saying nr_possible_cpus?
>>> Should we say nproc?
>
> nproc(1) means:
>
> Print the number of processing units available to the current
> process, which may be less than the number of online processors
>
> Which is the number of cpus currently available (AFAIU the result of the
> cpuset and sched affinity).
>
> What I really mean here is the maximum value for possible cpus which can
> be hotplugged into the system. So it's not the maximum number of
> possible CPUs per se, but rather the maximum enabled bit in the possible
> CPUs mask.
>
> Note that we could express this differently as well: rather than saying
> that it guarantees a value in the range [0, nr_possible_cpus - 1], we
> could say that the values are guaranteed to be part of the possible cpus
> mask, which would actually more accurate in case the possible cpus mask
> has a hole (it tends to happen with things like lxc containers nowadays).
>
> Do you agree that we should favor expressing this in terms of belonging
> to the possible cpumask set rather than a range starting from 0 ?

On 2/15/23 18:12, Mathieu Desnoyers wrote:
> Actually, the field may contain the value 0 even if 0 is not part of the
> possible cpumask. So forget what I just said about being guaranteed to
> be part of the possible cpus mask.
>
> Thoughts ?
>
> Thanks,
>
> Mathieu

I don't have a full understanding, so I will trust you for deciding what is
best. I'll try to understand it, but my kernel knowledge is rather limited :)

I suggest writing a detailed description, instead of (or complementary to it)
just using a range, since readers might wonder as I did, what nr_possible_cpus
is (it's not really described anywhere so far). With a worded description,
we can later improve it if we find it not clear enough, but should be enough
for an initial page.

Thanks!

Alex

>
> Thanks,
>
> Mathieu
>

--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


Attachments:
OpenPGP_signature (833.00 B)
OpenPGP digital signature

2023-02-15 18:59:00

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 1/1] rseq.2: New man page for the rseq(2) API

On 2023-02-15 12:16, Alejandro Colomar wrote:
[...]
>>>
>>>>> +user-space performs any side-effect
>>>>> +(e.g. storing to memory).
>>>>> +.IP
>>>>> +This field is always guaranteed to hold a valid CPU number in the range
>>>>> +[ 0 .. nr_possible_cpus - 1 ].
>>>>
>>>> Please use interval notation:
>>>> [0, nr_possible_cpus)
>>>> or
>>>> [0, nr_possible_cpus - 1]
>>>> whichever looks better to you.
>>>>
>>>> We did some consistency fix recently:
>>>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=147a60d792a5db8f3cb93ea16eefb73e16c1fb91>
>>>>
>>>> Also, do we have a more standard way of saying nr_possible_cpus?
>>>> Should we say nproc?
>>
>> nproc(1) means:
>>
>> Print the number of processing units available to the current
>> process, which may be less than the number of online processors
>>
>> Which is the number of cpus currently available (AFAIU the result of the
>> cpuset and sched affinity).
>>
>> What I really mean here is the maximum value for possible cpus which can
>> be hotplugged into the system. So it's not the maximum number of
>> possible CPUs per se, but rather the maximum enabled bit in the possible
>> CPUs mask.
>>
>> Note that we could express this differently as well: rather than saying
>> that it guarantees a value in the range [0, nr_possible_cpus - 1], we
>> could say that the values are guaranteed to be part of the possible cpus
>> mask, which would actually more accurate in case the possible cpus mask
>> has a hole (it tends to happen with things like lxc containers nowadays).
>>
>> Do you agree that we should favor expressing this in terms of belonging
>> to the possible cpumask set rather than a range starting from 0 ?
>
> On 2/15/23 18:12, Mathieu Desnoyers wrote:
>> Actually, the field may contain the value 0 even if 0 is not part of the
>> possible cpumask. So forget what I just said about being guaranteed to
>> be part of the possible cpus mask.
>>
>> Thoughts ?
>>
>> Thanks,
>>
>> Mathieu
>
> I don't have a full understanding, so I will trust you for deciding what is
> best. I'll try to understand it, but my kernel knowledge is rather limited :)
>
> I suggest writing a detailed description, instead of (or complementary to it)
> just using a range, since readers might wonder as I did, what nr_possible_cpus
> is (it's not really described anywhere so far). With a worded description,
> we can later improve it if we find it not clear enough, but should be enough
> for an initial page.

After giving it some thoughts, I think the most precise description
would be that the cpu number is guaranteed to be either 0, or the CPU
number on which the registered thread is running. Let's not bring in
notions of possible cpus (those come from
/sys/devices/system/cpu/possible) unless it's absolutely required.

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com