| Message ID | 20220608234517.117295-1-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] madvise.2: Clarify addr/length and update hugetlb support | expand |
Hi Mike, On 6/9/22 01:45, 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> > Acked-by: Peter Xu <peterx@redhat.com> Please check some comments below. Thanks, Alex > --- > v2 -> v3 Rebased on man-pages-5.19-rc1. Minor change to wording for > sunsequent access of data after MADV_REMOVE. > 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 2a8f1cd0a..becddce93 100644 > --- a/man2/madvise.2 > +++ b/man2/madvise.2 > @@ -44,9 +44,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, use semantic newlines (i.e., break after '.'). See man-pages(7): 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 bound- aries. 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. > .I length > -bytes. > -In most cases, > +is rounded up to a multiple of page size. In most cases, > the goal of such advice is to improve system or application performance. > .PP > Initially, the system call supported a set of "conventional" > @@ -126,7 +130,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 > .B VM_PFNMAP > pages. > (Pages marked with the kernel-internal > @@ -136,6 +140,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 > +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. > .\" > .\" ====================================================================== > .\" > @@ -153,24 +162,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. > +data with a value of 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 > .B 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 > @@ -179,10 +188,12 @@ any filesystem which supports the > .B FALLOC_FL_PUNCH_HOLE > mode also supports > .BR MADV_REMOVE . > -Hugetlbfs fails with the error > -.B EINVAL > -and other filesystems fail with the error > +Filesystems which do not support > +.BR MADV_REMOVE s/BR/B/ See man(7): .B Bold .BI Bold alternating with italics (especially useful for function specifications) .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
On 6/9/22 06:24, Alejandro Colomar wrote: > Hi Mike, > > On 6/9/22 01:45, 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> >> Acked-by: Peter Xu <peterx@redhat.com> > > Please check some comments below. > > Thanks, > Thank you! And, my apologies for not looking at formatting requirements. Will send a v4 shortly.
On Thu, Jun 09, 2022 at 11:48:06AM -0700, Mike Kravetz wrote: > On 6/9/22 06:24, Alejandro Colomar wrote: > > Hi Mike, > > > > On 6/9/22 01:45, 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> > >> Acked-by: Peter Xu <peterx@redhat.com> > > > > Please check some comments below. > > > > Thanks, > > > > Thank you! > > And, my apologies for not looking at formatting requirements. > Will send a v4 shortly. Alex, Do you think we could add some of the semantic newline requirement into CONTRIBUTING file explicitly? Although there's a pointer to man7 man-pages but the semantic newlines rules seem to be easily overlooked. IMHO there can even be examples as you quoted in the link on "UNIX For Beginners": https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?h=alx/main&id=6ff6f43d68164f99a8c3fb66f4525d145571310c Maybe that'll ease both the maintainers and the contributors? Thanks,
On 09.06.22 22:13, Peter Xu wrote: > On Thu, Jun 09, 2022 at 11:48:06AM -0700, Mike Kravetz wrote: >> On 6/9/22 06:24, Alejandro Colomar wrote: >>> Hi Mike, >>> >>> On 6/9/22 01:45, 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> >>>> Acked-by: Peter Xu <peterx@redhat.com> >>> >>> Please check some comments below. >>> >>> Thanks, >>> >> >> Thank you! >> >> And, my apologies for not looking at formatting requirements. >> Will send a v4 shortly. > > Alex, > > Do you think we could add some of the semantic newline requirement into > CONTRIBUTING file explicitly? Although there's a pointer to man7 man-pages > but the semantic newlines rules seem to be easily overlooked. > > IMHO there can even be examples as you quoted in the link on "UNIX For > Beginners": > > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?h=alx/main&id=6ff6f43d68164f99a8c3fb66f4525d145571310c > > Maybe that'll ease both the maintainers and the contributors? Maybe something similar checkpatch in Linux/qemu could point out a lot of these possible issues and reduce Maintainer overhead. Nobody reads docs after all if not forced to ;)
Hi, David and Peter! On 6/10/22 11:01, David Hildenbrand wrote: > On 09.06.22 22:13, Peter Xu wrote: >> Alex, >> >> Do you think we could add some of the semantic newline requirement into >> CONTRIBUTING file explicitly? I plan to add something there by the end of this year more or less, when groff-1.23.0 is released. See below. >> Although there's a pointer to man7 man-pages >> but the semantic newlines rules seem to be easily overlooked. >> >> IMHO there can even be examples as you quoted in the link on "UNIX For >> Beginners": >> >> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man7/man-pages.7?h=alx/main&id=6ff6f43d68164f99a8c3fb66f4525d145571310c Yes, I think I'll point to the 'UNIX for Beginners::Hints for Preparing Documents' thing from man-pages(7) with a link. Good idea. >> >> Maybe that'll ease both the maintainers and the contributors? > > Maybe something similar checkpatch in Linux/qemu could point out a lot > of these possible issues and reduce Maintainer overhead. Nobody reads > docs after all if not forced to ;) > > Actually, I was working on something like that. I didn't tell because it's under development, and requires to build groff(1) from source from their git HEAD (I will probably make it more public when groff-1.23.0 is released). It even runs checkpatch(1) (or a fork of it that I did to make it standalone)[1] for the EXAMPLES programs. [1]: <http://www.alejandro-colomar.es/src/alx/linux/checkpatch.git/> You can run `make -i lint` in the man-pages source tree _before_ applying your patch, to ignore all warnings not caused by you, and then run `make lint` _after_ your patch to see some warnings caused by your patch. You'll also see warnings not created by you but that are in the same file that you modified, but you can ignore them. It's not perfect, and currently it doesn't detect semantic newline problems, I think, but I guess it could be improved in the future. There's also another make(1) target to compile and link the EXAMPLES programs, in case you want to try it, but I'm also working on it and still has many issues. But if you want to try it, it's `make -i build-ld`, and then `make build-ld`. You can try it, and maybe it helps you a bit. Those (lint and build) targets also have subtargets, so you can use autocomplete to see what subtargets there are, and run only the ones you're interested in. The Makefile may be too cryptic to try to read it from there, but you can try. :) You'll also see a lot of EXTRA_*FLAGS to modify commands. Since I expect groff-1.23.0 to be released by the end of this year, I think I'll wait for then to make these features public, and that'll give me some time to polish them. Cheers, Alex
On Fri, Jun 10, 2022 at 11:38:53AM +0200, Alejandro Colomar wrote: > It's not perfect, and currently it doesn't detect semantic newline problems, > I think, but I guess it could be improved in the future. Semantic newlines can be challenging as IIUC it's not deterministic? I mean, I had a feeling that some paragraph could have multiple valid ways to do the layout and newlines without violating the rule. IMHO that could be a challenging part for contributors. (Or maybe the rule was deterministic but I didn't really fully digest it..) But the tool (even without the newline detections) looks promising and helpful. Thanks,
On 6/10/22 16:07, Peter Xu wrote: > On Fri, Jun 10, 2022 at 11:38:53AM +0200, Alejandro Colomar wrote: >> It's not perfect, and currently it doesn't detect semantic newline problems, >> I think, but I guess it could be improved in the future. > > Semantic newlines can be challenging as IIUC it's not deterministic? > > I mean, I had a feeling that some paragraph could have multiple valid ways > to do the layout and newlines without violating the rule. IMHO that could > be a challenging part for contributors. > > (Or maybe the rule was deterministic but I didn't really fully digest it..) You're completely right; it's not machine parseable. But it could detect the easiest stuff such as "foo. Bar" for example. And only maybe "foo, bar". But going further, that's a job for humans. And yes, many times there are several ways to break the line while still following the rules. > > But the tool (even without the newline detections) looks promising and > helpful. Thanks! I'll try to make it usable. Cheers, Alex > > Thanks, >
Hi Mike, On 6/9/22 20:48, Mike Kravetz wrote: > On 6/9/22 06:24, Alejandro Colomar wrote: >> Hi Mike, >> >> On 6/9/22 01:45, 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> >>> Acked-by: Peter Xu <peterx@redhat.com> >> >> Please check some comments below. >> >> Thanks, >> > > Thank you! > > And, my apologies for not looking at formatting requirements. > Will send a v4 shortly. > I was revising old patches. I'm just pinging you on this, to make sure it's not lost. Thanks, Alex
On 8/15/22 22:41, Alejandro Colomar wrote: > Hi Mike, > > On 6/9/22 20:48, Mike Kravetz wrote: >> On 6/9/22 06:24, Alejandro Colomar wrote: >>> Hi Mike, >>> >>> On 6/9/22 01:45, 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> >>>> Acked-by: Peter Xu <peterx@redhat.com> >>> >>> Please check some comments below. >>> >>> Thanks, >>> >> >> Thank you! >> >> And, my apologies for not looking at formatting requirements. >> Will send a v4 shortly. >> > > I was revising old patches. I'm just pinging you on this, to make sure > it's not lost. Never mind, I just checked that the patch was already applied. I didn't find it by some error. :/ Cheers, Alex
diff --git a/man2/madvise.2 b/man2/madvise.2 index 2a8f1cd0a..becddce93 100644 --- a/man2/madvise.2 +++ b/man2/madvise.2 @@ -44,9 +44,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 .I length -bytes. -In most cases, +is rounded up to a multiple of page size. In most cases, the goal of such advice is to improve system or application performance. .PP Initially, the system call supported a set of "conventional" @@ -126,7 +130,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 .B VM_PFNMAP pages. (Pages marked with the kernel-internal @@ -136,6 +140,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 +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. .\" .\" ====================================================================== .\" @@ -153,24 +162,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. +data with a value of 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 .B 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 @@ -179,10 +188,12 @@ any filesystem which supports the .B FALLOC_FL_PUNCH_HOLE mode also supports .BR MADV_REMOVE . -Hugetlbfs fails with the error -.B EINVAL -and other filesystems fail with the error +Filesystems which do not support +.BR MADV_REMOVE +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