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

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

[Joe Damato]

Greetings:

This is my first contribution to the man-pages project, so please excuse
any obvious issues; I am happy to take feedback and send updated patches as
needed.

This change documents a new ioctl interface for epoll added to Linux kernel
6.9 [1] and glibc [2] for controlling busy poll on a per-epoll fd basis.

I noted that other ioctls have ioctl_*.2 files, so I followed that
pattern in this change.

Based on the current status of glibc, I would assume that this change will
be part of glibc 2.40 (it is listed under 2.40 in the NEWS section), which
may be released in a few months [3].

Given that, I am not sure if I should wait until glibc 2.40 has been
released before sending this change to this project or not.

Please let me know.

Thanks,
Joe

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/fs/eventpoll.c?h=v6.9&id=18e2bf0edf4dd88d9656ec92395aa47392e85b61
[2]: https://sourceware.org/git/?p=glibc.git;a=commit;h=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
[3]: https://sourceware.org/glibc/wiki/Glibc%20Timeline

Joe Damato (1):
  ioctl_epoll.2: New page describing ioctl(2) operations for epoll fds

 man/man2/epoll_create.2 |   1 +
 man/man2/epoll_ctl.2    |   1 +
 man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
 man/man7/epoll.7        |   1 +
 4 files changed, 206 insertions(+)
 create mode 100644 man/man2/ioctl_epoll.2

-- 
2.34.1

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

[Joe Damato]

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 man/man2/epoll_create.2 |   1 +
 man/man2/epoll_ctl.2    |   1 +
 man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
 man/man7/epoll.7        |   1 +
 4 files changed, 206 insertions(+)
 create mode 100644 man/man2/ioctl_epoll.2

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_epoll.2 b/man/man2/ioctl_epoll.2
new file mode 100644
index 000000000..1d53f458e
--- /dev/null
+++ b/man/man2/ioctl_epoll.2
@@ -0,0 +1,203 @@
+.\" 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
+.nf
+.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
+.B "#include <sys/ioctl.h>"
+.P
+.BI "int ioctl(int " fd ", int " op ", void " *argp ");"
+.fi
+.SH DESCRIPTION
+Various
+.BR ioctl (2)
+operations can be performed on an epoll file descriptor (created by a call
+to
+.BR epoll_create (2))
+(since Linux 6.9 and glibc 2.40) using calls of the form:
+.\" commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
+.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
+.P
+.in +4n
+.EX
+ioctl(fd, op, argp);
+.EE
+.in
+.P
+In the above,
+.I fd
+is a file descriptor referring to an epoll file descriptor obtained with a
+call to
+.BR epoll_create (2).
+.I op
+is one of the operations listed below, and
+.I argp
+is a pointer to the data structure described below.
+.\"
+.P
+All supported
+.I op
+values (described below) use an
+.I argp
+argument which is a pointer to a
+.I epoll_params
+structure, defined as:
+.P
+.in +4n
+.EX
+struct epoll_params {
+    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
+    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
+    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
+
+    /* pad the struct to a multiple of 64bits */
+    uint8_t __pad;              /* Must be zero */
+};
+.EE
+.in
+.P
+The
+.I 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. This value cannot exceed
+.B INT_MAX .
+.in
+.P
+The
+.I busy_poll_budget
+field denotes the maximum number of packets that the network stack will
+be retrieved 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
+The
+.I 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
+The supported
+.I op
+values:
+.TP
+.B EPIOCSPARAMS
+This operation allows the caller to specify an
+.I epoll_params
+structure to configure the operation of epoll. Refer to the structure
+description of the structure above to learn what configuration is
+supported.
+.TP
+.B EPIOCGPARAMS
+This operation allows the caller to retrieve the current
+.I epoll_params
+structure. This can be used to determine what the current settings are.
+.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 ENOIOCTLCMD
+The specified
+.I op
+is invalid.
+.TP
+.B EINVAL
+The
+.I fd
+specified is not an epoll file descriptor, or the
+.I op
+specified is invalid, or the
+.I __pad
+was not initialized to zero, or
+.I busy_poll_usecs
+exceeds
+.B INT_MAX ,
+or
+.I 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 busy_poll_budget
+exceeds
+.I 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 Documentation/networking/napi.rst
+.P
+and
+.P
+.I Documentation/admin-guide/sysctl/net.rst
+.P
+in the Linux kernel source tree
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 1/1] ioctl_epoll.2: New page describing ioctl(2) operations for epoll fds

[Alejandro Colomar]

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

Hi Joe,

On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> Signed-off-by: Joe Damato <jdamato@fastly.com>

Thanks for the patch!  Please see a few comments below.

Have a lovely night!
Alex

> ---
>  man/man2/epoll_create.2 |   1 +
>  man/man2/epoll_ctl.2    |   1 +
>  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
>  man/man7/epoll.7        |   1 +
>  4 files changed, 206 insertions(+)
>  create mode 100644 man/man2/ioctl_epoll.2
> 
> 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_epoll.2 b/man/man2/ioctl_epoll.2
> new file mode 100644
> index 000000000..1d53f458e
> --- /dev/null
> +++ b/man/man2/ioctl_epoll.2
> @@ -0,0 +1,203 @@
> +.\" Copyright (c) 2024, Joe Damato
> +.\" Written by Joe Damato <jdamato@fastly.com>
> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.\"
> +.\"

Please use only one consecutive .\"

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

Please add link pages <man2const/EPIOCSPARAMS.2const> and
<man2const/EPIOCGPARAMS.2const>.

> +.SH LIBRARY
> +Standard C library
> +.RI ( libc ", " \-lc )
> +.SH SYNOPSIS
> +.nf
> +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
> +.B "#include <sys/ioctl.h>"
> +.P
> +.BI "int ioctl(int " fd ", int " op ", void " *argp ");"

The '*' should be bold; not italics.

> +.fi
> +.SH DESCRIPTION
> +Various
> +.BR ioctl (2)
> +operations can be performed on an epoll file descriptor (created by a call
> +to
> +.BR epoll_create (2))
> +(since Linux 6.9 and glibc 2.40) using calls of the form:
> +.\" commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> +.P
> +.in +4n
> +.EX
> +ioctl(fd, op, argp);
> +.EE
> +.in
> +.P
> +In the above,
> +.I fd
> +is a file descriptor referring to an epoll file descriptor obtained with a
> +call to
> +.BR epoll_create (2).
> +.I op
> +is one of the operations listed below, and
> +.I argp
> +is a pointer to the data structure described below.

If argp is a pointer to a structure, then the prototype should document
that:

.BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );

Also, I would document the two operations separately:

.BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
.BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );

This allows documenting that the 'S' one doesn't modify the argp
(AFAICS).

> +.\"
> +.P
> +All supported
> +.I op
> +values (described below) use an
> +.I argp
> +argument which is a pointer to a
> +.I epoll_params
> +structure, defined as:
> +.P
> +.in +4n
> +.EX
> +struct epoll_params {
> +    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
> +    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
> +    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
> +
> +    /* pad the struct to a multiple of 64bits */
> +    uint8_t __pad;              /* Must be zero */
> +};

You could add this type definition to the SYNOPSIS, below the function
prototypes.

> +.EE
> +.in
> +.P
> +The
> +.I 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. This value cannot exceed
> +.B INT_MAX .
> +.in
> +.P
> +The
> +.I busy_poll_budget
> +field denotes the maximum number of packets that the network stack will
> +be retrieved 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
> +The
> +.I 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
> +The supported
> +.I op
> +values:
> +.TP
> +.B EPIOCSPARAMS
> +This operation allows the caller to specify an
> +.I epoll_params
> +structure to configure the operation of epoll. Refer to the structure
> +description of the structure above to learn what configuration is
> +supported.
> +.TP
> +.B EPIOCGPARAMS
> +This operation allows the caller to retrieve the current
> +.I epoll_params
> +structure. This can be used to determine what the current settings are.
> +.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 ENOIOCTLCMD

Is this a thing?

	$ grep -rn ENOIOCTLCMD /usr/include/
	$ 

I suspect this is EINVAL in user space?  (Actually, you list the same
exact error condition for EINVAL below.)

> +The specified
> +.I op
> +is invalid.
> +.TP
> +.B EINVAL
> +The
> +.I fd
> +specified is not an epoll file descriptor, or the
> +.I op
> +specified is invalid, or the
> +.I __pad
> +was not initialized to zero, or
> +.I busy_poll_usecs
> +exceeds
> +.B INT_MAX ,
> +or
> +.I prefer_busy_poll
> +is not 0 or 1.

Please separate the different conditions for EINVAL into separate
entries.  The ones that are related can go in the same one, but the
unrelated ones are better split.

> +.TP
> +.B EPERM
> +The process is being run without
> +.I CAP_NET_ADMIN

This should be bold.

> +and the specified
> +.I busy_poll_budget

This should be

.I argp.busy_poll_budget

> +exceeds
> +.I NAPI_POLL_WEIGHT

This should be bold.

> +(which is 64 as of Linux 6.9).
> +.TP
> +.B EFAULT
> +.I argp
> +does not point to a valid memory address.
> +.SH EXAMPLES
> +.EX

Could you write an entire program, with a main()?

If not, it's fine; this is better than nothing.  But we prefer having
entire programs that users can paste and try.

