On Thu, Dec 05, 2024 at 01:20:37PM +0100, Alejandro Colomar wrote: > Hi Lorenzo, Jann, > > > Subject: Re: [PATCH man-pages v4] madvise.2: add MADV_GUARD_INSTALL, MADV_GUARD_REMOVE description > > We use uppercase after the prefix, so s/add/Add/. > > On Thu, Dec 05, 2024 at 10:41:25AM +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 > > The right amount of inter-sentence space is two. See this: > <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/CONTRIBUTING.d/patches/description?id=bcf7d00fa4c7ce270f07d6e347c01b1f1e37580f> > <https://web.archive.org/web/20171217060354/http://www.heracliteanriver.com/?p=324> > > > operations. > > > > Reviewed-by: Jann Horn <jannh@xxxxxxxxxx> > > Thanks for the review! > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> > > Thanks for the patch! I've applied it, with some minor tweaks. See > comments below. > <https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=bb405ee3f6039226267fb1c6d2cb1fbb18d835bf> Thanks all seems reasonable. Just a quick question for future changes - I see you reference git://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git - but I've been working against git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git - is the latter occasionally synced from the former? Or should I be working against your personal repo for future changes? Thanks, Lorenzo > > Here's the diff that I applied when amending your patch: > > diff --git i/man/man2/madvise.2 w/man/man2/madvise.2 > index adb372424..fa24f6bf6 100644 > --- i/man/man2/madvise.2 > +++ w/man/man2/madvise.2 > @@ -678,7 +678,8 @@ .SS Linux-specific advice values > 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 > +Install a lightweight guard region > +into the range specified by > .I addr > and > .IR size , > @@ -686,22 +687,27 @@ .SS Linux-specific advice values > .B SIGSEGV > signal being raised. > .IP > -If the region maps memory pages they will be cleared as part of the operation, > +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, > +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. > +This operation is supported > +only 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 > +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 > @@ -740,12 +746,15 @@ .SS Linux-specific advice values > 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 > +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 > +All mappings in the range > +other than lightweight guard regions > +are left in place > (including mlock'd mappings). > The operation is, > however, > > > --- > > v4: > > * Reference function chapters as per Alejandro. > > * Minor rewording as per Alejandro. > > > > v3: > > * Don't describe SIGSEGV as a fatal signal as per Jann. > > https://lore.kernel.org/all/20241202165829.72121-1-lorenzo.stoakes@xxxxxxxxxx > > > > v2: > > * Updated to use semantic newlines as suggested by Alejandro. > > I've broken lines a little bit more, even though they were correct, just > for having shorter lines. > > > * Avoided emboldening parens as suggested by Alejandro. > > * One very minor grammatical fix. > > https://lore.kernel.org/all/20241129155943.85215-1-lorenzo.stoakes@xxxxxxxxxx > > > > v1: > > https://lore.kernel.org/all/20241129093205.8664-1-lorenzo.stoakes@xxxxxxxxxx > > > > man/man2/madvise.2 | 93 ++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 93 insertions(+) > > > > diff --git a/man/man2/madvise.2 b/man/man2/madvise.2 > > index 4f2210ee2..7d682fa40 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 > > I missed this before. It was the same misplacement of only. :) > > Have a lovely day! > Alex > > > -- > <https://www.alejandro-colomar.es/>
