public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx@kernel.org>
To: Jeff Layton <jlayton@kernel.org>
Cc: linux-man@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH man-pages] man/man2const: document the new F_SETDELEG and F_GETDELEG constants
Date: Tue, 13 Jan 2026 16:12:04 +0100	[thread overview]
Message-ID: <aWZecUT7o_cYH5Rt@devuan> (raw)
In-Reply-To: <14e88ee6ff3ffd671f579d103c53ebe98b4f92e2.camel@kernel.org>

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

Hi Jeff,

On Tue, Jan 13, 2026 at 09:45:11AM -0500, Jeff Layton wrote:
[...]
> > > +.SH SYNOPSIS
> > > +.nf
> > > +.B #define _GNU_SOURCE
> > > +.B #include <fcntl.h>
> > > +.P
> > > +.EX
> > > +/* Argument structure for F_GETDELEG and F_SETDELEG */
> > 
> > I'd say this comment seems redundant, given this structure is defined
> > in this manual page.
> > 
> 
> Ack. Will remove.
> 
> > > +struct delegation {
> > > +        __u32   d_flags;
> > > +        __u16   d_type;
> > > +        __u16   __pad;
> > > +};
> > > +.EE
> > > +.P
> > > +.BI "int fcntl(int " fd ", F_SETDELEG, struct delegation *" deleg );
> > 
> > Is this really not const-qualified?  Does the kernel modify it?
> > 
> 
> Yes, it does modify it. F_GETDELEG populates d_type. I have some plans
> to expand this interface in the future so F_GETDELEG may eventually
> return other fields in there too.

But I mean F_SETDELEG (with an S).  I expect the setter wouldn't modify,
right?

> 
> > > +.br
> > 
> > This .br seems superfluous.
> > 
> 
> You would think, no? But when I remove it, man seems to stick both
> lines togther. I really do not grok groff at all.
> 
> I'm happy to accept other suggestions on how to fix that though.

Ahhh, sorry!  I see why.  So, we have EX/EE nested within nf/fi.  These
don't nest correctly, so one should close nf/fi, then do EX/EE, and then
open a new nf/fi pair.

> > > +.BI "int fcntl(int " fd ", F_GETDELEG, struct delegation *" deleg );
> > > +.fi
> > > +.SH DESCRIPTION
> > > +.SS Delegations
> > 
> > If the entire section is within a subsection, I think the subsection is
> > redundant, isn't it?
> > 
> 
> Oh ok, I cargo-cult copied most of this from F_GETLEASE.man2const, so
> this may be there as well.

I was supposing you had done that, but didn't figure out from which.  :)
Now it makes sense.  Yes, I should remove that from
F_GETLEASE.man2const.  It is a left-over from when I split the page
from fcntl(2).

> > > +.B F_SETDELEG
> > > +and
> > > +.B F_GETDELEG
> > > +are used to establish a new delegation,
> > > +and retrieve the current delegation, on the open file description
> > > +referred to by the file descriptor
> > 
> > Just to confirm: one can't retrieve delegations through a different file
> > description that refers to the same inode, right?
> > 
> 
> Correct. F_GETDELEG just fills out ->d_type with the type of delegation
> set on "fd".

Thanks!

> > > +.IR fd .
> > 
> > I'd separate the paragraph here.  The above serves as a brief
> > introduction of these two APIs, while the following text describes what
> > delegations are.
> > 
> 
> Ok.
> 
> > > +A file delegation is a mechanism whereby the process holding
> > > +the delegation (the "delegation holder") is notified (via delivery of a signal)
> > > +when a process (the "delegation breaker") tries to
> > > +.BR open (2)
> > > +or
> > > +.BR truncate (2)
> > > +the file referred to by that file descriptor, or attempts to
> > > +.BR unlink (2)
> > > +or
> > > +.BR rename (2)
> > > +the dentry that was originally opened for the file.
> > > +.BR F_RDLCK
> > > +delegations can also be set on directory file descriptors, in which case they will
> > > +be recalled if there is a create, delete or rename of a dirent within the directory.
> > 
> > Please use semantic newlines.  See man-pages(7):
> > 
> > $ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p'
> >    Use semantic newlines
> >      In the source of a manual page, new sentences should be started on
> >      new lines, long sentences should be split  into  lines  at  clause
> >      breaks  (commas,  semicolons, colons, and so on), and long clauses
> >      should be split at phrase boundaries.  This convention,  sometimes
> >      known as "semantic newlines", makes it easier to see the effect of
> >      patches, which often operate at the level of individual sentences,
> >      clauses, or phrases.
> > 
> 
> Ok, I'll reformat that.

