[PATCH v2 0/1] ioctl_epoll.2: Add epoll ioctl documentation

[PATCH v2 0/1] ioctl_epoll.2: Add epoll ioctl documentation

[Joe Damato]

Greetings:

Welcome to v2.

This revision fixes several style and formatting issues in the previous
revision and adds link pages for EPIOCSPARAMS and EPIOCGPARAMS.

Thanks,
Joe

v1->v2:
  - Add link pages for EPIOCSPARAMS and EPIOCGPARAMS.
  - Many edits to the main ioctl_epoll file to better describe the
    operations and document the interface.

 man/man2/epoll_create.2           |   1 +
 man/man2/epoll_ctl.2              |   1 +
 man/man2/ioctl.2                  |   1 +
 man/man2/ioctl_epoll.2            | 171 ++++++++++++++++++++++++++++++
 man/man2const/EPIOCGPARAMS.2const |   1 +
 man/man2const/EPIOCSPARAMS.2const |   1 +
 man/man7/epoll.7                  |   1 +
 7 files changed, 177 insertions(+)
 create mode 100644 man/man2/ioctl_epoll.2
 create mode 100644 man/man2const/EPIOCGPARAMS.2const
 create mode 100644 man/man2const/EPIOCSPARAMS.2const

-- 
2.34.1

[PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Joe Damato]

A new page is added which describes epoll fd ioctls: EPIOCSPARAMS and
EPIOCGPARAMS which allow the user to control epoll-based busy polling.

Also add link pages for EPIOCSPARAMS and EPIOCGPARAMS.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 man/man2/epoll_create.2           |   1 +
 man/man2/epoll_ctl.2              |   1 +
 man/man2/ioctl.2                  |   1 +
 man/man2/ioctl_epoll.2            | 171 ++++++++++++++++++++++++++++++
 man/man2const/EPIOCGPARAMS.2const |   1 +
 man/man2const/EPIOCSPARAMS.2const |   1 +
 man/man7/epoll.7                  |   1 +
 7 files changed, 177 insertions(+)
 create mode 100644 man/man2/ioctl_epoll.2
 create mode 100644 man/man2const/EPIOCGPARAMS.2const
 create mode 100644 man/man2const/EPIOCSPARAMS.2const

diff --git a/man/man2/epoll_create.2 b/man/man2/epoll_create.2
index f0327e8ba..2aa1745f5 100644
--- a/man/man2/epoll_create.2
+++ b/man/man2/epoll_create.2
@@ -141,4 +141,5 @@ on overrun.
 .BR close (2),
 .BR epoll_ctl (2),
 .BR epoll_wait (2),
+.BR ioctl_epoll (2),
 .BR epoll (7)
diff --git a/man/man2/epoll_ctl.2 b/man/man2/epoll_ctl.2
index 6d5bc032e..24bbe7405 100644
--- a/man/man2/epoll_ctl.2
+++ b/man/man2/epoll_ctl.2
@@ -425,5 +425,6 @@ flag.
 .SH SEE ALSO
 .BR epoll_create (2),
 .BR epoll_wait (2),
+.BR ioctl_epoll (2),
 .BR poll (2),
 .BR epoll (7)
diff --git a/man/man2/ioctl.2 b/man/man2/ioctl.2
index 5b8c28a9c..d96777d1f 100644
--- a/man/man2/ioctl.2
+++ b/man/man2/ioctl.2
@@ -225,6 +225,7 @@ for the various architectures.
 .BR ioctl_ns (2),
 .BR ioctl_tty (2),
 .BR ioctl_userfaultfd (2),
+.BR ioctl_epoll (2),
 .BR open (2),
 .\" .BR mt (4),
 .BR sd (4),
diff --git a/man/man2/ioctl_epoll.2 b/man/man2/ioctl_epoll.2
new file mode 100644
index 000000000..458e72e9a
--- /dev/null
+++ b/man/man2/ioctl_epoll.2
@@ -0,0 +1,171 @@
+.\" Copyright (c) 2024, Joe Damato
+.\" Written by Joe Damato <jdamato@fastly.com>
+.\"
+.\" SPDX-License-Identifier: Linux-man-pages-copyleft
+.\"
+.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
+.SH NAME
+ioctl_epoll \- ioctl() operations for epoll file descriptors
+.SH LIBRARY
+Standard C library
+.RI ( libc ", " \-lc )
+.SH SYNOPSIS
+.EX
+.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
+.B "#include <sys/ioctl.h>"
+.P
+.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
+.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
+.P
+.B struct epoll_params {
+.BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
+.BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
+.BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
+\&
+.BR " " "   /* pad the struct to a multiple of 64bits */"
+.BR "    uint8_t __pad;"            "  /* Must be zero */"
+.B };
+.EE
+.SH DESCRIPTION
+.TP
+.B EPIOCSPARAMS
+Set the
+.I epoll_params
+structure to configure the operation of epoll. Refer to the structure
+description below to learn what configuration is
+supported.
+.TP
+.B EPIOCGPARAMS
+Get the current
+.I epoll_params
+configuration settings.
+.TP
+All
+.BR ioctl (2)
+operations documented above must be performed on an epoll file descriptor,
+which can be created with a call to
+.BR epoll_create (2)
+or
+.BR epoll_create1 (2).
+.\" kernel commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
+.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
+.P
+.I argp.busy_poll_usecs
+field denotes the number of microseconds that the network stack will busy
+poll. During this time period, the network device will be polled
+repeatedly for packets. This value cannot exceed
+.B INT_MAX.
+.in
+.P
+.I argp.busy_poll_budget
+field denotes the maximum number of packets that the network stack will
+retrieve on each poll attempt. This value cannot exceed
+.B NAPI_POLL_WEIGHT
+which, as of Linux 6.9, is 64, unless the process is run with
+.B CAP_NET_ADMIN.
+.P
+.I argp.prefer_busy_poll
+field is a boolean field and must be either 0 (disabled) or 1 (enabled). If
+enabled, this indicates to the network stack that busy poll is the
+preferred method of processing network data and the network stack should
+give the application the opportunity to busy poll. Without this option,
+very busy systems may continue to do network processing via the normal
+method of IRQs triggering softIRQ and NAPI.
+.P
+.I argp.__pad
+must be zero.
+.P
+.SH RETURN VALUE
+On success, 0 is returned.
+On failure, \-1 is returned, and
+.I errno
+is set to indicate the error.
+.SH ERRORS
+.TP
+.B EOPNOTSUPP
+The kernel was not compiled with busy poll support.
+.TP
+.B EINVAL
+.I fd
+is not a valid file descriptor.
+.TP
+.B EINVAL
+.I op
+specified is invalid.
+.TP
+.B EINVAL
+.I argp.__pad
+was not zero.
+.TP
+.B EINVAL
+.I argp.busy_poll_usecs
+exceeds
+.B INT_MAX .
+.TP
+.B EINVAL
+.I argp.prefer_busy_poll
+is not 0 or 1.
+.TP
+.B EPERM
+The process is being run without
+.I CAP_NET_ADMIN
+and the specified
+.I argp.busy_poll_budget
+exceeds
+.B NAPI_POLL_WEIGHT
+(which is 64 as of Linux 6.9).
+.TP
+.B EFAULT
+.I argp
+does not point to a valid memory address.
+.SH EXAMPLES
+.EX
+/* Code to set the epoll params to enable busy polling */
+\&
+int epollfd = epoll_create1(0);
+struct epoll_params params;
+\&
+if (epollfd == \-1) {
+    perror("epoll_create1");
+    exit(EXIT_FAILURE);
+}
+\&
+memset(&params, 0, sizeof(struct epoll_params));
+\&
+params.busy_poll_usecs = 25;
+params.busy_poll_budget = 8;
+params.prefer_busy_poll = 1;
+\&
+if (ioctl(epollfd, EPIOCSPARAMS, &params) == \-1) {
+    perror("ioctl");
+    exit(EXIT_FAILURE);
+}
+\&
+/* Code to show how to retrieve the current settings */
+\&
+memset(&params, 0, sizeof(struct epoll_params));
+\&
+if (ioctl(epollfd, EPIOCGPARAMS, &params) == \-1) {
+    perror("ioctl");
+    exit(EXIT_FAILURE);
+}
+\&
+/* params struct now contains the current parameters */
+\&
+fprintf(stderr, "epoll usecs: %lu\\n", params.busy_poll_usecs);
+fprintf(stderr, "epoll packet budget: %u\\n", params.busy_poll_budget);
+fprintf(stderr, "epoll prefer busy poll: %u\\n", params.prefer_busy_poll);
+\&
+.SH History
+Linux 6.9, glibc 2.40.
+.SH SEE ALSO
+.BR ioctl (2),
+.BR epoll_create (2),
+.BR epoll_create1 (2),
+.BR epoll (7)
+.P
+.I linux.git/Documentation/networking/napi.rst
+.P
+and
+.P
+.I linux.git/Documentation/admin-guide/sysctl/net.rst
diff --git a/man/man2const/EPIOCGPARAMS.2const b/man/man2const/EPIOCGPARAMS.2const
new file mode 100644
index 000000000..6fbc5f0f8
--- /dev/null
+++ b/man/man2const/EPIOCGPARAMS.2const
@@ -0,0 +1 @@
+.so man2/ioctl_epoll.2
diff --git a/man/man2const/EPIOCSPARAMS.2const b/man/man2const/EPIOCSPARAMS.2const
new file mode 100644
index 000000000..6fbc5f0f8
--- /dev/null
+++ b/man/man2const/EPIOCSPARAMS.2const
@@ -0,0 +1 @@
+.so man2/ioctl_epoll.2
diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
index e7892922e..4ad032bdd 100644
--- a/man/man7/epoll.7
+++ b/man/man7/epoll.7
@@ -606,5 +606,6 @@ is present in an epoll instance.
 .BR epoll_create1 (2),
 .BR epoll_ctl (2),
 .BR epoll_wait (2),