> +/* 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 Documentation/networking/napi.rst
> +.P
> +and
> +.P
> +.I Documentation/admin-guide/sysctl/net.rst
> +.P
> +in the Linux kernel source tree

We could document that as

.I linux.git/Documentation/...

to not have to say "in the Linux kernel source tree".

> 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
> 

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

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

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

[Alejandro Colomar]

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

On Thu, Jun 06, 2024 at 11:40:01PM GMT, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> 
> Thanks for the patch!  Please see a few comments below.

BTW, it triggers the following warning:

$ make lint build check -j24
GROTTY		.tmp/man/man2/epoll_create.2.cat
GROTTY		.tmp/man/man2/epoll_ctl.2.cat
TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'

> 
> Have a lovely night!
> Alex
> 
> > ---
> >  man/man2/epoll_create.2 |   1 +
> >  man/man2/epoll_ctl.2    |   1 +
> >  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
> >  man/man7/epoll.7        |   1 +
> >  4 files changed, 206 insertions(+)
> >  create mode 100644 man/man2/ioctl_epoll.2
> > 
> > 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_epoll.2 b/man/man2/ioctl_epoll.2
> > new file mode 100644
> > index 000000000..1d53f458e
> > --- /dev/null
> > +++ b/man/man2/ioctl_epoll.2
> > @@ -0,0 +1,203 @@
> > +.\" Copyright (c) 2024, Joe Damato
> > +.\" Written by Joe Damato <jdamato@fastly.com>
> > +.\"
> > +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> > +.\"
> > +.\"
> 
> Please use only one consecutive .\"
> 
> > +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> > +.SH NAME
> > +ioctl_epoll \- ioctl() operations for epoll file descriptors
> 
> Please add link pages <man2const/EPIOCSPARAMS.2const> and
> <man2const/EPIOCGPARAMS.2const>.
> 
> > +.SH LIBRARY
> > +Standard C library
> > +.RI ( libc ", " \-lc )
> > +.SH SYNOPSIS
> > +.nf
> > +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
> > +.B "#include <sys/ioctl.h>"
> > +.P
> > +.BI "int ioctl(int " fd ", int " op ", void " *argp ");"
> 
> The '*' should be bold; not italics.
> 
> > +.fi
> > +.SH DESCRIPTION
> > +Various
> > +.BR ioctl (2)
> > +operations can be performed on an epoll file descriptor (created by a call
> > +to
> > +.BR epoll_create (2))
> > +(since Linux 6.9 and glibc 2.40) using calls of the form:
> > +.\" commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > +.P
> > +.in +4n
> > +.EX
> > +ioctl(fd, op, argp);
> > +.EE
> > +.in
> > +.P
> > +In the above,
> > +.I fd
> > +is a file descriptor referring to an epoll file descriptor obtained with a
> > +call to
> > +.BR epoll_create (2).
> > +.I op
> > +is one of the operations listed below, and
> > +.I argp
> > +is a pointer to the data structure described below.
> 
> If argp is a pointer to a structure, then the prototype should document
> that:
> 
> .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );
> 
> Also, I would document the two operations separately:
> 
> .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> 
> This allows documenting that the 'S' one doesn't modify the argp
> (AFAICS).
> 
> > +.\"
> > +.P
> > +All supported
> > +.I op
> > +values (described below) use an
> > +.I argp
> > +argument which is a pointer to a
> > +.I epoll_params
> > +structure, defined as:
> > +.P
> > +.in +4n
> > +.EX
> > +struct epoll_params {
> > +    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
> > +    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
> > +    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
> > +

The warning is triggered here.  You should use a dummy character to
avoid the warning:

\&

> > +    /* pad the struct to a multiple of 64bits */
> > +    uint8_t __pad;              /* Must be zero */
> > +};
> 
> You could add this type definition to the SYNOPSIS, below the function
> prototypes.
> 
> > +.EE
> > +.in
> > +.P
> > +The
> > +.I 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. This value cannot exceed
> > +.B INT_MAX .
> > +.in
> > +.P
> > +The
> > +.I busy_poll_budget
> > +field denotes the maximum number of packets that the network stack will
> > +be retrieved 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
> > +The
> > +.I 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
> > +The supported
> > +.I op
> > +values:
> > +.TP
> > +.B EPIOCSPARAMS
> > +This operation allows the caller to specify an
> > +.I epoll_params
> > +structure to configure the operation of epoll. Refer to the structure
> > +description of the structure above to learn what configuration is
> > +supported.
> > +.TP
> > +.B EPIOCGPARAMS
> > +This operation allows the caller to retrieve the current
> > +.I epoll_params
> > +structure. This can be used to determine what the current settings are.
> > +.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 ENOIOCTLCMD
> 
> Is this a thing?
> 
> 	$ grep -rn ENOIOCTLCMD /usr/include/
> 	$ 
> 
> I suspect this is EINVAL in user space?  (Actually, you list the same
> exact error condition for EINVAL below.)
> 
> > +The specified
> > +.I op
> > +is invalid.
> > +.TP
> > +.B EINVAL
> > +The
> > +.I fd
> > +specified is not an epoll file descriptor, or the
> > +.I op
> > +specified is invalid, or the
> > +.I __pad
> > +was not initialized to zero, or
> > +.I busy_poll_usecs
> > +exceeds
> > +.B INT_MAX ,
> > +or
> > +.I prefer_busy_poll
> > +is not 0 or 1.
> 
> Please separate the different conditions for EINVAL into separate
> entries.  The ones that are related can go in the same one, but the
> unrelated ones are better split.
> 
> > +.TP
> > +.B EPERM
> > +The process is being run without
> > +.I CAP_NET_ADMIN
> 
> This should be bold.
> 
> > +and the specified
> > +.I busy_poll_budget
> 
> This should be
> 
> .I argp.busy_poll_budget
> 
> > +exceeds
> > +.I NAPI_POLL_WEIGHT
> 
> This should be bold.
> 
> > +(which is 64 as of Linux 6.9).
> > +.TP
> > +.B EFAULT
> > +.I argp
> > +does not point to a valid memory address.
> > +.SH EXAMPLES
> > +.EX
> 
> Could you write an entire program, with a main()?
> 
> If not, it's fine; this is better than nothing.  But we prefer having
> entire programs that users can paste and try.
> 
> > +/* 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 Documentation/networking/napi.rst
> > +.P
> > +and
> > +.P
> > +.I Documentation/admin-guide/sysctl/net.rst
> > +.P
> > +in the Linux kernel source tree
> 
> We could document that as
> 
> .I linux.git/Documentation/...
> 
> to not have to say "in the Linux kernel source tree".
> 
> > 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
> > 
> 
> -- 
> <https://www.alejandro-colomar.es/>



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

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

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

[Joe Damato]

On Thu, Jun 06, 2024 at 11:46:47PM +0200, Alejandro Colomar wrote:
> On Thu, Jun 06, 2024 at 11:40:01PM GMT, Alejandro Colomar wrote:
> > Hi Joe,
> > 
> > On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > 
> > Thanks for the patch!  Please see a few comments below.
> 
> BTW, it triggers the following warning:
> 
> $ make lint build check -j24
> GROTTY		.tmp/man/man2/epoll_create.2.cat
> GROTTY		.tmp/man/man2/epoll_ctl.2.cat
> TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
> an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
> make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
> make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'

Hmm..

When I run make I get an error:

$ make
TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line

I tried to run the lint build check target and got an error about
checkpatch:

$ make lint build check -j8
CHECKPATCH	.tmp/man/man2/fork.2.d/fork.c.lint-c.checkpatch.touch
bash: line 1: checkpatch: command not found

I have mandoc, groff, and clang-tidy installed, but maybe I'm
missing other dependency?

> > 
> > Have a lovely night!
> > Alex
> > 
> > > ---
> > >  man/man2/epoll_create.2 |   1 +
> > >  man/man2/epoll_ctl.2    |   1 +
> > >  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
> > >  man/man7/epoll.7        |   1 +
> > >  4 files changed, 206 insertions(+)
> > >  create mode 100644 man/man2/ioctl_epoll.2
> > > 
> > > 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_epoll.2 b/man/man2/ioctl_epoll.2
> > > new file mode 100644
> > > index 000000000..1d53f458e
> > > --- /dev/null
> > > +++ b/man/man2/ioctl_epoll.2
> > > @@ -0,0 +1,203 @@
> > > +.\" Copyright (c) 2024, Joe Damato
> > > +.\" Written by Joe Damato <jdamato@fastly.com>
> > > +.\"
> > > +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> > > +.\"
> > > +.\"
> > 
> > Please use only one consecutive .\"
> > 
> > > +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> > > +.SH NAME
> > > +ioctl_epoll \- ioctl() operations for epoll file descriptors
> > 
> > Please add link pages <man2const/EPIOCSPARAMS.2const> and
> > <man2const/EPIOCGPARAMS.2const>.
> > 
> > > +.SH LIBRARY
> > > +Standard C library
> > > +.RI ( libc ", " \-lc )
> > > +.SH SYNOPSIS
> > > +.nf
> > > +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
> > > +.B "#include <sys/ioctl.h>"
> > > +.P
> > > +.BI "int ioctl(int " fd ", int " op ", void " *argp ");"
> > 
> > The '*' should be bold; not italics.
> > 
> > > +.fi
> > > +.SH DESCRIPTION
> > > +Various
> > > +.BR ioctl (2)
> > > +operations can be performed on an epoll file descriptor (created by a call
> > > +to
> > > +.BR epoll_create (2))
> > > +(since Linux 6.9 and glibc 2.40) using calls of the form:
> > > +.\" commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > > +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > > +.P
> > > +.in +4n
> > > +.EX
> > > +ioctl(fd, op, argp);
> > > +.EE
> > > +.in
> > > +.P
> > > +In the above,
> > > +.I fd
> > > +is a file descriptor referring to an epoll file descriptor obtained with a
> > > +call to
> > > +.BR epoll_create (2).
> > > +.I op
> > > +is one of the operations listed below, and
> > > +.I argp
> > > +is a pointer to the data structure described below.
> > 
> > If argp is a pointer to a structure, then the prototype should document
> > that:
> > 
> > .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );
> > 
> > Also, I would document the two operations separately:
> > 
> > .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> > 
> > This allows documenting that the 'S' one doesn't modify the argp
> > (AFAICS).
> > 
> > > +.\"
> > > +.P
> > > +All supported
> > > +.I op
> > > +values (described below) use an
> > > +.I argp
> > > +argument which is a pointer to a
> > > +.I epoll_params
> > > +structure, defined as:
> > > +.P
> > > +.in +4n
> > > +.EX
> > > +struct epoll_params {
> > > +    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
> > > +    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
> > > +    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
> > > +
> 
> The warning is triggered here.  You should use a dummy character to
> avoid the warning:
> 
> \&
> 
> > > +    /* pad the struct to a multiple of 64bits */
> > > +    uint8_t __pad;              /* Must be zero */
> > > +};
> > 
> > You could add this type definition to the SYNOPSIS, below the function
> > prototypes.
> > 
> > > +.EE
> > > +.in
> > > +.P
> > > +The
> > > +.I 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. This value cannot exceed
> > > +.B INT_MAX .
> > > +.in
> > > +.P
> > > +The
> > > +.I busy_poll_budget
> > > +field denotes the maximum number of packets that the network stack will
> > > +be retrieved 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
> > > +The
> > > +.I 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
> > > +The supported
> > > +.I op
> > > +values:
> > > +.TP
> > > +.B EPIOCSPARAMS
> > > +This operation allows the caller to specify an
> > > +.I epoll_params
> > > +structure to configure the operation of epoll. Refer to the structure
> > > +description of the structure above to learn what configuration is
> > > +supported.
> > > +.TP
> > > +.B EPIOCGPARAMS
> > > +This operation allows the caller to retrieve the current
> > > +.I epoll_params
> > > +structure. This can be used to determine what the current settings are.
> > > +.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 ENOIOCTLCMD
> > 
> > Is this a thing?
> > 
> > 	$ grep -rn ENOIOCTLCMD /usr/include/
> > 	$ 
> > 
> > I suspect this is EINVAL in user space?  (Actually, you list the same
> > exact error condition for EINVAL below.)
> > 
> > > +The specified
> > > +.I op
> > > +is invalid.
> > > +.TP
> > > +.B EINVAL
> > > +The
> > > +.I fd
> > > +specified is not an epoll file descriptor, or the
> > > +.I op
> > > +specified is invalid, or the
> > > +.I __pad
> > > +was not initialized to zero, or
> > > +.I busy_poll_usecs
> > > +exceeds
> > > +.B INT_MAX ,
> > > +or
> > > +.I prefer_busy_poll
> > > +is not 0 or 1.
> > 
> > Please separate the different conditions for EINVAL into separate
> > entries.  The ones that are related can go in the same one, but the
> > unrelated ones are better split.
> > 
> > > +.TP
> > > +.B EPERM
> > > +The process is being run without
> > > +.I CAP_NET_ADMIN
> > 
> > This should be bold.
> > 
> > > +and the specified
> > > +.I busy_poll_budget
> > 
> > This should be
> > 
> > .I argp.busy_poll_budget
> > 
> > > +exceeds
> > > +.I NAPI_POLL_WEIGHT
> > 
> > This should be bold.
> > 
> > > +(which is 64 as of Linux 6.9).
> > > +.TP
> > > +.B EFAULT
> > > +.I argp
> > > +does not point to a valid memory address.
> > > +.SH EXAMPLES
> > > +.EX
> > 
> > Could you write an entire program, with a main()?
> > 
> > If not, it's fine; this is better than nothing.  But we prefer having
> > entire programs that users can paste and try.
> > 
> > > +/* 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 Documentation/networking/napi.rst
> > > +.P
> > > +and
> > > +.P
> > > +.I Documentation/admin-guide/sysctl/net.rst
> > > +.P
> > > +in the Linux kernel source tree
> > 
> > We could document that as
> > 
> > .I linux.git/Documentation/...
> > 
> > to not have to say "in the Linux kernel source tree".
> > 
> > > 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
> > > 
> > 
> > -- 
> > <https://www.alejandro-colomar.es/>
> 
> 
> 
> -- 
> <https://www.alejandro-colomar.es/>

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

[Alejandro Colomar]

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

Hi Joe,

On Fri, Jun 07, 2024 at 02:53:19PM GMT, Joe Damato wrote:
> On Thu, Jun 06, 2024 at 11:46:47PM +0200, Alejandro Colomar wrote:
> > $ make lint build check -j24
> > GROTTY		.tmp/man/man2/epoll_create.2.cat
> > GROTTY		.tmp/man/man2/epoll_ctl.2.cat
> > TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
> > an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
> > make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
> > make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'
> 
> Hmm..
> 
> When I run make I get an error:
> 
> $ make
> TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
> troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line

Hmm, I can't reproduce it.  Can you run with `make --debug=print` (needs
a recent make(1))?

> I tried to run the lint build check target and got an error about
> checkpatch:
> 
> $ make lint build check -j8
> CHECKPATCH	.tmp/man/man2/fork.2.d/fork.c.lint-c.checkpatch.touch
> bash: line 1: checkpatch: command not found
> 
> I have mandoc, groff, and clang-tidy installed, but maybe I'm
> missing other dependency?

That's a fork of the checkpatch.pl from the kernel.  I'm working on a
repository to make it public.  Don't worry about it.

You can `make -t lint-c-checkpatch` to ignore all checkpatch lints.

Have a lovely day!
Alex

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

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

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

[Joe Damato]

On Sun, Jun 09, 2024 at 07:04:40PM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Fri, Jun 07, 2024 at 02:53:19PM GMT, Joe Damato wrote:
> > On Thu, Jun 06, 2024 at 11:46:47PM +0200, Alejandro Colomar wrote:
> > > $ make lint build check -j24
> > > GROTTY		.tmp/man/man2/epoll_create.2.cat
> > > GROTTY		.tmp/man/man2/epoll_ctl.2.cat
> > > TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
> > > an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
> > > make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
> > > make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'
> > 
> > Hmm..
> > 
> > When I run make I get an error:
> > 
> > $ make
> > TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
> > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> 
> Hmm, I can't reproduce it.  Can you run with `make --debug=print` (needs
> a recent make(1))?

I don't think I have a recent enough make:

$ make --debug=print
make: *** unknown debug level specification 'print'.  Stop.
$ make --version
GNU Make 4.3


> > I tried to run the lint build check target and got an error about
> > checkpatch:
> > 
> > $ make lint build check -j8
> > CHECKPATCH	.tmp/man/man2/fork.2.d/fork.c.lint-c.checkpatch.touch
> > bash: line 1: checkpatch: command not found
> > 
> > I have mandoc, groff, and clang-tidy installed, but maybe I'm
> > missing other dependency?
> 
> That's a fork of the checkpatch.pl from the kernel.  I'm working on a
> repository to make it public.  Don't worry about it.
> 
> You can `make -t lint-c-checkpatch` to ignore all checkpatch lints.

$ make -t lint-c-checkpatch
$ echo $?
0

Does that mean I'm good to go and ready to submit v2 ? ;)

> 
> Have a lovely day!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>

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

[Alejandro Colomar]

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

Hi Joe,

On Mon, Jun 10, 2024 at 10:29:44AM GMT, Joe Damato wrote:
> On Sun, Jun 09, 2024 at 07:04:40PM +0200, Alejandro Colomar wrote:
> > On Fri, Jun 07, 2024 at 02:53:19PM GMT, Joe Damato wrote:
> > > On Thu, Jun 06, 2024 at 11:46:47PM +0200, Alejandro Colomar wrote:
> > > > $ make lint build check -j24
> > > > GROTTY		.tmp/man/man2/epoll_create.2.cat
> > > > GROTTY		.tmp/man/man2/epoll_ctl.2.cat
> > > > TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
> > > > an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
> > > > make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
> > > > make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'
> > > 
> > > Hmm..
> > > 
> > > When I run make I get an error:
> > > 
> > > $ make
> > > TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
> > > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> > 
> > Hmm, I can't reproduce it.  Can you run with `make --debug=print` (needs
> > a recent make(1))?
> 
> I don't think I have a recent enough make:
> 
> $ make --debug=print
> make: *** unknown debug level specification 'print'.  Stop.
> $ make --version
> GNU Make 4.3

You can do this:

	sed -i '/SILENT/s/^/$(V)/' GNUmakefile;

And after that, you can `make V=1` to see more verbosity.  This will
work with any make(1).  And I would appreciate if you can ping your
distro maintainer to package a newer version of make(1).  (Most likely,
you're on Debian (right?), where the maintainer of make(1) is AWOL, so
hopefully somebody else will pick it up if there are many pings.)  :-)

> > > I tried to run the lint build check target and got an error about
> > > checkpatch:
> > > 
> > > $ make lint build check -j8
> > > CHECKPATCH	.tmp/man/man2/fork.2.d/fork.c.lint-c.checkpatch.touch
> > > bash: line 1: checkpatch: command not found
> > > 
> > > I have mandoc, groff, and clang-tidy installed, but maybe I'm
> > > missing other dependency?
> > 
> > That's a fork of the checkpatch.pl from the kernel.  I'm working on a
> > repository to make it public.  Don't worry about it.
> > 
> > You can `make -t lint-c-checkpatch` to ignore all checkpatch lints.
> 
> $ make -t lint-c-checkpatch
> $ echo $?
> 0
> 
> Does that mean I'm good to go and ready to submit v2 ? ;)

Nope.  That means you're ready to `make`, and you won't see any errors
due to missing a checkpatch binary.  make -t is a trick that few know,
but quite useful:

$ MANWIDTH=72 man make | sed -n '/   -t/,/^$/p'
     -t, --touch
          Touch  files  (mark  them  up to date without really changing
          them) instead of running their commands.   This  is  used  to
          pretend  that the commands were done, in order to fool future
          invocations of make.

So what we did is trick make(1) to think that it has successfully run
the 'lint-c-checkpatch', by touch(1)ing all the files that would have
been created if that target had been successful.

Have a lovely day!
Alex

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

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

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

[Joe Damato]

On Mon, Jun 10, 2024 at 08:24:24PM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Mon, Jun 10, 2024 at 10:29:44AM GMT, Joe Damato wrote:
> > On Sun, Jun 09, 2024 at 07:04:40PM +0200, Alejandro Colomar wrote:
> > > On Fri, Jun 07, 2024 at 02:53:19PM GMT, Joe Damato wrote:
> > > > On Thu, Jun 06, 2024 at 11:46:47PM +0200, Alejandro Colomar wrote:
> > > > > $ make lint build check -j24
> > > > > GROTTY		.tmp/man/man2/epoll_create.2.cat
> > > > > GROTTY		.tmp/man/man2/epoll_ctl.2.cat
> > > > > TROFF		.tmp/man/man2/ioctl_epoll.2.cat.set
> > > > > an.tmac:.tmp/man/man2/ioctl_epoll.2:61: style: blank line in input
> > > > > make: *** [/home/alx/src/linux/man-pages/man-pages/contrib/share/mk/build/catman/troff.mk:63: .tmp/man/man2/ioctl_epoll.2.cat.set] Error 1
> > > > > make: *** Deleting file '.tmp/man/man2/ioctl_epoll.2.cat.set'
> > > > 
> > > > Hmm..
> > > > 
> > > > When I run make I get an error:
> > > > 
> > > > $ make
> > > > TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
> > > > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> > > 
> > > Hmm, I can't reproduce it.  Can you run with `make --debug=print` (needs
> > > a recent make(1))?
> > 
> > I don't think I have a recent enough make:
> > 
> > $ make --debug=print
> > make: *** unknown debug level specification 'print'.  Stop.
> > $ make --version
> > GNU Make 4.3
> 
> You can do this:
> 
> 	sed -i '/SILENT/s/^/$(V)/' GNUmakefile;
> 
> And after that, you can `make V=1` to see more verbosity.  This will
> work with any make(1).  And I would appreciate if you can ping your
> distro maintainer to package a newer version of make(1).  (Most likely,
> you're on Debian (right?), where the maintainer of make(1) is AWOL, so
> hopefully somebody else will pick it up if there are many pings.)  :-)

I am using Ubuntu 22.04.

I did what you suggested got the same output about s390_sthyi,
here's what I think is the relevant output:

SED		.tmp/man/man2/s390_sthyi.2
<man/man2/s390_sthyi.2 \
sed "/^\.TH/s/(date)/$(git log --format=%cs -1 -- man/man2/s390_sthyi.2 2>/dev/null)/" \
| sed '/^\.TH/s/(unreleased)/6.8-152-g97abd8f14-dirty/' >.tmp/man/man2/s390_sthyi.2
PRECONV		.tmp/man/man2/s390_sthyi.2.tbl
preconv   .tmp/man/man2/s390_sthyi.2 >.tmp/man/man2/s390_sthyi.2.tbl
TBL		.tmp/man/man2/s390_sthyi.2.eqn
tbl <.tmp/man/man2/s390_sthyi.2.tbl >.tmp/man/man2/s390_sthyi.2.eqn
EQN		.tmp/man/man2/s390_sthyi.2.cat.troff
! (eqn -Tutf8   <.tmp/man/man2/s390_sthyi.2.eqn 2>&1 >.tmp/man/man2/s390_sthyi.2.cat.troff) \
| grep ^ >&2
TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
! (troff -man -wbreak -rS12  -Tutf8 -rLL=78n -rCHECKSTYLE=3 -ww  <.tmp/man/man2/s390_sthyi.2.cat.troff 2>&1 >.tmp/man/man2/s390_sthyi.2.cat.set \
   | grep -v -f '/home/jdamato/code/man-pages/share/mk/build/catman/troff.ignore.grep' \
   || true; \
) \
| grep ^ >&2
troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
make: *** [/home/jdamato/code/man-pages/share/mk/build/catman/troff.mk:63: .tmp/man/man2/s390_sthyi.2.cat.set] Error 1

> > > > I tried to run the lint build check target and got an error about
> > > > checkpatch:
> > > > 
> > > > $ make lint build check -j8
> > > > CHECKPATCH	.tmp/man/man2/fork.2.d/fork.c.lint-c.checkpatch.touch
> > > > bash: line 1: checkpatch: command not found
> > > > 
> > > > I have mandoc, groff, and clang-tidy installed, but maybe I'm
> > > > missing other dependency?
> > > 
> > > That's a fork of the checkpatch.pl from the kernel.  I'm working on a
> > > repository to make it public.  Don't worry about it.
> > > 
> > > You can `make -t lint-c-checkpatch` to ignore all checkpatch lints.
> > 
> > $ make -t lint-c-checkpatch
> > $ echo $?
> > 0
> > 
> > Does that mean I'm good to go and ready to submit v2 ? ;)
> 
> Nope.  That means you're ready to `make`, and you won't see any errors
> due to missing a checkpatch binary.  make -t is a trick that few know,
> but quite useful:
> 
> $ MANWIDTH=72 man make | sed -n '/   -t/,/^$/p'
>      -t, --touch
>           Touch  files  (mark  them  up to date without really changing
>           them) instead of running their commands.   This  is  used  to
>           pretend  that the commands were done, in order to fool future
>           invocations of make.
> 
> So what we did is trick make(1) to think that it has successfully run
> the 'lint-c-checkpatch', by touch(1)ing all the files that would have
> been created if that target had been successful.

Ah, I see -- thanks for the pointer!

I've re-run make and it is still failing as above but on an
unrelated file.

I see in the output that ioctl_epoll seemed to be processed OK:

SED             .tmp/man/man2/ioctl_epoll.2
<man/man2/ioctl_epoll.2 \
sed "/^\.TH/s/(date)/$(git log --format=%cs -1 -- man/man2/ioctl_epoll.2 2>/dev/null)/" \
| sed '/^\.TH/s/(unreleased)/6.8-152-g97abd8f14-dirty/' >.tmp/man/man2/ioctl_epoll.2
PRECONV         .tmp/man/man2/ioctl_epoll.2.tbl
preconv   .tmp/man/man2/ioctl_epoll.2 >.tmp/man/man2/ioctl_epoll.2.tbl
TBL             .tmp/man/man2/ioctl_epoll.2.eqn
tbl <.tmp/man/man2/ioctl_epoll.2.tbl >.tmp/man/man2/ioctl_epoll.2.eqn
EQN             .tmp/man/man2/ioctl_epoll.2.cat.troff
! (eqn -Tutf8   <.tmp/man/man2/ioctl_epoll.2.eqn 2>&1 >.tmp/man/man2/ioctl_epoll.2.cat.troff) \
| grep ^ >&2
TROFF           .tmp/man/man2/ioctl_epoll.2.cat.set
! (troff -man -wbreak -rS12  -Tutf8 -rLL=78n -rCHECKSTYLE=3 -ww  <.tmp/man/man2/ioctl_epoll.2.cat.troff 2>&1 >.tmp/man/man2/ioctl_epoll.2.cat.set \
   | grep -v -f '/home/jdamato/code/man-pages/share/mk/build/catman/troff.ignore.grep' \
   || true; \
) \
| grep ^ >&2
GROTTY          .tmp/man/man2/ioctl_epoll.2.cat
grotty -c  <.tmp/man/man2/ioctl_epoll.2.cat.set >.tmp/man/man2/ioctl_epoll.2.cat

Should I send a v2? The s390 thing seems unrelated?

What do you think?

Thanks,
Joe

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

[Alejandro Colomar]

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

Hi Joe,

On Mon, Jun 10, 2024 at 01:16:15PM GMT, Joe Damato wrote:
> I am using Ubuntu 22.04.

Yep, they get the upstream package, I guess.

> I did what you suggested got the same output about s390_sthyi,
> here's what I think is the relevant output:
> 
> SED		.tmp/man/man2/s390_sthyi.2
> <man/man2/s390_sthyi.2 \
> sed "/^\.TH/s/(date)/$(git log --format=%cs -1 -- man/man2/s390_sthyi.2 2>/dev/null)/" \
> | sed '/^\.TH/s/(unreleased)/6.8-152-g97abd8f14-dirty/' >.tmp/man/man2/s390_sthyi.2
> PRECONV		.tmp/man/man2/s390_sthyi.2.tbl
> preconv   .tmp/man/man2/s390_sthyi.2 >.tmp/man/man2/s390_sthyi.2.tbl
> TBL		.tmp/man/man2/s390_sthyi.2.eqn
> tbl <.tmp/man/man2/s390_sthyi.2.tbl >.tmp/man/man2/s390_sthyi.2.eqn
> EQN		.tmp/man/man2/s390_sthyi.2.cat.troff
> ! (eqn -Tutf8   <.tmp/man/man2/s390_sthyi.2.eqn 2>&1 >.tmp/man/man2/s390_sthyi.2.cat.troff) \
> | grep ^ >&2
> TROFF		.tmp/man/man2/s390_sthyi.2.cat.set
> ! (troff -man -wbreak -rS12  -Tutf8 -rLL=78n -rCHECKSTYLE=3 -ww  <.tmp/man/man2/s390_sthyi.2.cat.troff 2>&1 >.tmp/man/man2/s390_sthyi.2.cat.set \
>    | grep -v -f '/home/jdamato/code/man-pages/share/mk/build/catman/troff.ignore.grep' \
>    || true; \
> ) \
> | grep ^ >&2
> troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> make: *** [/home/jdamato/code/man-pages/share/mk/build/catman/troff.mk:63: .tmp/man/man2/s390_sthyi.2.cat.set] Error 1

Hmmm; can't reproduce it.  My only difference (AFAICS) is I have
groff-1.23.0, while you'll have groff-1.22.4.  1.22.4 has many many
bugs, so I guess this is one of them.  You can skip this specific error
with

	touch .tmp/man/man2/s390_sthyi.2.cat.set;

Thanks!

> > So what we did is trick make(1) to think that it has successfully run
> > the 'lint-c-checkpatch', by touch(1)ing all the files that would have
> > been created if that target had been successful.
> 
> Ah, I see -- thanks for the pointer!

:-)

> I've re-run make and it is still failing as above but on an
> unrelated file.
> 
> I see in the output that ioctl_epoll seemed to be processed OK:
> 
> SED             .tmp/man/man2/ioctl_epoll.2
> <man/man2/ioctl_epoll.2 \
> sed "/^\.TH/s/(date)/$(git log --format=%cs -1 -- man/man2/ioctl_epoll.2 2>/dev/null)/" \
> | sed '/^\.TH/s/(unreleased)/6.8-152-g97abd8f14-dirty/' >.tmp/man/man2/ioctl_epoll.2
> PRECONV         .tmp/man/man2/ioctl_epoll.2.tbl
> preconv   .tmp/man/man2/ioctl_epoll.2 >.tmp/man/man2/ioctl_epoll.2.tbl
> TBL             .tmp/man/man2/ioctl_epoll.2.eqn
> tbl <.tmp/man/man2/ioctl_epoll.2.tbl >.tmp/man/man2/ioctl_epoll.2.eqn
> EQN             .tmp/man/man2/ioctl_epoll.2.cat.troff
> ! (eqn -Tutf8   <.tmp/man/man2/ioctl_epoll.2.eqn 2>&1 >.tmp/man/man2/ioctl_epoll.2.cat.troff) \
> | grep ^ >&2
> TROFF           .tmp/man/man2/ioctl_epoll.2.cat.set
> ! (troff -man -wbreak -rS12  -Tutf8 -rLL=78n -rCHECKSTYLE=3 -ww  <.tmp/man/man2/ioctl_epoll.2.cat.troff 2>&1 >.tmp/man/man2/ioctl_epoll.2.cat.set \
>    | grep -v -f '/home/jdamato/code/man-pages/share/mk/build/catman/troff.ignore.grep' \
>    || true; \
> ) \
> | grep ^ >&2
> GROTTY          .tmp/man/man2/ioctl_epoll.2.cat
> grotty -c  <.tmp/man/man2/ioctl_epoll.2.cat.set >.tmp/man/man2/ioctl_epoll.2.cat
> 
> Should I send a v2? The s390 thing seems unrelated?

Sure; this seems to be good.  I may notice some warnings that are new in
groff-1.23.0 and not present in 1.22.4, but I'll let you know if I see
them.

> What do you think?

Have a lovely night!
Alex

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

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

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

[G. Branden Robinson]

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

Hi Alex,

At 2024-06-11T00:21:59+0200, Alejandro Colomar wrote:
> > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> > make: *** [/home/jdamato/code/man-pages/share/mk/build/catman/troff.mk:63: .tmp/man/man2/s390_sthyi.2.cat.set] Error 1
> 
> Hmmm; can't reproduce it.  My only difference (AFAICS) is I have
> groff-1.23.0, while you'll have groff-1.22.4.  1.22.4 has many many
> bugs, so I guess this is one of them.  You can skip this specific
> error with
> 
> 	touch .tmp/man/man2/s390_sthyi.2.cat.set;

groff 1.22.4 did have, by my count, over 400 bugs in it that groff
1.23.0 fixed.[1]  However this is not one of them.  groff 1.23.0 still
complains.

Let us recall what "adjustment" is in typography.

5.1.5 Adjustment
----------------

After GNU 'troff' performs an automatic break, it may then "adjust" the
line, widening inter-word spaces until the text reaches the right
margin.  Extra spaces between words are preserved.  Leading and trailing
spaces are handled as noted above.  Text can be aligned to the left or
right margin only, or centered; see *note Manipulating Filling and
Adjustment::.

> > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line

What is at line 124 of this document?

$ cat -n ./man2/s390_sthyi.2 | sed -n '120,125p'
   120  .SH NOTES
   121  For details of the STHYI instruction, see
   122  .UR https://www.ibm.com\:/support\:/knowledgecenter\:/SSB27U_6.3.0\:/com.ibm.zvm.v630.hcpb4\:/hcpb4sth.htm
   123  the documentation page
   124  .UE .
   125  .P

>> cannot adjust line

This is a legitimate diagnostic arising from a ridiculously long URL
colliding with the formatter's frustrated attempt to adjust the output
line.  Here's how that renders.

NOTES
       For  details  of  the  STHYI  instruction,  see  the documentation page
       ⟨https://www.ibm.com/support/knowledgecenter/SSB27U_6.3.0
       /com.ibm.zvm.v630.hcpb4/hcpb4sth.htm⟩.

Not much the formatter can do about this monstrosity.  People with
crazily wide terminal windows, or with adjustment of man pages disabled
(a groff 1.23.0 feature)[2] won't experience the warning.

You _could_ make it less obnoxious in the source document with input
line continuation.

.UR https://www.ibm.com\:/support\:/knowledgecenter\:/SSB27U_6.3.0\:/\
com.ibm.zvm.v630.hcpb4\:/hcpb4sth.htm

Regards,
Branden

[1] https://lists.gnu.org/archive/html/groff/2023-07/msg00051.html

[2] NEWS:

o The an (man) and doc (mdoc) macro packages support a new `AD` string
  to put the default adjustment mode under user control at rendering
  time.  The default is "b" (adjust lines to both margins) as has been
  the Unix man(7) default for typesetters since 1979.

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

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

[Alejandro Colomar]

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

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

Hi Branden,

> groff 1.22.4 did have, by my count, over 400 bugs in it that groff
> 1.23.0 fixed.[1]  However this is not one of them.  groff 1.23.0 still
> complains.

Hummm.

> Let us recall what "adjustment" is in typography.
> 
> 5.1.5 Adjustment
> ----------------
> 
> After GNU 'troff' performs an automatic break, it may then "adjust" the
> line, widening inter-word spaces until the text reaches the right
> margin.  Extra spaces between words are preserved.  Leading and trailing
> spaces are handled as noted above.  Text can be aligned to the left or
> right margin only, or centered; see *note Manipulating Filling and
> Adjustment::.
> 
> > > troff: .tmp/man/man2/s390_sthyi.2:124: warning [p 2, 2.8i]: cannot adjust line
> 
> What is at line 124 of this document?
> 
> $ cat -n ./man2/s390_sthyi.2 | sed -n '120,125p'
>    120  .SH NOTES
>    121  For details of the STHYI instruction, see
>    122  .UR https://www.ibm.com\:/support\:/knowledgecenter\:/SSB27U_6.3.0\:/com.ibm.zvm.v630.hcpb4\:/hcpb4sth.htm
>    123  the documentation page
>    124  .UE .
>    125  .P
> 
> >> cannot adjust line
> 
> This is a legitimate diagnostic arising from a ridiculously long URL
> colliding with the formatter's frustrated attempt to adjust the output
> line.  Here's how that renders.
> 
> NOTES
>        For  details  of  the  STHYI  instruction,  see  the documentation page
>        ⟨https://www.ibm.com/support/knowledgecenter/SSB27U_6.3.0
>        /com.ibm.zvm.v630.hcpb4/hcpb4sth.htm⟩.

I see:

NOTES
     For details of the STHYI instruction, see the documentation page.

In xfce4-terminal(1), that's an underdotted hyperlink.  In xterm(1), I
see the same, but it's not underdotted, and seems not clickable.

Why am I unable to see the explicit URI?  That seems the reason why
I can't reproduce the warning.

> Not much the formatter can do about this monstrosity.  People with
> crazily wide terminal windows, or with adjustment of man pages disabled
> (a groff 1.23.0 feature)[2] won't experience the warning.
> 
> You _could_ make it less obnoxious in the source document with input
> line continuation.
> 
> .UR https://www.ibm.com\:/support\:/knowledgecenter\:/SSB27U_6.3.0\:/\
> com.ibm.zvm.v630.hcpb4\:/hcpb4sth.htm

I don't like breaking URIs.  I'll accept the warning and add it as an
XFAIL.  However, I'd like to be able to reproduce it.  :|

Have a lovely day!
Alex

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

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

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

[G. Branden Robinson]

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

Hi Alex,

At 2024-06-11T16:34:54+0200, Alejandro Colomar wrote:
> I see:
> 
> NOTES
>      For details of the STHYI instruction, see the documentation page.
> 
> In xfce4-terminal(1), that's an underdotted hyperlink.  In xterm(1), I
> see the same, but it's not underdotted, and seems not clickable.

Ahhh, I reckon you have OSC 8 hyperlinks turned on.  This defaulted off
in stock groff 1.23.0, but some distributors may have turned it on.
(Which is fine--that's what man.local is there for.)

groff_man(7) (from my working copy):
     -rU0     Disable generation of URI hyperlinks in output drivers
              capable of them, making the arguments to MT and UR calls
              visible as formatted text.  grohtml(1), gropdf(1), and
              grotty(1) enable hyperlinks by default (the last only if
              not in legacy output mode).

So, for you, GNU troff is not complaining about being unable to adjust
MC 900 Foot URL for formatting...because it's _not trying to format it_.

> Why am I unable to see the explicit URI?  That seems the reason why
> I can't reproduce the warning.

With register `U` set to a true value, GNU troff assumes your output
device is capable of making a hyperlink clickable.  It can't otherwise
know.  (Well, it knows that some devices have no such capability.[1])

> I don't like breaking URIs.  I'll accept the warning and add it as an
> XFAIL.  However, I'd like to be able to reproduce it.  :|

Try running groff with `-rU0` (or `-r U=0`).  That should turn it up.

Regards,
Branden

[1] Two things I'd like to see:

    A.  ...an extended ("user") capability enabling terminfo
    applications to query a terminal (emulator) whether it supports OSC
    8 sequences so that it can then do the right thing.  See
    user_caps(5).

    B.  ...xterm(1) support for OSC 8 hyperlinks only as far as marking
    them and giving you a menu option to copy the link to the clipboard
    (or the primary selection buffer).[2]  Thomas Dickey has been pretty
    skeptical of OSC 8, and I don't blame him for not wanting to get
    into the URL management business--it presses people's security
    buttons.  I am hoping my suggestion is a Solomonic one.

[2] So you'd probably have to middle-click while hovering over the link
    text.  Even if implemented, I'm sure that this would be yet another
    feature gated behind an X resource and command-line option.  ;-)

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

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

[Alejandro Colomar]

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

On Tue, Jun 11, 2024 at 11:06:32AM GMT, G. Branden Robinson wrote:
> Hi Alex,

Hi Branden,

> At 2024-06-11T16:34:54+0200, Alejandro Colomar wrote:
> > I see:
> > 
> > NOTES
> >      For details of the STHYI instruction, see the documentation page.
> > 
> > In xfce4-terminal(1), that's an underdotted hyperlink.  In xterm(1), I
> > see the same, but it's not underdotted, and seems not clickable.
> 
> Ahhh, I reckon you have OSC 8 hyperlinks turned on.  This defaulted off
> in stock groff 1.23.0, but some distributors may have turned it on.
> (Which is fine--that's what man.local is there for.)
> 
> groff_man(7) (from my working copy):
>      -rU0     Disable generation of URI hyperlinks in output drivers
>               capable of them, making the arguments to MT and UR calls
>               visible as formatted text.  grohtml(1), gropdf(1), and
>               grotty(1) enable hyperlinks by default (the last only if
>               not in legacy output mode).

Hmmm; thanks!  I suspected some of this was probably the fault, but
didn't know what exactly.  I'll add -rU0 to the build system, since that
will trigger more warnings.

Have a lovely day!
Alex

> So, for you, GNU troff is not complaining about being unable to adjust
> MC 900 Foot URL for formatting...because it's _not trying to format it_.
> 
> > Why am I unable to see the explicit URI?  That seems the reason why
> > I can't reproduce the warning.
> 
> With register `U` set to a true value, GNU troff assumes your output
> device is capable of making a hyperlink clickable.  It can't otherwise
> know.  (Well, it knows that some devices have no such capability.[1])
> 
> > I don't like breaking URIs.  I'll accept the warning and add it as an
> > XFAIL.  However, I'd like to be able to reproduce it.  :|
> 
> Try running groff with `-rU0` (or `-r U=0`).  That should turn it up.
> 
> Regards,
> Branden
> 
> [1] Two things I'd like to see:
> 
>     A.  ...an extended ("user") capability enabling terminfo
>     applications to query a terminal (emulator) whether it supports OSC
>     8 sequences so that it can then do the right thing.  See
>     user_caps(5).
> 
>     B.  ...xterm(1) support for OSC 8 hyperlinks only as far as marking
>     them and giving you a menu option to copy the link to the clipboard
>     (or the primary selection buffer).[2]  Thomas Dickey has been pretty
>     skeptical of OSC 8, and I don't blame him for not wanting to get
>     into the URL management business--it presses people's security
>     buttons.  I am hoping my suggestion is a Solomonic one.
> 
> [2] So you'd probably have to middle-click while hovering over the link
>     text.  Even if implemented, I'm sure that this would be yet another
>     feature gated behind an X resource and command-line option.  ;-)



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

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

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

[Joe Damato]

On Thu, Jun 06, 2024 at 11:39:58PM +0200, Alejandro Colomar wrote:
> Hi Joe,
> 
> On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> 
> Thanks for the patch!  Please see a few comments below.

I've included some questions below just to make sure I give you a v2
that is much closer to correct :)

> 
> > ---
> >  man/man2/epoll_create.2 |   1 +
> >  man/man2/epoll_ctl.2    |   1 +
> >  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
> >  man/man7/epoll.7        |   1 +
> >  4 files changed, 206 insertions(+)
> >  create mode 100644 man/man2/ioctl_epoll.2
> > 
> > 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_epoll.2 b/man/man2/ioctl_epoll.2
> > new file mode 100644
> > index 000000000..1d53f458e
> > --- /dev/null
> > +++ b/man/man2/ioctl_epoll.2
> > @@ -0,0 +1,203 @@
> > +.\" Copyright (c) 2024, Joe Damato
> > +.\" Written by Joe Damato <jdamato@fastly.com>
> > +.\"
> > +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> > +.\"
> > +.\"
> 
> Please use only one consecutive .\"

Thanks, fixed.
 
> > +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> > +.SH NAME
> > +ioctl_epoll \- ioctl() operations for epoll file descriptors
> 
> Please add link pages <man2const/EPIOCSPARAMS.2const> and
> <man2const/EPIOCGPARAMS.2const>.

And after adding those, I'd add those to SEE ALSO and I'd omit the
description of them from the ioctl_epoll.2 page (because they'd have
their own pages) ?
 
> > +.SH LIBRARY
> > +Standard C library
> > +.RI ( libc ", " \-lc )
> > +.SH SYNOPSIS
> > +.nf
> > +.BR "#include <linux/eventpoll.h>" "  /* Definition of " EPIOC* " constants and struct epoll_params */"
> > +.B "#include <sys/ioctl.h>"
> > +.P
> > +.BI "int ioctl(int " fd ", int " op ", void " *argp ");"
> 
> The '*' should be bold; not italics.

