On Fri, Dec 06, 2024 at 11:03:56AM +0000, Lorenzo Stoakes wrote: > On Thu, Dec 05, 2024 at 09:43:09PM +0100, Vlastimil Babka wrote: > > On 12/5/24 19:09, Lorenzo Stoakes wrote: > > > On Thu, Dec 05, 2024 at 06:50:09PM +0100, Vlastimil Babka wrote: > > >> On 12/5/24 11:41, 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. > > >> > > > >> > Reviewed-by: Jann Horn <jannh@xxxxxxxxxx> > > >> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> > > >> > --- > > >> > 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. > > >> > * 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 > > >> > > >> Hm this reads a bit ambiguous. One could read it as the memory pages are > > >> being cleared, but it's the page tables. > > > > > > This was really hard to word, because you don't want to say unmapped, and saying > > > 'clearing page tables' or 'zapping' is clear to us but not necessarily to a > > > reader. 'Clearing mapping' makes it ambiguous vs. munmap(), etc. etc. > > > > Maybe saying "removed" instead of "cleared" would be better? > > I was about to put more philosophical points here then realised we already have > a better solution... we can say 'the regions will become zero-fill-on-demand'. > > Let me respin a v5 with this corrected. We will need to revisit once we > support file-backed pages. Actually, as discussed on IRC, I think your suggested 'replaced' is better, so: "If the region maps memory pages, those mappings will be replaced". Will address in v5. > > > > > Anyway, I don't want to cause bikeshedding, so in any case: > > No it's appreciated! Review should be critical so we can make this as good > as we can make it. > > > > > Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> > > Thanks! > > > > > > But you want to make it clear (no pun intended) that anon pages, if there's any > > > data, it's probably lost. So I think this is a distinction that doesn't matter. > > > > > > Will revisit once we support file-backed mappings. > > >