+.BR ioctl_epoll (2),
 .BR poll (2),
 .BR select (2)
-- 
2.34.1

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 10890 bytes --]

Hi Joe,

On Mon, Jun 10, 2024 at 11:12:06PM GMT, Joe Damato wrote:
> A new page is added which describes epoll fd ioctls: EPIOCSPARAMS and
> EPIOCGPARAMS which allow the user to control epoll-based busy polling.
> 
> Also add link pages for EPIOCSPARAMS and EPIOCGPARAMS.
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>

Thanks!

> ---
>  man/man2/epoll_create.2           |   1 +
>  man/man2/epoll_ctl.2              |   1 +
>  man/man2/ioctl.2                  |   1 +
>  man/man2/ioctl_epoll.2            | 171 ++++++++++++++++++++++++++++++

I'm working on a general refactor of all ioctl manual pages, and I'm
making the pages have a name consistent with the UAPI header that
provides them.  You can see the progress here:
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=ioctl>

For consistency, please rename the page as ioctl_eventpoll(2).

>  man/man2const/EPIOCGPARAMS.2const |   1 +
>  man/man2const/EPIOCSPARAMS.2const |   1 +
>  man/man7/epoll.7                  |   1 +
>  7 files changed, 177 insertions(+)
>  create mode 100644 man/man2/ioctl_epoll.2
>  create mode 100644 man/man2const/EPIOCGPARAMS.2const
>  create mode 100644 man/man2const/EPIOCSPARAMS.2const
> 
> diff --git a/man/man2/epoll_create.2 b/man/man2/epoll_create.2
> index f0327e8ba..2aa1745f5 100644
> --- a/man/man2/epoll_create.2
> +++ b/man/man2/epoll_create.2
> @@ -141,4 +141,5 @@ on overrun.
>  .BR close (2),
>  .BR epoll_ctl (2),
>  .BR epoll_wait (2),
> +.BR ioctl_epoll (2),
>  .BR epoll (7)
> diff --git a/man/man2/epoll_ctl.2 b/man/man2/epoll_ctl.2
> index 6d5bc032e..24bbe7405 100644
> --- a/man/man2/epoll_ctl.2
> +++ b/man/man2/epoll_ctl.2
> @@ -425,5 +425,6 @@ flag.
>  .SH SEE ALSO
>  .BR epoll_create (2),
>  .BR epoll_wait (2),
> +.BR ioctl_epoll (2),
>  .BR poll (2),
>  .BR epoll (7)
> diff --git a/man/man2/ioctl.2 b/man/man2/ioctl.2
> index 5b8c28a9c..d96777d1f 100644
> --- a/man/man2/ioctl.2
> +++ b/man/man2/ioctl.2
> @@ -225,6 +225,7 @@ for the various architectures.
>  .BR ioctl_ns (2),
>  .BR ioctl_tty (2),
>  .BR ioctl_userfaultfd (2),
> +.BR ioctl_epoll (2),
>  .BR open (2),
>  .\" .BR mt (4),
>  .BR sd (4),
> diff --git a/man/man2/ioctl_epoll.2 b/man/man2/ioctl_epoll.2
> new file mode 100644
> index 000000000..458e72e9a
> --- /dev/null
> +++ b/man/man2/ioctl_epoll.2
> @@ -0,0 +1,171 @@
> +.\" Copyright (c) 2024, Joe Damato

Please reformat as:

.\" Copyright 2024, Joe Damato <jdamato@fastly.com>

(or another email if you want, but that's the format I'm trying to use
consistently.)

> +.\" Written by Joe Damato <jdamato@fastly.com>

You can remove this line.

> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.\"
> +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> +.SH NAME
> +ioctl_epoll \- ioctl() operations for epoll file descriptors

Please reformat as:

ioctl_eventpoll,
EPIOCSPARAMS,
EPIOCGPARAMS
\-
ioctl() operations for epoll file descriptors

(which has '\-' on a line of its own, and has the individual ops
listed.)

> +.SH LIBRARY
> +Standard C library
> +.RI ( libc ", " \-lc )
> +.SH SYNOPSIS
> +.EX
> +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"

Remove ' and struct ...' from that comment.  We only have constants in
those comments (except in a few pages, where I'm fixing it at the
moment).

> +.B "#include <sys/ioctl.h>"
> +.P
> +.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> +.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );

To document the header that provides this structure, let's add here:

.P
.B #include <linux/eventpoll.h>

> +.P
> +.B struct epoll_params {
> +.BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
> +.BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
> +.BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
> +\&
> +.BR " " "   /* pad the struct to a multiple of 64bits */"
> +.BR "    uint8_t __pad;"            "  /* Must be zero */"
> +.B };
> +.EE
> +.SH DESCRIPTION
> +.TP
> +.B EPIOCSPARAMS
> +Set the
> +.I epoll_params
> +structure to configure the operation of epoll. Refer to the structure

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

$ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p'
   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  (commas,  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.

> +description below to learn what configuration is
> +supported.
> +.TP
> +.B EPIOCGPARAMS
> +Get the current
> +.I epoll_params
> +configuration settings.
> +.TP
> +All
> +.BR ioctl (2)

We can omit 'ioctl(2)' here, since it's obvious from the context, I
think.  How about 'All operations documented ...'?

> +operations documented above must be performed on an epoll file descriptor,
> +which can be created with a call to

s/created/obtained/?

> +.BR epoll_create (2)
> +or
> +.BR epoll_create1 (2).
> +.\" kernel commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61

Let's reformat these:

.\" linux.git 18e2bf0edf4dd88d9656ec92395aa47392e85b61
.\" glibc.git 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158

(maybe say linux.git commit 1234...?  What do you prefer?)

> +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> +.P

I would use a subsection (.SS) for documenting the structure.

> +.I argp.busy_poll_usecs
> +field denotes the number of microseconds that the network stack will busy

s/field //?

> +poll. During this time period, the network device will be polled
> +repeatedly for packets. This value cannot exceed
> +.B INT_MAX.
> +.in
> +.P
> +.I argp.busy_poll_budget
> +field denotes the maximum number of packets that the network stack will

s/field //?

> +retrieve on each poll attempt. This value cannot exceed
> +.B NAPI_POLL_WEIGHT
> +which, as of Linux 6.9, is 64, unless the process is run with
> +.B CAP_NET_ADMIN.

This seems a bit ambiguous: 'unless the process is run with
CAP_NET_ADMIN' could refer to 'cannot exceed' or 'is 64'.  Using
parentheses instead of commas, it would be unambiguous.

> +.P
> +.I argp.prefer_busy_poll
> +field is a boolean field and must be either 0 (disabled) or 1 (enabled). If

s/field is/is/?

> +enabled, this indicates to the network stack that busy poll is the
> +preferred method of processing network data and the network stack should
> +give the application the opportunity to busy poll. Without this option,
> +very busy systems may continue to do network processing via the normal
> +method of IRQs triggering softIRQ and NAPI.
> +.P
> +.I argp.__pad
> +must be zero.
> +.P

.P is redundant right before .SH

> +.SH RETURN VALUE
> +On success, 0 is returned.
> +On failure, \-1 is returned, and
> +.I errno
> +is set to indicate the error.
> +.SH ERRORS
> +.TP
> +.B EOPNOTSUPP
> +The kernel was not compiled with busy poll support.
> +.TP
> +.B EINVAL
> +.I fd
> +is not a valid file descriptor.
> +.TP
> +.B EINVAL
> +.I op
> +specified is invalid.

Let's not document this one, since it's already documented in ioctl(2).

> +.TP
> +.B EINVAL
> +.I argp.__pad
> +was not zero.
> +.TP
> +.B EINVAL
> +.I argp.busy_poll_usecs
> +exceeds

There's a bit of an inconsistency: the previous entry uses the past
tense, but this one uses the present.  I prefer to use the present in
both.  See also the next one.

> +.B INT_MAX .
> +.TP
> +.B EINVAL
> +.I argp.prefer_busy_poll
> +is not 0 or 1.
> +.TP
> +.B EPERM
> +The process is being run without
> +.I CAP_NET_ADMIN
> +and the specified
> +.I argp.busy_poll_budget
> +exceeds
> +.B NAPI_POLL_WEIGHT
> +(which is 64 as of Linux 6.9).

I prefer to not repeat the 64 here.

> +.TP
> +.B EFAULT
> +.I argp
> +does not point to a valid memory address.
> +.SH EXAMPLES
> +.EX
> +/* Code to set the epoll params to enable busy polling */
> +\&
> +int epollfd = epoll_create1(0);
> +struct epoll_params params;
> +\&
> +if (epollfd == \-1) {
> +    perror("epoll_create1");
> +    exit(EXIT_FAILURE);
> +}
> +\&
> +memset(&params, 0, sizeof(struct epoll_params));
> +\&
> +params.busy_poll_usecs = 25;
> +params.busy_poll_budget = 8;
> +params.prefer_busy_poll = 1;
> +\&
> +if (ioctl(epollfd, EPIOCSPARAMS, &params) == \-1) {
> +    perror("ioctl");
> +    exit(EXIT_FAILURE);
> +}
> +\&
> +/* Code to show how to retrieve the current settings */
> +\&
> +memset(&params, 0, sizeof(struct epoll_params));
> +\&
> +if (ioctl(epollfd, EPIOCGPARAMS, &params) == \-1) {
> +    perror("ioctl");
> +    exit(EXIT_FAILURE);
> +}
> +\&
> +/* params struct now contains the current parameters */
> +\&
> +fprintf(stderr, "epoll usecs: %lu\\n", params.busy_poll_usecs);

We use '\e', not '\\'.  (I haven't checked whether it also works, and
don't remember.)

> +fprintf(stderr, "epoll packet budget: %u\\n", params.busy_poll_budget);
> +fprintf(stderr, "epoll prefer busy poll: %u\\n", params.prefer_busy_poll);
> +\&
> +.SH History
> +Linux 6.9, glibc 2.40.

Let's reformat this as:

Linux 6.9.
glibc 2.40.

> +.SH SEE ALSO
> +.BR ioctl (2),
> +.BR epoll_create (2),
> +.BR epoll_create1 (2),
> +.BR epoll (7)
> +.P
> +.I linux.git/Documentation/networking/napi.rst
> +.P
> +and
> +.P

I think we can remove the 'and'.

> +.I linux.git/Documentation/admin-guide/sysctl/net.rst
> diff --git a/man/man2const/EPIOCGPARAMS.2const b/man/man2const/EPIOCGPARAMS.2const
> new file mode 100644
> index 000000000..6fbc5f0f8
> --- /dev/null
> +++ b/man/man2const/EPIOCGPARAMS.2const
> @@ -0,0 +1 @@
> +.so man2/ioctl_epoll.2
> diff --git a/man/man2const/EPIOCSPARAMS.2const b/man/man2const/EPIOCSPARAMS.2const
> new file mode 100644
> index 000000000..6fbc5f0f8
> --- /dev/null
> +++ b/man/man2const/EPIOCSPARAMS.2const
> @@ -0,0 +1 @@
> +.so man2/ioctl_epoll.2
> diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
> index e7892922e..4ad032bdd 100644
> --- a/man/man7/epoll.7
> +++ b/man/man7/epoll.7
> @@ -606,5 +606,6 @@ is present in an epoll instance.
>  .BR epoll_create1 (2),
>  .BR epoll_ctl (2),
>  .BR epoll_wait (2),
> +.BR ioctl_epoll (2),
>  .BR poll (2),
>  .BR select (2)
> -- 
> 2.34.1

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Joe Damato]

On Tue, Jun 11, 2024 at 01:45:57AM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Mon, Jun 10, 2024 at 11:12:06PM GMT, Joe Damato wrote:
> > A new page is added which describes epoll fd ioctls: EPIOCSPARAMS and
> > EPIOCGPARAMS which allow the user to control epoll-based busy polling.
> > 
> > Also add link pages for EPIOCSPARAMS and EPIOCGPARAMS.
> > 
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> 
> Thanks!

Thanks again for your careful review. Sorry this wasn't the winning
revision :)

I made almost all of the changes you mentioned, with several more questions
listed below (sorry).

Thanks for your patience on these reviews and all the very helpful
feedback.

> > ---
> >  man/man2/epoll_create.2           |   1 +
> >  man/man2/epoll_ctl.2              |   1 +
> >  man/man2/ioctl.2                  |   1 +
> >  man/man2/ioctl_epoll.2            | 171 ++++++++++++++++++++++++++++++
> 
> I'm working on a general refactor of all ioctl manual pages, and I'm
> making the pages have a name consistent with the UAPI header that
> provides them.  You can see the progress here:
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=ioctl>
> 
> For consistency, please rename the page as ioctl_eventpoll(2).

OK I've renamed it to

man/man2/ioctl_eventpoll.2

> >  man/man2const/EPIOCGPARAMS.2const |   1 +
> >  man/man2const/EPIOCSPARAMS.2const |   1 +
> >  man/man7/epoll.7                  |   1 +
> >  7 files changed, 177 insertions(+)
> >  create mode 100644 man/man2/ioctl_epoll.2
> >  create mode 100644 man/man2const/EPIOCGPARAMS.2const
> >  create mode 100644 man/man2const/EPIOCSPARAMS.2const
> > 
> > diff --git a/man/man2/epoll_create.2 b/man/man2/epoll_create.2
> > index f0327e8ba..2aa1745f5 100644
> > --- a/man/man2/epoll_create.2
> > +++ b/man/man2/epoll_create.2
> > @@ -141,4 +141,5 @@ on overrun.
> >  .BR close (2),
> >  .BR epoll_ctl (2),
> >  .BR epoll_wait (2),
> > +.BR ioctl_epoll (2),
> >  .BR epoll (7)
> > diff --git a/man/man2/epoll_ctl.2 b/man/man2/epoll_ctl.2
> > index 6d5bc032e..24bbe7405 100644
> > --- a/man/man2/epoll_ctl.2
> > +++ b/man/man2/epoll_ctl.2
> > @@ -425,5 +425,6 @@ flag.
> >  .SH SEE ALSO
> >  .BR epoll_create (2),
> >  .BR epoll_wait (2),
> > +.BR ioctl_epoll (2),
> >  .BR poll (2),
> >  .BR epoll (7)
> > diff --git a/man/man2/ioctl.2 b/man/man2/ioctl.2
> > index 5b8c28a9c..d96777d1f 100644
> > --- a/man/man2/ioctl.2
> > +++ b/man/man2/ioctl.2
> > @@ -225,6 +225,7 @@ for the various architectures.
> >  .BR ioctl_ns (2),
> >  .BR ioctl_tty (2),
> >  .BR ioctl_userfaultfd (2),
> > +.BR ioctl_epoll (2),
> >  .BR open (2),
> >  .\" .BR mt (4),
> >  .BR sd (4),
> > diff --git a/man/man2/ioctl_epoll.2 b/man/man2/ioctl_epoll.2
> > new file mode 100644
> > index 000000000..458e72e9a
> > --- /dev/null
> > +++ b/man/man2/ioctl_epoll.2
> > @@ -0,0 +1,171 @@
> > +.\" Copyright (c) 2024, Joe Damato
> 
> Please reformat as:
> 
> .\" Copyright 2024, Joe Damato <jdamato@fastly.com>
> 
> (or another email if you want, but that's the format I'm trying to use
> consistently.)
> 
> > +.\" Written by Joe Damato <jdamato@fastly.com>
> 
> You can remove this line.

Fixed both of the above, thanks!

> > +.\"
> > +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> > +.\"
> > +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> > +.SH NAME
> > +ioctl_epoll \- ioctl() operations for epoll file descriptors
> 
> Please reformat as:
> 
> ioctl_eventpoll,
> EPIOCSPARAMS,
> EPIOCGPARAMS
> \-
> ioctl() operations for epoll file descriptors
> 
> (which has '\-' on a line of its own, and has the individual ops
> listed.)

OK, fixed, thanks!
 
> > +.SH LIBRARY
> > +Standard C library
> > +.RI ( libc ", " \-lc )
> > +.SH SYNOPSIS
> > +.EX
> > +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
> 
> Remove ' and struct ...' from that comment.  We only have constants in
> those comments (except in a few pages, where I'm fixing it at the
> moment).

Fixed, thanks!

> > +.B "#include <sys/ioctl.h>"
> > +.P
> > +.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > +.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> 
> To document the header that provides this structure, let's add here:
> 
> .P
> .B #include <linux/eventpoll.h>

Hmm, that's the linux sources header file, I think.

Should I be showing the glibc header instead?

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sys/epoll.h;h=45e546fa4440a83bb94288c220bfbe9295f02cc9;hb=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158#l91

Which would be:

.B #include <sys/epoll.h>

It's in the same header as epoll_create(2) and
epoll_create1(2).

Let me know what you think.

> > +.P
> > +.B struct epoll_params {
> > +.BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
> > +.BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
> > +.BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
> > +\&
> > +.BR " " "   /* pad the struct to a multiple of 64bits */"
> > +.BR "    uint8_t __pad;"            "  /* Must be zero */"
> > +.B };
> > +.EE
> > +.SH DESCRIPTION
> > +.TP
> > +.B EPIOCSPARAMS
> > +Set the
> > +.I epoll_params
> > +structure to configure the operation of epoll. Refer to the structure
> 
> Please use semantic newlines.  See man-pages(7):
> 
> $ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p'
>    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  (commas,  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.

OK, I've tried to fix this up just now throughout the file, thanks
for letting me know. I hope I've gotten them all!
 
> > +description below to learn what configuration is
> > +supported.
> > +.TP
> > +.B EPIOCGPARAMS
> > +Get the current
> > +.I epoll_params
> > +configuration settings.
> > +.TP

I think this .TP should be a .P, not a .TP. It looks better as a .P,
at least :)

Let me know what you think.

> > +All
> > +.BR ioctl (2)
> 
> We can omit 'ioctl(2)' here, since it's obvious from the context, I
> think.  How about 'All operations documented ...'?

Sure, fixed!

> > +operations documented above must be performed on an epoll file descriptor,
> > +which can be created with a call to
> 
> s/created/obtained/?

Sure, fixed!

> > +.BR epoll_create (2)
> > +or
> > +.BR epoll_create1 (2).
> > +.\" kernel commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> 
> Let's reformat these:
> 
> .\" linux.git 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> .\" glibc.git 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> 
> (maybe say linux.git commit 1234...?  What do you prefer?)

I've made them:

.\" linux.git commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
.\" glibc.git commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158

> > +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > +.P
> 
> I would use a subsection (.SS) for documenting the structure.

Sure, I can do that.

.SS
The epoll_params structure
.I argp.busy_poll_usecs

Is that OK for a heading?

I saw this is how man/man2/stat.2 does the subsection.

Let me know what you think.

> > +.I argp.busy_poll_usecs
> > +field denotes the number of microseconds that the network stack will busy
> 
> s/field //?

Removed, thanks!

> > +poll. During this time period, the network device will be polled
> > +repeatedly for packets. This value cannot exceed
> > +.B INT_MAX.
> > +.in
> > +.P
> > +.I argp.busy_poll_budget
> > +field denotes the maximum number of packets that the network stack will
> 
> s/field //?

Removed, thanks!

> > +retrieve on each poll attempt. This value cannot exceed
> > +.B NAPI_POLL_WEIGHT
> > +which, as of Linux 6.9, is 64, unless the process is run with
> > +.B CAP_NET_ADMIN.
> 
> This seems a bit ambiguous: 'unless the process is run with
> CAP_NET_ADMIN' could refer to 'cannot exceed' or 'is 64'.  Using
> parentheses instead of commas, it would be unambiguous.

Changed this to:

retrieve on each poll attempt. This value cannot exceed
.B NAPI_POLL_WEIGHT
(which is 64 as of Linux 6.9), unless the process is run with
.B CAP_NET_ADMIN.

How is that?

> > +.P
> > +.I argp.prefer_busy_poll
> > +field is a boolean field and must be either 0 (disabled) or 1 (enabled). If
> 
> s/field is/is/?

Thanks, fixed!

> > +enabled, this indicates to the network stack that busy poll is the
> > +preferred method of processing network data and the network stack should
> > +give the application the opportunity to busy poll. Without this option,
> > +very busy systems may continue to do network processing via the normal
> > +method of IRQs triggering softIRQ and NAPI.
> > +.P
> > +.I argp.__pad
> > +must be zero.
> > +.P
> 
> .P is redundant right before .SH

Removed, thanks!

> > +.SH RETURN VALUE
> > +On success, 0 is returned.
> > +On failure, \-1 is returned, and
> > +.I errno
> > +is set to indicate the error.
> > +.SH ERRORS
> > +.TP
> > +.B EOPNOTSUPP
> > +The kernel was not compiled with busy poll support.

This line here has a weird indentation compared to the rest of the
errors when rendered.

Maybe I am doing something wrong with this one?

> > +.TP
> > +.B EINVAL
> > +.I fd
> > +is not a valid file descriptor.
> > +.TP
> > +.B EINVAL
> > +.I op
> > +specified is invalid.
> 
> Let's not document this one, since it's already documented in ioctl(2).

OK, removed.

> > +.TP
> > +.B EINVAL
> > +.I argp.__pad
> > +was not zero.
> > +.TP
> > +.B EINVAL
> > +.I argp.busy_poll_usecs
> > +exceeds
> 
> There's a bit of an inconsistency: the previous entry uses the past
> tense, but this one uses the present.  I prefer to use the present in
> both.  See also the next one.

Fixed tense to be present tense.

> > +.B INT_MAX .
> > +.TP
> > +.B EINVAL
> > +.I argp.prefer_busy_poll
> > +is not 0 or 1.
> > +.TP
> > +.B EPERM
> > +The process is being run without
> > +.I CAP_NET_ADMIN
> > +and the specified
> > +.I argp.busy_poll_budget
> > +exceeds
> > +.B NAPI_POLL_WEIGHT
> > +(which is 64 as of Linux 6.9).
> 
> I prefer to not repeat the 64 here.

OK removed that line.

> > +.TP
> > +.B EFAULT
> > +.I argp
> > +does not point to a valid memory address.
> > +.SH EXAMPLES
> > +.EX
> > +/* Code to set the epoll params to enable busy polling */
> > +\&
> > +int epollfd = epoll_create1(0);
> > +struct epoll_params params;
> > +\&
> > +if (epollfd == \-1) {
> > +    perror("epoll_create1");
> > +    exit(EXIT_FAILURE);
> > +}
> > +\&
> > +memset(&params, 0, sizeof(struct epoll_params));
> > +\&
> > +params.busy_poll_usecs = 25;
> > +params.busy_poll_budget = 8;
> > +params.prefer_busy_poll = 1;
> > +\&
> > +if (ioctl(epollfd, EPIOCSPARAMS, &params) == \-1) {
> > +    perror("ioctl");
> > +    exit(EXIT_FAILURE);
> > +}
> > +\&
> > +/* Code to show how to retrieve the current settings */
> > +\&
> > +memset(&params, 0, sizeof(struct epoll_params));
> > +\&
> > +if (ioctl(epollfd, EPIOCGPARAMS, &params) == \-1) {
> > +    perror("ioctl");
> > +    exit(EXIT_FAILURE);
> > +}
> > +\&
> > +/* params struct now contains the current parameters */
> > +\&
> > +fprintf(stderr, "epoll usecs: %lu\\n", params.busy_poll_usecs);
> 
> We use '\e', not '\\'.  (I haven't checked whether it also works, and
> don't remember.)

Change this to '\e' and tested it. It looks like it works to me :)

> > +fprintf(stderr, "epoll packet budget: %u\\n", params.busy_poll_budget);
> > +fprintf(stderr, "epoll prefer busy poll: %u\\n", params.prefer_busy_poll);
> > +\&
> > +.SH History
> > +Linux 6.9, glibc 2.40.
> 
> Let's reformat this as:
> 
> Linux 6.9.
> glibc 2.40.

Fixed.
 
