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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Joe,

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

:-}

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

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

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

You may want to also read this commit:

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

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

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

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

	s/retrieve on/retrieve\non/

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

and also break after the comma.

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

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

TL;DR:

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

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

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

Have a lovely day!
Alex

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

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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux