Re: [PATCH v2] delete_module.2: Update man to current syscall behaviour

[Alejandro Colomar <alx@kernel.org>]

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

Hi Alexander,

On Wed, Feb 21, 2024 at 12:48:52PM +0300, Alexander Ofitserov wrote:
> Parameter O_NONBLOCK described in man doesn't exist anymore
> in kernel versions 3.13+ (particularly in commit
> 3f2b9c9cdf389e303b2273679af08aab5f153517 aka v3.13-rc1~83^2~5),
> which is quite old, only O_TRUNC parameter present for current kernel version,
> O_NONBLOCK does nothing.
> 
> v1 -> v2:

Please add to the commit message tags to CC those who participated in
the discussion of the patch, including previous revisions.

Cc: "Dmitry V. Levin" <ldv@altlinux.org>
Cc: "G. Branden Robinson" <branden@debian.org>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: dutyrok@altlinux.org
Cc: kovalev@altlinux.org

(You may or may not include me, since I'll sign the patch anyway.)

> added behaviour of syscall for kernel 3.12 and earlier
> in history section
> added commit hash to commit message
> changed word 'actual' to 'current' due to ambigious
> meaning
> ---
>  man2/delete_module.2 | 95 +++++++++++++++++++++++++-------------------
>  1 file changed, 55 insertions(+), 40 deletions(-)
> 
> diff --git a/man2/delete_module.2 b/man2/delete_module.2
> index e9c432e84..d6e7a4bdb 100644
> --- a/man2/delete_module.2
> +++ b/man2/delete_module.2
> @@ -50,42 +50,20 @@ is zero, then the module is immediately unloaded.
>  If a module has a nonzero reference count,
>  then the behavior depends on the bits set in
>  .IR flags .
> -In normal usage (see NOTES), the
> -.B O_NONBLOCK
> -flag is always specified, and the
> +The
>  .B O_TRUNC
>  flag may additionally be specified.
>  .\"  	O_TRUNC == KMOD_REMOVE_FORCE in kmod library
> -.\"  	O_NONBLOCK == KMOD_REMOVE_NOWAIT in kmod library
>  .IP
>  The various combinations for
>  .I flags
>  have the following effect:
>  .RS
>  .TP
> -.B flags == O_NONBLOCK
> -The call returns immediately, with an error.
> -.TP
> -.B flags == (O_NONBLOCK | O_TRUNC)
> +.B flags == O_TRUNC
>  The module is unloaded immediately,
>  regardless of whether it has a nonzero reference count.
> -.TP
> -.B (flags & O_NONBLOCK) == 0
> -If
> -.I flags
> -does not specify
> -.BR O_NONBLOCK ,
> -the following steps occur:
>  .RS
> -.IP \[bu] 3
> -The module is marked so that no new references are permitted.
> -.IP \[bu]
> -If the module's reference count is nonzero,
> -the caller is placed in an uninterruptible sleep state
> -.RB ( TASK_UNINTERRUPTIBLE )
> -until the reference count is zero, at which point the call unblocks.
> -.IP \[bu]
> -The module is unloaded in the usual way.
>  .RE
>  .RE
>  .P
> @@ -151,11 +129,7 @@ in
>  .TP
>  .B EWOULDBLOCK
>  Other modules depend on this module;
> -or,
> -.B O_NONBLOCK
> -was specified in
> -.IR flags ,
> -but the reference count of this module is nonzero and
> +or, the reference count of this module is nonzero and
>  .B O_TRUNC
>  was not specified in
>  .IR flags .
> @@ -172,6 +146,57 @@ it is (before glibc 2.23) sufficient to
>  manually declare the interface in your code;
>  alternatively, you can invoke the system call using
>  .BR syscall (2).
> +.SS Linux 3.12 and earlier

I think it would be better to name the subsection O_NONBLOCK.

.SS O_NONBLOCK

> +In Linux 3.12 and earlier, the system call took two arguments:
> +.P
> +.BI "   int delete_module(const char *" name ", unsigned int " flags );

Since the current syscall still has two arguments (right?), I'd remove
the above about the arguments.

> +.P
> +Parameter
> +.I flags
> +also can contain 
> +.B O_NONBLOCK
> +flag in addition to 
> +.B O_TRUNC
> +flag. Behavior depends on the bits set in

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

$ MANWIDTH=72 man 7 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  sen‐
     tences, clauses, or phrases.

We always break the line after a period.  And if we didn't groff(1)
requires two spaces (the right amount, BTW :) after a period.

Have a lovely day!
Alex

> +.IR flags .
> +In normal usage (see NOTES), the
> +.B O_NONBLOCK
> +flag is always specified, and the
> +.B O_TRUNC
> +flag may additionally be specified.
> +.\"  	O_TRUNC == KMOD_REMOVE_FORCE in kmod library
> +.\"  	O_NONBLOCK == KMOD_REMOVE_NOWAIT in kmod library
> +.IP
> +The various combinations for
> +.I flags
> +have the following effect:
> +.RS
> +.TP
> +.B flags == O_NONBLOCK
> +The call returns immediately, with an error.
> +.TP
> +.B flags == (O_NONBLOCK | O_TRUNC)
> +The module is unloaded immediately,
> +regardless of whether it has a nonzero reference count.
> +.TP
> +.B (flags & O_NONBLOCK) == 0
> +If
> +.I flags
> +does not specify
> +.BR O_NONBLOCK ,
> +the following steps occur:
> +.RS
> +.IP \[bu] 3
> +The module is marked so that no new references are permitted.
> +.IP \[bu]
> +If the module's reference count is nonzero,
> +the caller is placed in an uninterruptible sleep state
> +.RB ( TASK_UNINTERRUPTIBLE )
> +until the reference count is zero, at which point the call unblocks.
> +.IP \[bu]
> +The module is unloaded in the usual way.
> +.RE
> +.RE
>  .SS Linux 2.4 and earlier
>  In Linux 2.4 and earlier, the system call took only one argument:
>  .P
> @@ -183,19 +208,9 @@ is NULL, all unused modules marked auto-clean are removed.
>  .P
>  Some further details of differences in the behavior of
>  .BR delete_module ()
> -in Linux 2.4 and earlier are
> +in Linux 3.12 and earlier are
>  .I not
>  currently explained in this manual page.
> -.SH NOTES
> -The uninterruptible sleep that may occur if
> -.B O_NONBLOCK
> -is omitted from
> -.I flags
> -is considered undesirable, because the sleeping process is left
> -in an unkillable state.
> -As at Linux 3.7, specifying
> -.B O_NONBLOCK
> -is optional, but in future kernels it is likely to become mandatory.
>  .SH SEE ALSO
>  .BR create_module (2),
>  .BR init_module (2),
> -- 
> 2.33.8
> 

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

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