> > +.SH SEE ALSO
> > +.BR ioctl (2),
> > +.BR epoll_create (2),
> > +.BR epoll_create1 (2),
> > +.BR epoll (7)
> > +.P
> > +.I linux.git/Documentation/networking/napi.rst
> > +.P
> > +and
> > +.P
> 
> I think we can remove the 'and'.

Removed, thanks!

> > +.I linux.git/Documentation/admin-guide/sysctl/net.rst
> > diff --git a/man/man2const/EPIOCGPARAMS.2const b/man/man2const/EPIOCGPARAMS.2const
> > new file mode 100644
> > index 000000000..6fbc5f0f8
> > --- /dev/null
> > +++ b/man/man2const/EPIOCGPARAMS.2const
> > @@ -0,0 +1 @@
> > +.so man2/ioctl_epoll.2
> > diff --git a/man/man2const/EPIOCSPARAMS.2const b/man/man2const/EPIOCSPARAMS.2const
> > new file mode 100644
> > index 000000000..6fbc5f0f8
> > --- /dev/null
> > +++ b/man/man2const/EPIOCSPARAMS.2const
> > @@ -0,0 +1 @@
> > +.so man2/ioctl_epoll.2
> > diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
> > index e7892922e..4ad032bdd 100644
> > --- a/man/man7/epoll.7
> > +++ b/man/man7/epoll.7
> > @@ -606,5 +606,6 @@ is present in an epoll instance.
> >  .BR epoll_create1 (2),
> >  .BR epoll_ctl (2),
> >  .BR epoll_wait (2),
> > +.BR ioctl_epoll (2),
> >  .BR poll (2),
> >  .BR select (2)
> > -- 
> > 2.34.1
> 
> Have a lovely night!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 6404 bytes --]

Hi Joe,

On Mon, Jun 10, 2024 at 05:29:06PM GMT, Joe Damato wrote:
> On Tue, Jun 11, 2024 at 01:45:57AM +0200, Alejandro Colomar wrote:
> > Hi Joe,
> > 
> > On Mon, Jun 10, 2024 at 11:12:06PM GMT, Joe Damato wrote:
> > > A new page is added which describes epoll fd ioctls: EPIOCSPARAMS and
> > > EPIOCGPARAMS which allow the user to control epoll-based busy polling.
> > > 
> > > Also add link pages for EPIOCSPARAMS and EPIOCGPARAMS.
> > > 
> > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > 
> > Thanks!
> 
> Thanks again for your careful review. Sorry this wasn't the winning
> revision :)

No problem.  Sorry for being so pedantic.  (Not sorry, actually.)  :-)
And thanks for your patience on my review.

[...]

> > > +.B "#include <sys/ioctl.h>"
> > > +.P
> > > +.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > > +.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> > 
> > To document the header that provides this structure, let's add here:
> > 
> > .P
> > .B #include <linux/eventpoll.h>
> 
> Hmm, that's the linux sources header file, I think.
> 
> Should I be showing the glibc header instead?
> 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sys/epoll.h;h=45e546fa4440a83bb94288c220bfbe9295f02cc9;hb=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158#l91

Ahh, sure, and for the constants too.  We prefer glibc headers when
available.  I had the inertia that most ioctl(2)s do not have glibc
headers.

> Which would be:
> 
> .B #include <sys/epoll.h>
> 
> It's in the same header as epoll_create(2) and
> epoll_create1(2).
> 
> Let me know what you think.

Like.

[...]

> > > +description below to learn what configuration is
> > > +supported.
> > > +.TP
> > > +.B EPIOCGPARAMS
> > > +Get the current
> > > +.I epoll_params
> > > +configuration settings.
> > > +.TP
> 
> I think this .TP should be a .P, not a .TP. It looks better as a .P,
> at least :)
> 
> Let me know what you think.

Yup.

BTW, here's the mnemonics:

P	paragraph
TP	tagged paragraph
IP	indented paragraph

[...]

> > > +.BR epoll_create (2)
> > > +or
> > > +.BR epoll_create1 (2).
> > > +.\" kernel commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > 
> > Let's reformat these:
> > 
> > .\" linux.git 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > .\" glibc.git 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > 
> > (maybe say linux.git commit 1234...?  What do you prefer?)
> 
> I've made them:
> 
> .\" linux.git commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> .\" glibc.git commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158

Good.  I'll start using that format everywhere.

> > > +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > > +.P
> > 
> > I would use a subsection (.SS) for documenting the structure.
> 
> Sure, I can do that.
> 
> .SS
> The epoll_params structure
> .I argp.busy_poll_usecs
> 
> Is that OK for a heading?
> 
> I saw this is how man/man2/stat.2 does the subsection.
> 
> Let me know what you think.

Yep.  Except that the title goes on the SS line:

$ grep '\.SS' man/man2/stat.2 
.SS The stat structure
.SS fstatat()
.SS C library/kernel differences

[...]

> > > +retrieve on each poll attempt. This value cannot exceed
> > > +.B NAPI_POLL_WEIGHT
> > > +which, as of Linux 6.9, is 64, unless the process is run with
> > > +.B CAP_NET_ADMIN.
> > 
> > This seems a bit ambiguous: 'unless the process is run with
> > CAP_NET_ADMIN' could refer to 'cannot exceed' or 'is 64'.  Using
> > parentheses instead of commas, it would be unambiguous.
> 
> Changed this to:
> 
> retrieve on each poll attempt. This value cannot exceed
> .B NAPI_POLL_WEIGHT
> (which is 64 as of Linux 6.9), unless the process is run with
> .B CAP_NET_ADMIN.
> 
> How is that?

Much better.  (But still needs to use semantic newlines.)

[...]

> > > +.SH RETURN VALUE
> > > +On success, 0 is returned.
> > > +On failure, \-1 is returned, and
> > > +.I errno
> > > +is set to indicate the error.
> > > +.SH ERRORS
> > > +.TP
> > > +.B EOPNOTSUPP
> > > +The kernel was not compiled with busy poll support.
> 
> This line here has a weird indentation compared to the rest of the
> errors when rendered.
> 
> Maybe I am doing something wrong with this one?

Nope; it's all right.  When the tag of the tagged paragraph is larger
than the indentation, the paragraph is moved to the next line, so that
it starts with the same indentation.

[...]

> > > +.TP
> > > +.B EFAULT
> > > +.I argp
> > > +does not point to a valid memory address.
> > > +.SH EXAMPLES
> > > +.EX
> > > +/* Code to set the epoll params to enable busy polling */
> > > +\&
> > > +int epollfd = epoll_create1(0);
> > > +struct epoll_params params;
> > > +\&
> > > +if (epollfd == \-1) {
> > > +    perror("epoll_create1");
> > > +    exit(EXIT_FAILURE);
> > > +}
> > > +\&
> > > +memset(&params, 0, sizeof(struct epoll_params));
> > > +\&
> > > +params.busy_poll_usecs = 25;
> > > +params.busy_poll_budget = 8;
> > > +params.prefer_busy_poll = 1;
> > > +\&
> > > +if (ioctl(epollfd, EPIOCSPARAMS, &params) == \-1) {
> > > +    perror("ioctl");
> > > +    exit(EXIT_FAILURE);
> > > +}
> > > +\&
> > > +/* Code to show how to retrieve the current settings */
> > > +\&
> > > +memset(&params, 0, sizeof(struct epoll_params));
> > > +\&
> > > +if (ioctl(epollfd, EPIOCGPARAMS, &params) == \-1) {
> > > +    perror("ioctl");
> > > +    exit(EXIT_FAILURE);
> > > +}
> > > +\&
> > > +/* params struct now contains the current parameters */
> > > +\&
> > > +fprintf(stderr, "epoll usecs: %lu\\n", params.busy_poll_usecs);
> > 
> > We use '\e', not '\\'.  (I haven't checked whether it also works, and
> > don't remember.)
> 
> Change this to '\e' and tested it. It looks like it works to me :)

Hmm, yep, both work the same.  I remember there's a small difference in
meaning, but I don't know why we use \e.  Anyway.

$ cat esc.man 
.TH a s d f
.SH example
.EX
a\\b\ec
.EE
$ groff -man -Tutf8 esc.man -rLL=72n
a(s)                                                                a(s)

example
     a\b\c

f                                   d                               a(s)


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[G. Branden Robinson]

[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]

Hi Alex,

At 2024-06-11T10:54:27+0200, Alejandro Colomar wrote:
> > > We use '\e', not '\\'.  (I haven't checked whether it also works,
> > > and don't remember.)
> > 
> > Change this to '\e' and tested it. It looks like it works to me :)
> 
> Hmm, yep, both work the same.  I remember there's a small difference
> in meaning, but I don't know why we use \e.  Anyway.

GNU troff's Texinfo manual explains:

--snip--
   The escape character is nearly always interpreted when encountered;
it is therefore desirable to have a way to interpolate it, disable it,
or change it.

 -- Escape sequence: \e
     Interpolate the escape character.  '\e' is interpreted even in copy
     mode (*note Copy Mode::).
...
   The complement of copy mode--a 'roff' formatter's behavior when not
defining or appending to a macro, string, or diversion--where all macros
are interpolated, requests invoked, and valid escape sequences processed
immediately upon recognition, can be termed "interpretation mode".

 -- Escape sequence: \\
     The escape character, '\' by default, can escape itself.  This
     enables you to control whether a given '\n', '\g', '\$', '\*',
     '\V', or '\?' escape sequence is interpreted at the time the macro
     containing it is defined, or later when the macro is called.(1)
     (*note Copy Mode-Footnote-1::)

          .nr x 20
          .de y
          .nr x 10
          \&\nx
          \&\\nx
          ..
          .y
              => 20 10

     You can think of '\\' as a "delayed" backslash; it is the escape
     character followed by a backslash from which the escape character
     has removed its special meaning.  Consequently, '\\' is not an
     escape sequence in the usual sense.  In any escape sequence '\X'
     that GNU 'troff' does not recognize, the escape character is
     ignored and X is output.  An unrecognized escape sequence causes a
     warning in category 'escape', with two exceptions--'\\' is the
     first.
--end snip--

This matters when you use "\\" inside macro arguments, for example.

Personally, if what you want is a _backslash_, to the Linux man-pages
project I would recommend the special character escape sequence that
_means_ "backslash".

GNU troff, Heirloom Doctools troff, and mandoc all recognize it; that
should be (more) than enough for places where the Linux man-pages get
installed.

groff_man_style(7):

     \(rs   Reverse solidus (backslash).  The backslash is the default
            escape character in the roff language, so it does not
            represent itself in output.  Also see \e above.

You can of course spell it \[rs], which is even better.

I would not give the same advice to bash or ncurses, which must be
portable to geriatric commercial Unix, for example.

Regards
Branden

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

On Tue, Jun 11, 2024 at 07:24:53AM GMT, G. Branden Robinson wrote:
> Hi Alex,

Hi Branden!

> 
> At 2024-06-11T10:54:27+0200, Alejandro Colomar wrote:
> > > > We use '\e', not '\\'.  (I haven't checked whether it also works,
> > > > and don't remember.)
> > > 
> > > Change this to '\e' and tested it. It looks like it works to me :)
> > 
> > Hmm, yep, both work the same.  I remember there's a small difference
> > in meaning, but I don't know why we use \e.  Anyway.
> 
> GNU troff's Texinfo manual explains:
> 
> --snip--
>    The escape character is nearly always interpreted when encountered;
> it is therefore desirable to have a way to interpolate it, disable it,
> or change it.
> 
>  -- Escape sequence: \e
>      Interpolate the escape character.  '\e' is interpreted even in copy
>      mode (*note Copy Mode::).
> ...
>    The complement of copy mode--a 'roff' formatter's behavior when not
> defining or appending to a macro, string, or diversion--where all macros
> are interpolated, requests invoked, and valid escape sequences processed
> immediately upon recognition, can be termed "interpretation mode".
> 
>  -- Escape sequence: \\
>      The escape character, '\' by default, can escape itself.  This
>      enables you to control whether a given '\n', '\g', '\$', '\*',
>      '\V', or '\?' escape sequence is interpreted at the time the macro
>      containing it is defined, or later when the macro is called.(1)
>      (*note Copy Mode-Footnote-1::)
> 
>           .nr x 20
>           .de y
>           .nr x 10
>           \&\nx
>           \&\\nx
>           ..
>           .y
>               => 20 10
> 
>      You can think of '\\' as a "delayed" backslash; it is the escape
>      character followed by a backslash from which the escape character
>      has removed its special meaning.  Consequently, '\\' is not an
>      escape sequence in the usual sense.  In any escape sequence '\X'
>      that GNU 'troff' does not recognize, the escape character is
>      ignored and X is output.  An unrecognized escape sequence causes a
>      warning in category 'escape', with two exceptions--'\\' is the
>      first.
> --end snip--
> 
> This matters when you use "\\" inside macro arguments, for example.
> 
> Personally, if what you want is a _backslash_, to the Linux man-pages
> project I would recommend the special character escape sequence that
> _means_ "backslash".
> 
> GNU troff, Heirloom Doctools troff, and mandoc all recognize it; that
> should be (more) than enough for places where the Linux man-pages get
> installed.
> 
> groff_man_style(7):
> 
>      \(rs   Reverse solidus (backslash).  The backslash is the default
>             escape character in the roff language, so it does not
>             represent itself in output.  Also see \e above.
> 
> You can of course spell it \[rs], which is even better.

Hmmm, so we should \e => \[rs].  Thanks!  I'll try to prepare a sed
script for it.  BTW, how are you doing with MR.sed?  :)

Have a lovely day!
Alex

> I would not give the same advice to bash or ncurses, which must be
> portable to geriatric commercial Unix, for example.
> 
> Regards
> Branden



-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[G. Branden Robinson]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

At 2024-06-11T16:22:28+0200, Alejandro Colomar wrote:
> Hmmm, so we should \e => \[rs].  Thanks!  I'll try to prepare a sed
> script for it.

Cool!

> BTW, how are you doing with MR.sed?  :)

I have not returned to the problem since last we spoke about it.
Working up the steam to tackle the staged-sed-attack on syscalls.2
tables again.

I will risk surrendering a hostage to fortune by venturing that MR.sed
_itself_ is already pretty well tested.  It's getting that table ready
for MR.sed to go to work on it that is the devilish detail.

I am conscious that groff 1.23.0's first anniversary is nigh...

Regards,
Branden

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Joe Damato]

On Tue, Jun 11, 2024 at 10:54:27AM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Mon, Jun 10, 2024 at 05:29:06PM GMT, Joe Damato wrote:
> > On Tue, Jun 11, 2024 at 01:45:57AM +0200, Alejandro Colomar wrote:
> > > Hi Joe,
> > > 
> > > On Mon, Jun 10, 2024 at 11:12:06PM GMT, Joe Damato wrote:
> > > > A new page is added which describes epoll fd ioctls: EPIOCSPARAMS and
> > > > EPIOCGPARAMS which allow the user to control epoll-based busy polling.
> > > > 
> > > > Also add link pages for EPIOCSPARAMS and EPIOCGPARAMS.
> > > > 
> > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > 
> > > Thanks!
> > 
> > Thanks again for your careful review. Sorry this wasn't the winning
> > revision :)
> 
> No problem.  Sorry for being so pedantic.  (Not sorry, actually.)  :-)
> And thanks for your patience on my review.

No problem at all; I really do appreciate your work here keeping the
man pages consistent and usable. Thanks for giving your time to help
me get this man page setup properly.

> [...]
> 
> > > > +.B "#include <sys/ioctl.h>"
> > > > +.P
> > > > +.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > > > +.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> > > 
> > > To document the header that provides this structure, let's add here:
> > > 
> > > .P
> > > .B #include <linux/eventpoll.h>
> > 
> > Hmm, that's the linux sources header file, I think.
> > 
> > Should I be showing the glibc header instead?
> > 
> > https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sys/epoll.h;h=45e546fa4440a83bb94288c220bfbe9295f02cc9;hb=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158#l91
> 
> Ahh, sure, and for the constants too.  We prefer glibc headers when
> available.  I had the inertia that most ioctl(2)s do not have glibc
> headers.

Ah right forgot about the constants, so what I did instead was this, which
includes the header only once:

.SH SYNOPSIS
.EX
.BR "#include <sys/epoll.h>" "  /* Definition of " EPIOC* " constants */"
.B "#include <sys/ioctl.h>"
.P
.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
.P
.B struct epoll_params {
.BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
.BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
.BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
\&
.BR " " "   /* pad the struct to a multiple of 64bits */"
.BR "    uint8_t __pad;"            "  /* Must be zero */"
.B };

Does that look OK?

[...]

> > Sure, I can do that.
> > 
> > .SS
> > The epoll_params structure
> > .I argp.busy_poll_usecs
> > 
> > Is that OK for a heading?
> > 
> > I saw this is how man/man2/stat.2 does the subsection.
> > 
> > Let me know what you think.
> 
> Yep.  Except that the title goes on the SS line:
> 
> $ grep '\.SS' man/man2/stat.2 
> .SS The stat structure
> .SS fstatat()
> .SS C library/kernel differences

OK, fixed!

> > > > +retrieve on each poll attempt. This value cannot exceed
> > > > +.B NAPI_POLL_WEIGHT
> > > > +which, as of Linux 6.9, is 64, unless the process is run with
> > > > +.B CAP_NET_ADMIN.
> > > 
> > > This seems a bit ambiguous: 'unless the process is run with
> > > CAP_NET_ADMIN' could refer to 'cannot exceed' or 'is 64'.  Using
> > > parentheses instead of commas, it would be unambiguous.
> > 
> > Changed this to:
> > 
> > retrieve on each poll attempt. This value cannot exceed
> > .B NAPI_POLL_WEIGHT
> > (which is 64 as of Linux 6.9), unless the process is run with
> > .B CAP_NET_ADMIN.
> > 
> > How is that?
> 
> Much better.  (But still needs to use semantic newlines.)

Hmm, I need to go back and re-read the semantic newline email because I made
this section look like this:

.P
.I argp.busy_poll_budget
the maximum number of packets that the network stack will retrieve on each poll attempt.
This value cannot exceed
.B NAPI_POLL_WEIGHT
(which is 64 as of Linux 6.9), unless the process is run with
.B CAP_NET_ADMIN.

But I get the feeling this is still incorrect.

> [...]
>

[...]
 
> > > > +.TP
> > > > +.B EFAULT
> > > > +.I argp
> > > > +does not point to a valid memory address.
> > > > +.SH EXAMPLES
> > > > +.EX
> > > > +/* Code to set the epoll params to enable busy polling */
> > > > +\&
> > > > +int epollfd = epoll_create1(0);
> > > > +struct epoll_params params;
> > > > +\&
> > > > +if (epollfd == \-1) {
> > > > +    perror("epoll_create1");
> > > > +    exit(EXIT_FAILURE);
> > > > +}
> > > > +\&
> > > > +memset(&params, 0, sizeof(struct epoll_params));
> > > > +\&
> > > > +params.busy_poll_usecs = 25;
> > > > +params.busy_poll_budget = 8;
> > > > +params.prefer_busy_poll = 1;
> > > > +\&
> > > > +if (ioctl(epollfd, EPIOCSPARAMS, &params) == \-1) {
> > > > +    perror("ioctl");
> > > > +    exit(EXIT_FAILURE);
> > > > +}
> > > > +\&
> > > > +/* Code to show how to retrieve the current settings */
> > > > +\&
> > > > +memset(&params, 0, sizeof(struct epoll_params));
> > > > +\&
> > > > +if (ioctl(epollfd, EPIOCGPARAMS, &params) == \-1) {
> > > > +    perror("ioctl");
> > > > +    exit(EXIT_FAILURE);
> > > > +}
> > > > +\&
> > > > +/* params struct now contains the current parameters */
> > > > +\&
> > > > +fprintf(stderr, "epoll usecs: %lu\\n", params.busy_poll_usecs);
> > > 
> > > We use '\e', not '\\'.  (I haven't checked whether it also works, and
> > > don't remember.)
> > 
> > Change this to '\e' and tested it. It looks like it works to me :)
> 
> Hmm, yep, both work the same.  I remember there's a small difference in
> meaning, but I don't know why we use \e.  Anyway.

I tried to follow the discussion you and Branden had in the following emails. I
apologize, but I don't think I quite follow what I should be using as a result
of that conversation?

\en 
\\n

or something else?

Thanks,
Joe

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3967 bytes --]

Hi Joe,

On Tue, Jun 11, 2024 at 09:34:29AM GMT, Joe Damato wrote:
> No problem at all; I really do appreciate your work here keeping the
> man pages consistent and usable. Thanks for giving your time to help
> me get this man page setup properly.

:-}

> > Ahh, sure, and for the constants too.  We prefer glibc headers when
> > available.  I had the inertia that most ioctl(2)s do not have glibc
> > headers.
> 
> Ah right forgot about the constants, so what I did instead was this, which
> includes the header only once:
> 
> .SH SYNOPSIS
> .EX
> .BR "#include <sys/epoll.h>" "  /* Definition of " EPIOC* " constants */"
> .B "#include <sys/ioctl.h>"
> .P
> .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> .P
> .B struct epoll_params {
> .BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
> .BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
> .BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
> \&
> .BR " " "   /* pad the struct to a multiple of 64bits */"
> .BR "    uint8_t __pad;"            "  /* Must be zero */"
> .B };
> 
> Does that look OK?

Nah, that makes it unclear which header provides the type.  I'd add the
include again right before the struct definition.  Some other pages have
similar style (although I can't remember at the moment any example).

> > > Changed this to:
> > > 
> > > retrieve on each poll attempt. This value cannot exceed
> > > .B NAPI_POLL_WEIGHT
> > > (which is 64 as of Linux 6.9), unless the process is run with
> > > .B CAP_NET_ADMIN.
> > > 
> > > How is that?
> > 
> > Much better.  (But still needs to use semantic newlines.)
> 
> Hmm, I need to go back and re-read the semantic newline email because I made
> this section look like this:

You may want to also read this commit:

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?id=6ff6f43d68164f99a8c3fb66f4525d145571310c>

It includes a quote from Brian W. Kernighan, which is a little bit more
detailed than man-pages(7) about it.

> 
> .P
> .I argp.busy_poll_budget
> the maximum number of packets that the network stack will retrieve on each poll attempt.
> This value cannot exceed
> .B NAPI_POLL_WEIGHT
> (which is 64 as of Linux 6.9), unless the process is run with
> .B CAP_NET_ADMIN.
> 
> But I get the feeling this is still incorrect.

Yep; it's incorrect.  We have a strict limit on column 80, so you'd need
to find a good break point in the middle.  I'd say

	s/retrieve on/retrieve\non/

(although there are other good break points, such as for example maybe
before 'retrieve').

and also break after the comma.

However, it was more correct than the previous, which continued the line
after a period, which is a no-no.  :)

> I tried to follow the discussion you and Branden had in the following emails. I
> apologize, but I don't think I quite follow what I should be using as a result
> of that conversation?
> 
> \en 
> \\n

TL;DR:

\\n is not appropriate, since it can be misinterpreted in some cases.
For example, in some cases, you'd need to double-escape: \\\\n.  Don't
use it.

\e is meh.  It means "write an escape character, whatever that is".
This is incorrect, since we want a backslash, not "whatever an escape
character is".  However, since the project has been using that for a
long time, you can use it in your patch.

\[rs] is the most appropriate, which means write a "reverse solidus"
(a.k.a., the backslash).  I'll prepare a churny patch for replacing \e
by \[rs] globally in the project.  If you feel like using it in your
patch, you're invited.  :)

Have a lovely day!
Alex

> 
> or something else?
> 
> Thanks,
> Joe
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Joe Damato]

On Tue, Jun 11, 2024 at 07:14:14PM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Tue, Jun 11, 2024 at 09:34:29AM GMT, Joe Damato wrote:
> > No problem at all; I really do appreciate your work here keeping the
> > man pages consistent and usable. Thanks for giving your time to help
> > me get this man page setup properly.
> 
> :-}
> 
> > > Ahh, sure, and for the constants too.  We prefer glibc headers when
> > > available.  I had the inertia that most ioctl(2)s do not have glibc
> > > headers.
> > 
> > Ah right forgot about the constants, so what I did instead was this, which
> > includes the header only once:
> > 
> > .SH SYNOPSIS
> > .EX
> > .BR "#include <sys/epoll.h>" "  /* Definition of " EPIOC* " constants */"
> > .B "#include <sys/ioctl.h>"
> > .P
> > .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> > .P
> > .B struct epoll_params {
> > .BR "    uint32_t busy_poll_usecs;" "  /* Number of usecs to busy poll */"
> > .BR "    uint16_t busy_poll_budget;" " /* Maximum number of packets to retrieve per poll */"
> > .BR "    uint8_t prefer_busy_poll;" "  /* Boolean to enable or disable prefer busy poll  */"
> > \&
> > .BR " " "   /* pad the struct to a multiple of 64bits */"
> > .BR "    uint8_t __pad;"            "  /* Must be zero */"
> > .B };
> > 
> > Does that look OK?
> 
> Nah, that makes it unclear which header provides the type.  I'd add the
> include again right before the struct definition.  Some other pages have
> similar style (although I can't remember at the moment any example).

OK, I've included it twice -- once before the ioctls and once before
the struct, with a comment:

.BR "#include <sys/epoll.h>" " /* Definition of " struct " "epoll_params " */"
.P
.B struct epoll_params {

Hope that is OK! If not, let me know ;)

> > > > Changed this to:
> > > > 
> > > > retrieve on each poll attempt. This value cannot exceed
> > > > .B NAPI_POLL_WEIGHT
> > > > (which is 64 as of Linux 6.9), unless the process is run with
> > > > .B CAP_NET_ADMIN.
> > > > 
> > > > How is that?
> > > 
> > > Much better.  (But still needs to use semantic newlines.)
> > 
> > Hmm, I need to go back and re-read the semantic newline email because I made
> > this section look like this:
> 
> You may want to also read this commit:
> 
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?id=6ff6f43d68164f99a8c3fb66f4525d145571310c>
> 
> It includes a quote from Brian W. Kernighan, which is a little bit more
> detailed than man-pages(7) about it.

I just read that and will continue read it a few more times. I will
try to better understand how to format the man page text as you've
explained.

Please accept my apologies if I've still gotten it wrong in the v3,
I'm not quite sure I've totally wrapped my head around when/where
are good places to wrap long lines that exceed 80 characters.

> > 
> > .P
> > .I argp.busy_poll_budget
> > the maximum number of packets that the network stack will retrieve on each poll attempt.
> > This value cannot exceed
> > .B NAPI_POLL_WEIGHT
> > (which is 64 as of Linux 6.9), unless the process is run with
> > .B CAP_NET_ADMIN.
> > 
> > But I get the feeling this is still incorrect.
> 
> Yep; it's incorrect.  We have a strict limit on column 80, so you'd need
> to find a good break point in the middle.  I'd say
> 
> 	s/retrieve on/retrieve\non/
> 
> (although there are other good break points, such as for example maybe
> before 'retrieve').
> 
> and also break after the comma.
> 
> However, it was more correct than the previous, which continued the line
> after a period, which is a no-no.  :)

Thanks, I've made the change you've suggested above and am
re-reading each line looking for anything over 80 chars that I can
break on punctuation or any other clause.

I have already broken lines at periods and simple cases like that,
but I am sure to be missing a few.

> > I tried to follow the discussion you and Branden had in the following emails. I
> > apologize, but I don't think I quite follow what I should be using as a result
> > of that conversation?
> > 
> > \en 
> > \\n
> 
> TL;DR:
> 
> \\n is not appropriate, since it can be misinterpreted in some cases.
> For example, in some cases, you'd need to double-escape: \\\\n.  Don't
> use it.
> 
> \e is meh.  It means "write an escape character, whatever that is".
> This is incorrect, since we want a backslash, not "whatever an escape
> character is".  However, since the project has been using that for a
> long time, you can use it in your patch.
> 
> \[rs] is the most appropriate, which means write a "reverse solidus"
> (a.k.a., the backslash).  I'll prepare a churny patch for replacing \e
> by \[rs] globally in the project.  If you feel like using it in your
> patch, you're invited.  :)

OK, I'll switch the \e for \[rs], which also seems to render
properly for me.

Thanks,
Joe

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3473 bytes --]

Hi Joe,

On Tue, Jun 11, 2024 at 10:28:48AM GMT, Joe Damato wrote:
> OK, I've included it twice -- once before the ioctls and once before
> the struct, with a comment:
> 
> .BR "#include <sys/epoll.h>" " /* Definition of " struct " "epoll_params " */"

No comment here, please.

> .P
> .B struct epoll_params {
> 
> Hope that is OK! If not, let me know ;)
> 
> > > > > Changed this to:
> > > > > 
> > > > > retrieve on each poll attempt. This value cannot exceed
> > > > > .B NAPI_POLL_WEIGHT
> > > > > (which is 64 as of Linux 6.9), unless the process is run with
> > > > > .B CAP_NET_ADMIN.
> > > > > 
> > > > > How is that?
> > > > 
> > > > Much better.  (But still needs to use semantic newlines.)
> > > 
> > > Hmm, I need to go back and re-read the semantic newline email because I made
> > > this section look like this:
> > 
> > You may want to also read this commit:
> > 
> > <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?id=6ff6f43d68164f99a8c3fb66f4525d145571310c>
> > 
> > It includes a quote from Brian W. Kernighan, which is a little bit more
> > detailed than man-pages(7) about it.
> 
> I just read that and will continue read it a few more times. I will
> try to better understand how to format the man page text as you've
> explained.
> 
> Please accept my apologies if I've still gotten it wrong in the v3,
> I'm not quite sure I've totally wrapped my head around when/where
> are good places to wrap long lines that exceed 80 characters.

No problems; if there are only a few small issues, I'll fix them while
applying.  Otherwise, as long as you're patient, I am too.  :)

Clause boundaries aren't as easy to spot as sentence boundaries.
Prepositions are usually good places.  'that' is usually a good place
too.  Separating the subject or an adverbial group from the rest of the
sentence is also a good choice.  But it's in the end a matter of taste.
It's maybe easier to see the places where you wouldn't want to break it,
such as:

	the maximum number of packets that the network
	stack will retrieve on each poll attempt.

because 'network stack' is an noun group (or whatever it's called in
English).

> > > .P
> > > .I argp.busy_poll_budget
> > > the maximum number of packets that the network stack will retrieve on each poll attempt.
> > > This value cannot exceed
> > > .B NAPI_POLL_WEIGHT
> > > (which is 64 as of Linux 6.9), unless the process is run with
> > > .B CAP_NET_ADMIN.
> > > 
> > > But I get the feeling this is still incorrect.
> > 
> > Yep; it's incorrect.  We have a strict limit on column 80, so you'd need
> > to find a good break point in the middle.  I'd say
> > 
> > 	s/retrieve on/retrieve\non/
> > 
> > (although there are other good break points, such as for example maybe
> > before 'retrieve').
> > 
> > and also break after the comma.
> > 
> > However, it was more correct than the previous, which continued the line
> > after a period, which is a no-no.  :)
> 
> Thanks, I've made the change you've suggested above and am
> re-reading each line looking for anything over 80 chars that I can
> break on punctuation or any other clause.
> 
> I have already broken lines at periods and simple cases like that,
> but I am sure to be missing a few.

Okay; as long as there's nothing egregious, that should be good.  :)

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/1] ioctl_epoll.2: New page describing epoll ioctl(2)

[Joe Damato]

On Tue, Jun 11, 2024 at 07:43:52PM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Tue, Jun 11, 2024 at 10:28:48AM GMT, Joe Damato wrote:
> > OK, I've included it twice -- once before the ioctls and once before
> > the struct, with a comment:
> > 
> > .BR "#include <sys/epoll.h>" " /* Definition of " struct " "epoll_params " */"
> 
> No comment here, please.

OK removed and switched to .B (instead of .BR).

> > .P
> > .B struct epoll_params {
> > 
> > Hope that is OK! If not, let me know ;)
> > 
> > > > > > Changed this to:
> > > > > > 
> > > > > > retrieve on each poll attempt. This value cannot exceed
> > > > > > .B NAPI_POLL_WEIGHT
> > > > > > (which is 64 as of Linux 6.9), unless the process is run with
> > > > > > .B CAP_NET_ADMIN.
> > > > > > 
> > > > > > How is that?
> > > > > 
> > > > > Much better.  (But still needs to use semantic newlines.)
> > > > 
> > > > Hmm, I need to go back and re-read the semantic newline email because I made
> > > > this section look like this:
> > > 
> > > You may want to also read this commit:
> > > 
> > > <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?id=6ff6f43d68164f99a8c3fb66f4525d145571310c>
> > > 
> > > It includes a quote from Brian W. Kernighan, which is a little bit more
> > > detailed than man-pages(7) about it.
> > 
> > I just read that and will continue read it a few more times. I will
> > try to better understand how to format the man page text as you've
> > explained.
> > 
> > Please accept my apologies if I've still gotten it wrong in the v3,
> > I'm not quite sure I've totally wrapped my head around when/where
> > are good places to wrap long lines that exceed 80 characters.
> 
> No problems; if there are only a few small issues, I'll fix them while
> applying.  Otherwise, as long as you're patient, I am too.  :)
> 
> Clause boundaries aren't as easy to spot as sentence boundaries.
> Prepositions are usually good places.  'that' is usually a good place
> too.  Separating the subject or an adverbial group from the rest of the
> sentence is also a good choice.  But it's in the end a matter of taste.
> It's maybe easier to see the places where you wouldn't want to break it,
> such as:
> 
> 	the maximum number of packets that the network
> 	stack will retrieve on each poll attempt.
> 
> because 'network stack' is an noun group (or whatever it's called in
> English).

OK thanks for the examples.

I'll take another read through what I have and send a v3 shortly.

[...]