Thanks, fixed.

> > +.fi
> > +.SH DESCRIPTION
> > +Various
> > +.BR ioctl (2)
> > +operations can be performed on an epoll file descriptor (created by a call
> > +to
> > +.BR epoll_create (2))
> > +(since Linux 6.9 and glibc 2.40) using calls of the form:
> > +.\" commit 18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > +.\" glibc commit 92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > +.P
> > +.in +4n
> > +.EX
> > +ioctl(fd, op, argp);
> > +.EE
> > +.in
> > +.P
> > +In the above,
> > +.I fd
> > +is a file descriptor referring to an epoll file descriptor obtained with a
> > +call to
> > +.BR epoll_create (2).
> > +.I op
> > +is one of the operations listed below, and
> > +.I argp
> > +is a pointer to the data structure described below.
> 
> If argp is a pointer to a structure, then the prototype should document
> that:
> 
> .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );
> 
> Also, I would document the two operations separately:
> 
> .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> 
> This allows documenting that the 'S' one doesn't modify the argp
> (AFAICS).

Do you mean that I should omit the generic 

 .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );

and instead have the above with the two ops?
 
> > +.\"
> > +.P
> > +All supported
> > +.I op
> > +values (described below) use an
> > +.I argp
> > +argument which is a pointer to a
> > +.I epoll_params
> > +structure, defined as:
> > +.P
> > +.in +4n
> > +.EX
> > +struct epoll_params {
> > +    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
> > +    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
> > +    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
> > +
> > +    /* pad the struct to a multiple of 64bits */
> > +    uint8_t __pad;              /* Must be zero */
> > +};
> 
> You could add this type definition to the SYNOPSIS, below the function
> prototypes.