This applies also to the rest of the page, BTW.

> > > +.TP
> > > +.B F_SETDELEG
> > > +Set or remove a file or directory delegation according to which of the following
> > > +values is specified in the
> > > +.IR d_type
> > 
> > s/IR/I/
> > 
> > IR is for alternating Italics and Roman.
> > 
> > Also, it would be good to use '.d_type' notation, for making it easier
> > to distinguish struct members.  A few manual pages already do this.
> > Thus:
> > 
> > 	.I .d_type
> > 
> > > +field of
> > > +.IR deleg :
> > 
> > Maybe we could even simplify this as:
> > 
> > 	...
> > 	specified in
> > 	.IR deleg->d_type :
> > 
> 
> Ok
> 
> > > +.RS
> > > +.TP
> > > +.B F_RDLCK
> > > +Take out a read delegation.
> > 
> > 'Take out' means to remove, but by reading the context, I think this
> > instead creates a read delegation, right?  Would it be correct to say
> > this?:
> > 
> > 	Create a read delegation.
> > 
> > (I'm not a native English speaker, so I may be mistaken.)
> > 
> 
> Delegations are returnable, so "take out" in the sense of a library
> book or some other returnable object. I'll rephrase that, since it's
> unclear.
> 
> > > +This will cause the calling process to be notified when
> > > +the file is opened for writing or is truncated, or when the file is unlinked or renamed.
> > > +A read delegation can be placed only on a file descriptor that
> > > +is opened read-only. If
> > > +.IR fd
> > > +refers to a directory, then the calling process
> > > +will be notified if there are changes to filenames within the directory, or when the
> > > +directory itself is renamed.
> > > +.TP
> > > +.B F_WRLCK
> > 
> > Are we in time to rename this?
> > 
> > F_RDLCK blocks essentially writing.
> > F_WRLCK blocks both reading and writing.
> > 
> > What do you think of this rename:
> > 
> > 	RD => WR
> > 	WR => RW
> > 
> 
> These are constants from file locking. I suppose we could add new
> constants that overload those values?

Ahhh, so this is already a historical thing.  Indeed:

	$ grep -rn F_RDLCK man/
	man/man2const/F_GETLEASE.2const:45:.B F_RDLCK
	man/man2const/F_GETLEASE.2const:89:.BR F_RDLCK ", " F_WRLCK ", or " F_UNLCK ,
	man/man2const/F_GETLEASE.2const:123:.BR F_RDLCK .
	man/man2const/F_GETLEASE.2const:133:.B F_RDLCK
	man/man2/fcntl_locking.2:52:    short l_type;    /* Type of lock: F_RDLCK,
	man/man2/fcntl_locking.2:123:.RB ( F_RDLCK )
	man/man2/fcntl_locking.2:143:.B F_RDLCK
	man/man2/fcntl_locking.2:360:.B F_RDLCK

I guess that answers my question: no, we can't rename these.
Let's keep the historical names.

> > > +Take out a write delegation.
> > > +This will cause the caller to be notified when
> > > +the file is opened for reading or writing or is truncated or when the file is renamed
> > > +or unlinked.  A write delegation may be placed on a file only if there are no
> > > +other open file descriptors for the file. The file must be opened for write in order
> > > +to set a write delegation on it. Write delegations cannot be set on directory
> > > +file descriptors.
> > > +.TP
> > > +.B F_UNLCK
> > > +Remove our delegation from the file.
> > > +.RE
> > > +.P
> > > +Like leases, delegations are associated with an open file description (see
> > > +.BR open (2)).
> > > +This means that duplicate file descriptors (created by, for example,
> > > +.BR fork (2)
> > > +or
> > > +.BR dup (2))
> > > +refer to the same delegation, and this delegation may be modified
> > > +or released using any of these descriptors.
> > > +Furthermore, the delegation is released by either an explicit
> > > +.B F_UNLCK
> > > +operation on any of these duplicate file descriptors, or when all
> > > +such file descriptors have been closed.
> > > +.P
> > > +An unprivileged process may take out a delegation only on a file whose
> > > +UID (owner) matches the filesystem UID of the process.
> > > +A process with the
> > > +.B CAP_LEASE
> > > +capability may take out delegations on arbitrary files or directories.
> > > +.TP
> > > +.B F_GETDELEG
> > > +Indicates what type of delegation is associated with the file descriptor
> > > +.I fd
> > > +by setting the
> > > +.IR d_type
> > > +field in
> > > +.IR deleg
> > > +to either
> > > +.BR F_RDLCK ", " F_WRLCK ", or " F_UNLCK ,
> > 
> > Please use a separate line for each:
> > 
> > 	.BR F_RDLCK ,
> > 	.BR F_WRLCK ,
> > 	or
> > 	.BR F_UNLCK ,
> > 
> > > +indicating, respectively, a read delegation , a write delegation, or no delegation.
> >
> 
> ACK
> 
> > Spurious white space before comma.
> > 
> 
> ACK
> 
> > > +.P
> > > +When a process (the "delegation breaker") performs an activity
> > > +that conflicts with a delegation established via
> > > +.BR F_SETDELEG ,
> > > +the system call is blocked by the kernel and
> > > +the kernel notifies the delegation holder by sending it a signal
> > > +.RB ( SIGIO
> > > +by default).
> > > +The delegation holder should respond to receipt of this signal by doing
> > > +whatever cleanup is required in preparation for the file to be
> > > +accessed by another process (e.g., flushing cached buffers) and
> > > +then either remove or downgrade its delegation.
> > > +A delegation is removed by performing an
> > > +.B F_SETDELEG
> > > +operation specifying
> > > +.I d_type
> > > +in
> > > +.I deleg
> > > +as
> > > +.BR F_UNLCK .
> > > +If the delegation holder currently holds a write delegation on the file,
> > > +and the delegation breaker is opening the file for reading,
> > > +then it is sufficient for the delegation holder to downgrade
> > > +the delegation to a read delegation.
> > > +This is done by performing an
> > > +.B F_SETDELEG
> > > +operation specifying
> > > +.I d_type
> > > +in
> > > +.I deleg
> > > +as
> > > +.BR F_RDLCK .
> > > +.P
> > > +If the delegation holder fails to downgrade or remove the delegation within
> > > +the number of seconds specified in
> > > +.IR /proc/sys/fs/lease\-break\-time ,
> > > +then the kernel forcibly removes or downgrades the delegation holder's delegation.
> > > +.P
> > > +Once a delegation break has been initiated,
> > > +.B F_GETDELEG
> > > +returns the target delegation type in the
> > > +.I d_type
> > > +field in
> > > +.I deleg
> > > +(either
> > > +.B F_RDLCK
> > > +or
> > > +.BR F_UNLCK ,
> > > +depending on what would be compatible with the delegation breaker)
> > > +until the delegation holder voluntarily downgrades or removes the delegation or
> > > +the kernel forcibly does so after the delegation break timer expires.
> > > +.P
> > > +Once the delegation has been voluntarily or forcibly removed or downgraded,
> > > +and assuming the delegation breaker has not unblocked its system call,
> > > +the kernel permits the delegation breaker's system call to proceed.
> > > +.P
> > > +If the delegation breaker's blocked system call
> > > +is interrupted by a signal handler,
> > > +then the system call fails with the error
> > > +.BR EINTR ,
> > > +but the other steps still occur as described above.
> > > +If the delegation breaker is killed by a signal while blocked in
> > > +.BR open (2)
> > > +or
> > > +.BR truncate (2),
> > > +then the other steps still occur as described above.
> > > +If the delegation breaker specifies the
> > > +.B O_NONBLOCK
> > > +flag when calling
> > > +.BR open (2),
> > > +then the call immediately fails with the error
> > > +.BR EWOULDBLOCK ,
> > > +but the other steps still occur as described above.
> > > +.P
> > > +The default signal used to notify the delegation holder is
> > > +.BR SIGIO ,
> > > +but this can be changed using the
> > > +.B F_SETSIG
> > > +operation to
> > > +.BR fcntl ().
> > > +If a
> > > +.B F_SETSIG
> > > +operation is performed (even one specifying
> > > +.BR SIGIO ),
> > > +and the signal
> > > +handler is established using
> > > +.BR SA_SIGINFO ,
> > > +then the handler will receive a
> > > +.I siginfo_t
> > > +structure as its second argument, and the
> > > +.I si_fd
> > > +field of this argument will hold the file descriptor of the file with the delegation
> > > +that has been accessed by another process.
> > > +(This is useful if the caller holds delegations against multiple files.)
> > > +.SH NOTES
> > > +Delegations were designed to implement NFSv4 delegations for the Linux NFS server, and
> > > +conform to the delegation semantics described in RFC\ 8881.
> > 
> > I'd say the part after the comma is redundant with the STANDARDS
> > section.
> > 
> 
> Ok.
> 
> > > +.SH RETURN VALUE
> > > +On success zero is returned. On error, \-1 is returned, and
> > > +.I errno
> > > +is set to indicate the error. A successful F_GETDELEG call will also update the
> > 
> > F_GETDELEG should be in bold.
> > 
> 
> Ok.
> 
> > > +.I d_type
> > > +field in the structure to which
> > > +.I deleg
> > > +points.
> > > +.SH ERRORS
> > > +See
> > > +.BR fcntl (2).
> > 
> > I guess there are also errors specific to delegations, right?  I expect
> > for example an error if F_SETDELEG is called with F_WRLCK but more file
> > descriptors are open for the same file.
> > 
> 
> Only if the file descriptor was opened non-blocking. The errors are
> basically the same as the ones for leases. I can transplant the
> relevant error messages here though.

Hmm, I see the F_SETLEASE page doesn't document those either.

I think it would be good to document the errors, and in F_GETLEASE too,
if you can.  (Feel free to leave that for a later patch.)

> > > +.SH STANDARDS
> > > +Linux, IETF RFC\ 8881.
> > > +.SH HISTORY
> > > +Linux v6.19.
> > 
> > Please remove the 'v'.
> > 
> 
> ACK.
> 
> > > +.SH SEE ALSO
> > > +.BR fcntl (2)
> > > diff --git a/man/man2const/F_SETDELEG.2const b/man/man2const/F_SETDELEG.2const
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..acabdfc139fb3d753dbf3061c31d59332d046c63
> > > --- /dev/null
> > > +++ b/man/man2const/F_SETDELEG.2const
> > > @@ -0,0 +1 @@
> > > +.so man2const/F_GETDELEG.2const
> > 
> > Thanks!
> > 
> > 
> > Have a lovely day!
> > Alex
> 
> Thank you for the review! I'll update and send a v2.

Thanks!


Cheers,
Alex

> 
> Cheers,
> -- 
> Jeff Layton <jlayton@kernel.org>

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

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

      parent reply	other threads:[~2026-01-13 15:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-12 18:47 [PATCH man-pages] man/man2const: document the new F_SETDELEG and F_GETDELEG constants Jeff Layton
2026-01-13 14:13 ` Alejandro Colomar
2026-01-13 14:45   ` Jeff Layton
2026-01-13 15:05     ` G. Branden Robinson
2026-01-13 15:12     ` Alejandro Colomar [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWZecUT7o_cYH5Rt@devuan \
    --to=alx@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox