Re: [PATCH man-pages v3] madvise.2: add MADV_GUARD_INSTALL, MADV_GUARD_REMOVE description

[Alejandro Colomar <alx@kernel.org>]

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

Hi Lorenzo,

On Mon, Dec 02, 2024 at 04:58:29PM +0000, Lorenzo Stoakes wrote:
> Lightweight guard region support has been added to Linux 6.13, which adds
> MADV_GUARD_INSTALL and MADV_GUARD_REMOVE flags to the madvise() system
> call. Therefore, update the manpage for madvise() and describe these
> operations.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Thanks for the patch!  A few minor comments below.

> ---
> v3:
> * Don't describe SIGSEGV as a fatal signal as per Jann.
> 
> v2:
> * Updated to use semantic newlines as suggested by Alejandro.
> * Avoided emboldening parens as suggested by Alejandro.
> * One very minor grammatical fix.
> https://lore.kernel.org/all/20241129155943.85215-1-lorenzo.stoakes@oracle.com
> 
> v1:
> https://lore.kernel.org/all/20241129093205.8664-1-lorenzo.stoakes@oracle.com
> 
>  man/man2/madvise.2 | 93 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/man/man2/madvise.2 b/man/man2/madvise.2
> index 4f2210ee2..4cb5e7302 100644
> --- a/man/man2/madvise.2
> +++ b/man/man2/madvise.2
> @@ -676,6 +676,91 @@ or secret memory regions created using
>  Note that with
>  .BR MADV_POPULATE_WRITE ,
>  the process can be killed at any moment when the system runs out of memory.
> +.TP
> +.BR MADV_GUARD_INSTALL " (since Linux 6.13)"
> +Install a lightweight guard region into the range specified by
> +.I addr
> +and
> +.IR size ,
> +causing any read or write in the range to result in a
> +.B SIGSEGV
> +signal being raised.
> +.IP
> +If the region maps memory pages they will be cleared as part of the operation,
> +though if
> +.B MADV_GUARD_INSTALL
> +is applied to regions containing pre-existing lightweight guard regions,
> +they are left in place.
> +.IP
> +This operation is only supported for writable anonymous private mappings which
> +have not been mlock'd.
> +An
> +.B EINVAL
> +error is returned if it is attempted on any other kind of mapping.
> +.IP
> +This operation is more efficient than mapping a new region of memory
> +.BR PROT_NONE ,
> +as it does not require the establishment of new mappings,
> +instead regions of an existing mapping simply have their page tables
> +manipulated to establish the desired behavior.
> +No additional memory is used.
> +.IP
> +Lightweight guard regions remain on fork
> +(except for any parts which have had
> +.B MADV_WIPEONFORK
> +applied to them),
> +and are not removed by
> +.BR MADV_DONTNEED ,
> +.BR MADV_FREE ,
> +.BR MADV_PAGEOUT ,
> +or
> +.BR MADV_COLD .
> +.IP
> +Attempting to
> +.BR mlock ()

We should specify the chapter in the manual: (2)

> +lightweight guard regions will fail,
> +as will
> +.B MADV_POPULATE_READ
> +or
> +.BR MADV_POPULATE_WRITE .
> +.IP
> +If the mapping has its attributes changed,
> +or is split or partially unmapped,
> +any existing guard regions remain in place
> +(except if they are unmapped).
> +.IP
> +If a mapping is moved using
> +.BR mremap (),

Same here.

> +lightweight guard regions are moved with it.
> +.IP
> +Lightweight guard regions are removed when unmapped,
> +on process teardown,
> +or when the
> +.B MADV_GUARD_REMOVE
> +operation is applied to them.
> +.TP
> +.BR MADV_GUARD_REMOVE " (since Linux 6.13)"
> +Remove any lightweight guard regions which exist in the range specified by
> +.I addr
> +and
> +.IR size .
> +.IP
> +All mappings in the range other than lightweight guard regions are left in place
> +(including mlock'd mappings).
> +The operation is,
> +however,
> +only valid for writable anonymous private mappings,

Wouldn't it be better to say "valid only" instead of "only valid"?

(Non-native speaker here, though.)

Have a lovely night!
Alex

> +returning an
> +.B EINVAL
> +error otherwise.
> +.IP
> +When lightweight guard regions are removed,
> +they act as empty regions of the containing mapping.
> +Since only writable anonymous private mappings are supported,
> +they therefore become zero-fill-on-demand pages.
> +.IP
> +If any transparent huge pages are encountered in the operation,
> +they are left in place.
>  .SH RETURN VALUE
>  On success,
>  .BR madvise ()
> @@ -787,6 +872,14 @@ or
>  or secret memory regions created using
>  .BR memfd_secret(2) .
>  .TP
> +.B EINVAL
> +.I advice
> +is
> +.B MADV_GUARD_INSTALL
> +or
> +.BR MADV_GUARD_REMOVE ,
> +but the specified address range contains an unsupported mapping.
> +.TP
>  .B EIO
>  (for
>  .BR MADV_WILLNEED )
> --
> 2.47.1

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

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