OK, I moved it.

I'm not sure if it is formatted properly, though. I'll see if I can
find other examples of this style to check against.

> > +.EE
> > +.in
> > +.P
> > +The
> > +.I 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. This value cannot exceed
> > +.B INT_MAX .
> > +.in
> > +.P
> > +The
> > +.I busy_poll_budget
> > +field denotes the maximum number of packets that the network stack will
> > +be retrieved 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
> > +The
> > +.I 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
> > +The supported
> > +.I op
> > +values:
> > +.TP
> > +.B EPIOCSPARAMS
> > +This operation allows the caller to specify an
> > +.I epoll_params
> > +structure to configure the operation of epoll. Refer to the structure
> > +description of the structure above to learn what configuration is
> > +supported.
> > +.TP
> > +.B EPIOCGPARAMS
> > +This operation allows the caller to retrieve the current
> > +.I epoll_params
> > +structure. This can be used to determine what the current settings are.
> > +.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 ENOIOCTLCMD
> 
> Is this a thing?
> 
> 	$ grep -rn ENOIOCTLCMD /usr/include/
> 	$ 
> 
> I suspect this is EINVAL in user space?  (Actually, you list the same
> exact error condition for EINVAL below.)

Good catch!

Yes, I looked at the kernel code, it looks like this
becomes ENOTTY before entering userland, but due to the way the code
was written (I'll double check again) I think I wrote this to return
EINVAL.

> > +The specified
> > +.I op
> > +is invalid.
> > +.TP
> > +.B EINVAL
> > +The
> > +.I fd
> > +specified is not an epoll file descriptor, or the
> > +.I op
> > +specified is invalid, or the
> > +.I __pad
> > +was not initialized to zero, or
> > +.I busy_poll_usecs
> > +exceeds
> > +.B INT_MAX ,
> > +or
> > +.I prefer_busy_poll
> > +is not 0 or 1.
> 
> Please separate the different conditions for EINVAL into separate
> entries.  The ones that are related can go in the same one, but the
> unrelated ones are better split.

Thanks!

I did something like this:

.TP
.B EINVAL
The
.I fd
specified is not an epoll file descriptor.
.TP
.B EINVAL
The
.I op
specified is invalid.
.TP
.B EINVAL
The
.I argp.__pad
was not initialized to zero.

is that what you meant?

> > +.TP
> > +.B EPERM
> > +The process is being run without
> > +.I CAP_NET_ADMIN
> 
> This should be bold.

Thanks, fixed.

> > +and the specified
> > +.I busy_poll_budget
> 
> This should be
> 
> .I argp.busy_poll_budget

Thanks, fixed.

> > +exceeds
> > +.I NAPI_POLL_WEIGHT
> 
> This should be bold.

Thanks, fixed.

> > +(which is 64 as of Linux 6.9).
> > +.TP
> > +.B EFAULT
> > +.I argp
> > +does not point to a valid memory address.
> > +.SH EXAMPLES
> > +.EX
> 
> Could you write an entire program, with a main()?
> 
> If not, it's fine; this is better than nothing.  But we prefer having
> entire programs that users can paste and try.

Hmm.. I can give it a try! It's a bit tricky because to actually use
busy polling, you need to have a program that accepts incoming
connections and calls epoll_wait (after setting the appropriate
values with the ioctl).

I could write something simple that does that but it would be a bit
long, I think.

> > +/* 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 Documentation/networking/napi.rst
> > +.P
> > +and
> > +.P
> > +.I Documentation/admin-guide/sysctl/net.rst
> > +.P
> > +in the Linux kernel source tree
> 
> We could document that as
> 
> .I linux.git/Documentation/...
> 
> to not have to say "in the Linux kernel source tree".

OK, I updated the above to be what you suggested.

> > 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
> > 
> 
> -- 
> <https://www.alejandro-colomar.es/>

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

[Alejandro Colomar]

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

Hi!

On Thu, Jun 06, 2024 at 07:06:05PM GMT, Joe Damato wrote:
> On Thu, Jun 06, 2024 at 11:39:58PM +0200, Alejandro Colomar wrote:
> > Hi Joe,
> > 
> > On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > 
> > Thanks for the patch!  Please see a few comments below.
> 
> I've included some questions below just to make sure I give you a v2
> that is much closer to correct :)

Nice :)

> > > +.TH ioctl_epoll 2 (date) "Linux man-pages (unreleased)"
> > > +.SH NAME
> > > +ioctl_epoll \- ioctl() operations for epoll file descriptors
> > 
> > Please add link pages <man2const/EPIOCSPARAMS.2const> and
> > <man2const/EPIOCGPARAMS.2const>.
> 
> And after adding those, I'd add those to SEE ALSO and I'd omit the
> description of them from the ioctl_epoll.2 page (because they'd have
> their own pages) ?

Nope.  The manual page would still be <ioctl_epoll.2>.  The other two
would be link pages, that is, if you `man EPIOCSPARAMS`, you'd get the
manual page ioctl_epoll(2).

See for example the manual page slist(3):

	$ head man/man3/slist.3
	.\" Copyright (c) 1993
	.\"    The Regents of the University of California.  All rights reserved.
	.\" and Copyright (c) 2020 by Alejandro Colomar <alx@kernel.org>
	.\"
	.\" SPDX-License-Identifier: BSD-3-Clause
	.\"
	.\"
	.TH SLIST 3 (date) "Linux man-pages (unreleased)"
	.SH NAME
	SLIST_EMPTY,

It has several aliases, one for each macro that it documents.  Here are
a few examples:

	$ head man/man3/SLIST_EMPTY.3
	.so man3/slist.3
	$ head man/man3/SLIST_FOREACH.3 
	.so man3/slist.3

That .so roff(7) request is similar to a C #include, so that they're
actually the same page.  You can check in your terminal, with
`man slist` and `man SLIST_FOREACH`, which will give you the same page.

So, basically, I want you to write these:

	$ cat man/man2const/EPIOCSPARAMS.2const
	.so man2/ioctl_epoll.2
	$ cat man/man2const/EPIOCGPARAMS.2const
	.so man2/ioctl_epoll.2

Nothing else is required.

> > > +In the above,
> > > +.I fd
> > > +is a file descriptor referring to an epoll file descriptor obtained with a
> > > +call to
> > > +.BR epoll_create (2).
> > > +.I op
> > > +is one of the operations listed below, and
> > > +.I argp
> > > +is a pointer to the data structure described below.
> > 
> > If argp is a pointer to a structure, then the prototype should document
> > that:
> > 
> > .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );
> > 
> > Also, I would document the two operations separately:
> > 
> > .BI "int ioctl(int " fd ", EPIOCSPARAMS, const struct epoll_params *" argp );
> > .BI "int ioctl(int " fd ", EPIOCGPARAMS, struct epoll_params *" argp );
> > 
> > This allows documenting that the 'S' one doesn't modify the argp
> > (AFAICS).
> 
> Do you mean that I should omit the generic 
> 
>  .BI "int ioctl(int " fd ", int " op ", struct epoll_params *" argp );
> 
> and instead have the above with the two ops?

Exactly.

As in for example, PR_SET_MM_START_DATA.2const.  You can have a look at
that page in the repository, because it hasn't yet been released.

I'll paste here a copy of it, since it's short:

$ MANWIDTH=72 man PR_SET_MM_START_DATA | cat
PR_SET_MM_START_DATA(2const)               PR_SET_MM_START_DATA(2const)

NAME
     PR_SET_MM_START_DATA,  PR_SET_MM_END_DATA  -  modify kernel memory
     map descriptor fields

LIBRARY
     Standard C library (libc, -lc)

SYNOPSIS
     #include <linux/prctl.h>  /* Definition of PR_* constants */
     #include <sys/prctl.h>

     int prctl(PR_SET_MM, PR_SET_MM_START_DATA, unsigned long addr, 0L, 0L);
     int prctl(PR_SET_MM, PR_SET_MM_END_DATA, unsigned long addr, 0L, 0L);

DESCRIPTION
     PR_SET_MM_START_DATA
            Set the address above which initialized  and  uninitialized
            (bss)  data are placed.  The corresponding memory area must
            be readable and writable, but not executable or shareable.

     PR_SET_MM_END_DATA
            Set the address below which initialized  and  uninitialized
            (bss)  data are placed.  The corresponding memory area must
            be readable and writable, but not executable or shareable.

RETURN VALUE
     On success, 0 is returned.  On error, -1 is returned, and errno is
     set to indicate the error.

ERRORS
     EINVAL
            addr is greater than TASK_SIZE (the limit on  the  size  of
            the user address space for this architecture).

     EINVAL
            The permissions of the corresponding memory area are not as
            required.

STANDARDS
     Linux.

HISTORY
     Linux 3.3.

SEE ALSO
     prctl(2)

Linux man‐pages 6.8‐151‐g58... 2024‐06‐01  PR_SET_MM_START_DATA(2const)


BTW, now I realize that page is missing a reference to PR_SET_MM(2const)
in the SEE ALSO section.  I'll fix that in a moment.

> > > +.\"
> > > +.P
> > > +All supported
> > > +.I op
> > > +values (described below) use an
> > > +.I argp
> > > +argument which is a pointer to a
> > > +.I epoll_params
> > > +structure, defined as:
> > > +.P
> > > +.in +4n
> > > +.EX
> > > +struct epoll_params {
> > > +    uint32_t busy_poll_usecs;   /* Number of usecs to busy poll */
> > > +    uint16_t busy_poll_budget;  /* Maximum number of packets to retrieve per poll */
> > > +    uint8_t prefer_busy_poll;   /* Boolean to enable or disable prefer busy poll  */
> > > +
> > > +    /* pad the struct to a multiple of 64bits */
> > > +    uint8_t __pad;              /* Must be zero */
> > > +};
> > 
> > You could add this type definition to the SYNOPSIS, below the function
> > prototypes.
> 
> OK, I moved it.
> 
> I'm not sure if it is formatted properly, though. I'll see if I can
> find other examples of this style to check against.

See timespec(3type) or sockaddr(3type) for examples of pages documenting
structures in the SYNOPSIS.

> > > +The specified
> > > +.I op
> > > +is invalid.
> > > +.TP
> > > +.B EINVAL
> > > +The
> > > +.I fd
> > > +specified is not an epoll file descriptor, or the
> > > +.I op
> > > +specified is invalid, or the
> > > +.I __pad
> > > +was not initialized to zero, or
> > > +.I busy_poll_usecs
> > > +exceeds
> > > +.B INT_MAX ,
> > > +or
> > > +.I prefer_busy_poll
> > > +is not 0 or 1.
> > 
> > Please separate the different conditions for EINVAL into separate
> > entries.  The ones that are related can go in the same one, but the
> > unrelated ones are better split.
> 
> Thanks!
> 
> I did something like this:
> 
> .TP
> .B EINVAL
> The
> .I fd
> specified is not an epoll file descriptor.

Please use the same exact wording as in ioctl(2), for consistency.

> .TP
> .B EINVAL
> The
> .I op
> specified is invalid.

I would remove this one, since we're documenting that the calls be done
with specific operations.  There's no 'op' variable.  The variable 'op'
error is already documented in ioctl(2).

> .TP
> .B EINVAL
> The

I would remove the above 'The'.

> .I argp.__pad
> was not initialized to zero.

Maybe remove 'initialized to', to abbreviate.  If it's not zero, it's
of course because it wasn't initialized to zero.  :)

> is that what you meant?

Yup.

> > > +(which is 64 as of Linux 6.9).
> > > +.TP
> > > +.B EFAULT
> > > +.I argp
> > > +does not point to a valid memory address.
> > > +.SH EXAMPLES
> > > +.EX
> > 
> > Could you write an entire program, with a main()?
> > 
> > If not, it's fine; this is better than nothing.  But we prefer having
> > entire programs that users can paste and try.
> 
> Hmm.. I can give it a try! It's a bit tricky because to actually use
> busy polling, you need to have a program that accepts incoming
> connections and calls epoll_wait (after setting the appropriate
> values with the ioctl).
> 
> I could write something simple that does that but it would be a bit
> long, I think.

Hmmm, please do write it, but if it's so big, please do it in a second
patch, so that we're always in time to revert that one if it's too much.
Keep the current example in the patch that adds the page.

Thank you,
and have a lovely day!
Alex

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

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

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

[Joe Damato]

On Fri, Jun 07, 2024 at 12:29:17PM +0200, Alejandro Colomar wrote:
> Hi!
> 
> On Thu, Jun 06, 2024 at 07:06:05PM GMT, Joe Damato wrote:
> > On Thu, Jun 06, 2024 at 11:39:58PM +0200, Alejandro Colomar wrote:
> > > Hi Joe,
> > > 
> > > On Tue, Jun 04, 2024 at 06:17:40PM GMT, Joe Damato wrote:
> > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > 
> > > Thanks for the patch!  Please see a few comments below.
> > 
> > I've included some questions below just to make sure I give you a v2
> > that is much closer to correct :)
> 
> Nice :)

[ ... ]

I think I've incorporated all of your feedback and given it a good
rewrite.

Once I sort out the linting issue in the other thread (and fix any
issues that finds), I should be ready to send a v2 that I hope will
be much closer to correct.

Thanks for your detailed and thoughtful review,
Joe

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

[Alejandro Colomar]

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

On Tue, Jun 04, 2024 at 06:17:39PM GMT, Joe Damato wrote:
> Greetings:

Greetings!

> This is my first contribution to the man-pages project, so please excuse
> any obvious issues; I am happy to take feedback and send updated patches as
> needed.

Sure; no problem.

> This change documents a new ioctl interface for epoll added to Linux kernel
> 6.9 [1] and glibc [2] for controlling busy poll on a per-epoll fd basis.

Thanks!

> I noted that other ioctls have ioctl_*.2 files, so I followed that
> pattern in this change.

Sure.  I used a different pattern with prctl(2)s, so let's use a mix.
Do the ioctl_*.2 tradition, but also add link pages with the names of
the actual operations (which will allow to search directly for the man
pages of the individual operations).

> Based on the current status of glibc, I would assume that this change will
> be part of glibc 2.40 (it is listed under 2.40 in the NEWS section), which
> may be released in a few months [3].

If you're certain that this will be part of glibc 2.40, I'm fine merging
it already.  We can always patch it if something changes.

> Given that, I am not sure if I should wait until glibc 2.40 has been
> released before sending this change to this project or not.
> 
> Please let me know.
> 
> Thanks,
> Joe
> 

Cheers,
Alex

> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/fs/eventpoll.c?h=v6.9&id=18e2bf0edf4dd88d9656ec92395aa47392e85b61
> [2]: https://sourceware.org/git/?p=glibc.git;a=commit;h=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> [3]: https://sourceware.org/glibc/wiki/Glibc%20Timeline
> 
> Joe Damato (1):
>   ioctl_epoll.2: New page describing ioctl(2) operations for epoll fds
> 
>  man/man2/epoll_create.2 |   1 +
>  man/man2/epoll_ctl.2    |   1 +
>  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
>  man/man7/epoll.7        |   1 +
>  4 files changed, 206 insertions(+)
>  create mode 100644 man/man2/ioctl_epoll.2
> 
> -- 
> 2.34.1
> 
> 

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

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

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

[Joe Damato]

On Thu, Jun 06, 2024 at 11:44:10PM +0200, Alejandro Colomar wrote:
> On Tue, Jun 04, 2024 at 06:17:39PM GMT, Joe Damato wrote:
> > Greetings:
> 
> Greetings!
> 
> > This is my first contribution to the man-pages project, so please excuse
> > any obvious issues; I am happy to take feedback and send updated patches as
> > needed.
> 
> Sure; no problem.
> 
> > This change documents a new ioctl interface for epoll added to Linux kernel
> > 6.9 [1] and glibc [2] for controlling busy poll on a per-epoll fd basis.
> 
> Thanks!
> 
> > I noted that other ioctls have ioctl_*.2 files, so I followed that
> > pattern in this change.
> 
> Sure.  I used a different pattern with prctl(2)s, so let's use a mix.
> Do the ioctl_*.2 tradition, but also add link pages with the names of
> the actual operations (which will allow to search directly for the man
> pages of the individual operations).

Thanks for your careful review. I will make the changes you
suggested for the v2.

> > Based on the current status of glibc, I would assume that this change will
> > be part of glibc 2.40 (it is listed under 2.40 in the NEWS section), which
> > may be released in a few months [3].
> 
> If you're certain that this will be part of glibc 2.40, I'm fine merging
> it already.  We can always patch it if something changes.

I have no idea if I can be certain of that. I am not a maintainer
nor do I have commit access to glibc, so I truly have no idea.

I suppose it is possible that they may decide to cut glibc 2.40 from
before my patch went in? It does not seem there is any 2.40 tag yet,
AFAICT.

How about I proceed by making all the changes you've requested and
get the patch into a shape where it can be merged. Hopefully,
that'll only take one (or two ;) more revisions.

Once the patch is in good shape, then we can decide whether to merge
or wait for glibc 2.40? If they are releasing it in 8/2024, it's not
too long to wait.

Does that seem OK to you?

> > Given that, I am not sure if I should wait until glibc 2.40 has been
> > released before sending this change to this project or not.
> > 
> > Please let me know.
> > 
> > Thanks,
> > Joe
> > 
> 
> Cheers,
> Alex
> 
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/fs/eventpoll.c?h=v6.9&id=18e2bf0edf4dd88d9656ec92395aa47392e85b61
> > [2]: https://sourceware.org/git/?p=glibc.git;a=commit;h=92c270d32caf3f8d5a02b8e46c7ec5d9d0315158
> > [3]: https://sourceware.org/glibc/wiki/Glibc%20Timeline
> > 
> > Joe Damato (1):
> >   ioctl_epoll.2: New page describing ioctl(2) operations for epoll fds
> > 
> >  man/man2/epoll_create.2 |   1 +
> >  man/man2/epoll_ctl.2    |   1 +
> >  man/man2/ioctl_epoll.2  | 203 ++++++++++++++++++++++++++++++++++++++++
> >  man/man7/epoll.7        |   1 +
> >  4 files changed, 206 insertions(+)
> >  create mode 100644 man/man2/ioctl_epoll.2
> > 
> > -- 
> > 2.34.1
> > 
> > 
> 
> -- 
> <https://www.alejandro-colomar.es/>

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

[Alejandro Colomar]

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

Hi!

On Thu, Jun 06, 2024 at 03:14:33PM GMT, Joe Damato wrote:
> On Thu, Jun 06, 2024 at 11:44:10PM +0200, Alejandro Colomar wrote:
> > Sure.  I used a different pattern with prctl(2)s, so let's use a mix.
> > Do the ioctl_*.2 tradition, but also add link pages with the names of
> > the actual operations (which will allow to search directly for the man
> > pages of the individual operations).
> 
> Thanks for your careful review. I will make the changes you
> suggested for the v2.

Thank you.

> 
> > > Based on the current status of glibc, I would assume that this change will
> > > be part of glibc 2.40 (it is listed under 2.40 in the NEWS section), which
> > > may be released in a few months [3].
> > 
> > If you're certain that this will be part of glibc 2.40, I'm fine merging
> > it already.  We can always patch it if something changes.
> 
> I have no idea if I can be certain of that. I am not a maintainer
> nor do I have commit access to glibc, so I truly have no idea.
> 
> I suppose it is possible that they may decide to cut glibc 2.40 from
> before my patch went in? It does not seem there is any 2.40 tag yet,
> AFAICT.
> 
> How about I proceed by making all the changes you've requested and
> get the patch into a shape where it can be merged. Hopefully,
> that'll only take one (or two ;) more revisions.
> 
> Once the patch is in good shape, then we can decide whether to merge
> or wait for glibc 2.40? If they are releasing it in 8/2024, it's not
> too long to wait.
> 
> Does that seem OK to you?

Yep, that's fine.  If you know who merged your patch, maybe you can CC
him/her?

Have a lovely night!
Alex

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

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

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

[Joe Damato]

On Fri, Jun 07, 2024 at 12:25:05AM +0200, Alejandro Colomar wrote:
> Hi!
> 
> On Thu, Jun 06, 2024 at 03:14:33PM GMT, Joe Damato wrote:
> > On Thu, Jun 06, 2024 at 11:44:10PM +0200, Alejandro Colomar wrote:
> > > Sure.  I used a different pattern with prctl(2)s, so let's use a mix.
> > > Do the ioctl_*.2 tradition, but also add link pages with the names of
> > > the actual operations (which will allow to search directly for the man
> > > pages of the individual operations).
> > 
> > Thanks for your careful review. I will make the changes you
> > suggested for the v2.
> 
> Thank you.
> 
> > 
> > > > Based on the current status of glibc, I would assume that this change will
> > > > be part of glibc 2.40 (it is listed under 2.40 in the NEWS section), which
> > > > may be released in a few months [3].
> > > 
> > > If you're certain that this will be part of glibc 2.40, I'm fine merging
> > > it already.  We can always patch it if something changes.
> > 
> > I have no idea if I can be certain of that. I am not a maintainer
> > nor do I have commit access to glibc, so I truly have no idea.
> > 
> > I suppose it is possible that they may decide to cut glibc 2.40 from
> > before my patch went in? It does not seem there is any 2.40 tag yet,
> > AFAICT.
> > 
> > How about I proceed by making all the changes you've requested and
> > get the patch into a shape where it can be merged. Hopefully,
> > that'll only take one (or two ;) more revisions.
> > 
> > Once the patch is in good shape, then we can decide whether to merge
> > or wait for glibc 2.40? If they are releasing it in 8/2024, it's not
> > too long to wait.
> > 
> > Does that seem OK to you?
> 
> Yep, that's fine.  If you know who merged your patch, maybe you can CC
> him/her?

I had an existing email thread with them on another topic; they
mentioned that my change should be included in 2.40 as there has
been no announcement of a release freeze yet.

As I mentioned above, if you'd like to be cautious, I'm happy to
just get the patch into a pristine state for merging and then
waiting to merge until glibc 2.40 is shipped.

Totally up to you!

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

[Alejandro Colomar]

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

On Thu, Jun 06, 2024 at 07:08:06PM GMT, Joe Damato wrote:
> > Yep, that's fine.  If you know who merged your patch, maybe you can CC
> > him/her?
> 
> I had an existing email thread with them on another topic; they
> mentioned that my change should be included in 2.40 as there has
> been no announcement of a release freeze yet.
> 
> As I mentioned above, if you'd like to be cautious, I'm happy to
> just get the patch into a pristine state for merging and then
> waiting to merge until glibc 2.40 is shipped.

Nah; we're always in time to fix it if they change their minds.

> 
> Totally up to you!

Cheers,
Alex

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

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