Re: [PATCH v2] madvise.2: Clarify addr/length and update hugetlb support

From: Alejandro Colomar <alx.manpages@gmail.com>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: David Hildenbrand <david@redhat.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Peter Xu <peterx@redhat.com>,
	linux-mm@kvack.org, linux-man@vger.kernel.org
Subject: Re: [PATCH v2] madvise.2: Clarify addr/length and update hugetlb support
Date: Mon, 6 Jun 2022 21:12:00 +0200	[thread overview]
Message-ID: <7db16a8d-f275-3c39-3fed-83a33886976c@gmail.com> (raw)
In-Reply-To: <20220526180950.13916-1-mike.kravetz@oracle.com>

[-- Attachment #1.1: Type: text/plain, Size: 5091 bytes --]

Hi Mike,

On 5/26/22 20:09, Mike Kravetz wrote:
> Clarify that madvise only works on full pages, and remove references
> to 'bytes'.
> 
> Update MADV_DONTNEED and MADV_REMOVE sections to remove notes that
> HugeTLB mappings are not supported.  Indicate the releases when they
> were first supported as well as alignment restrictions.
> 
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>

Could you please rebase against man-pages-5.19-rc1 in the kernel.org 
repo?  There have been important changes, especially in the formatting.

Thanks,

Alex

> ---
> v1 -> v2 Added releases when Huge TLB support was added and moved
> 	alignment requirements to corresponding section.  (Peter)
> 
>   man2/madvise.2 | 31 +++++++++++++++++++++----------
>   1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/man2/madvise.2 b/man2/madvise.2
> index f1f384c0c..50686e7e3 100644
> --- a/man2/madvise.2
> +++ b/man2/madvise.2
> @@ -61,9 +61,13 @@ system call is used to give advice or directions to the kernel
>   about the address range beginning at address
>   .I addr
>   and with size
> +.IR length .
> +.BR madvise ()
> +only operates on whole pages, therefore
> +.I addr
> +must be page-aligned.  The value of

Please break input lines at end-of-phrase/clause/sentence puntuation.

See man-pages(7):
    Use semantic newlines
        In the source of a manual page, new sentences  should  be
        started  on new lines, and long sentences should be split
        into lines at clause breaks (commas, semicolons,  colons,
        and  so on).  This convention, sometimes known as "seman‐
        tic newlines", makes it  easier  to  see  the  effect  of
        patches,  which  often operate at the level of individual
        sentences or sentence clauses.

>   .I length
> -bytes
> -In most cases,
> +is rounded up to a multiple of page size.  In most cases,

Same here.

>   the goal of such advice is to improve system or application performance.
>   .PP
>   Initially, the system call supported a set of "conventional"
> @@ -143,7 +147,7 @@ The resident set size (RSS) of the calling process will be immediately
>   reduced however.
>   .IP
>   .B MADV_DONTNEED
> -cannot be applied to locked pages, Huge TLB pages, or
> +cannot be applied to locked pages, or
>   .BR VM_PFNMAP
>   pages.
>   (Pages marked with the kernel-internal
> @@ -153,6 +157,11 @@ flag are special memory areas that are not managed
>   by the virtual memory subsystem.
>   Such pages are typically created by device drivers that
>   map the pages into user space.)
> +.IP
> +Support for Huge TLB pages was added in Linux v5.18.  Addresses within a

Same here.  And also break sentences at semantically-relevant points, 
instead of just before the 80 column.

> +mapping backed by Huge TLB pages must be aligned to the underlying Huge TLB
> +page size, and the range length is rounded up to a multiple of the underlying
> +Huge TLB page size.
>   .\"
>   .\" ======================================================================
>   .\"
> @@ -170,24 +179,24 @@ Note that some of these operations change the semantics of memory accesses.
>   .\" commit f6b3ec238d12c8cc6cc71490c6e3127988460349
>   Free up a given range of pages
>   and its associated backing store.
> -This is equivalent to punching a hole in the corresponding byte
> +This is equivalent to punching a hole in the corresponding
>   range of the backing store (see
>   .BR fallocate (2)).
>   Subsequent accesses in the specified address range will see
> -bytes containing zero.
> +pages containing zero.
>   .\" Databases want to use this feature to drop a section of their
>   .\" bufferpool (shared memory segments) - without writing back to
>   .\" disk/swap space.  This feature is also useful for supporting
>   .\" hot-plug memory on UML.
>   .IP
>   The specified address range must be mapped shared and writable.
> -This flag cannot be applied to locked pages, Huge TLB pages, or
> +This flag cannot be applied to locked pages, or
>   .BR VM_PFNMAP
>   pages.
>   .IP
>   In the initial implementation, only
>   .BR tmpfs (5)
> -was supported
> +supported
>   .BR MADV_REMOVE ;
>   but since Linux 3.5,
>   .\" commit 3f31d07571eeea18a7d34db9af21d2285b807a17
> @@ -196,10 +205,12 @@ any filesystem which supports the
>   .BR FALLOC_FL_PUNCH_HOLE
>   mode also supports
>   .BR MADV_REMOVE .
> -Hugetlbfs fails with the error
> -.BR EINVAL > -and other filesystems fail with the error
> +Filesystems which do not support
> +.BR MADV_REMOVE

s/BR/B/

See man(7):

        .B  Bold

        .BR Bold  alternating  with  Roman (especially useful for
            referring to other manual pages)

> +fail with the error
>   .BR EOPNOTSUPP .
> +.IP
> +Support for Huge TLB filesystem was added in Linux v4.3.
>   .TP
>   .BR MADV_DONTFORK " (since Linux 2.6.16)"
>   .\" commit f822566165dd46ff5de9bf895cfa6c51f53bb0c4

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

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