* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-19 9:17 UTC (permalink / raw)
To: David Hildenbrand
Cc: Kalesh Singh, Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas
In-Reply-To: <41af4ffb-0383-4d00-9639-0bf16e1f5f37@redhat.com>
On Wed, Feb 19, 2025 at 10:15:47AM +0100, David Hildenbrand wrote:
> On 19.02.25 10:03, Lorenzo Stoakes wrote:
> > On Wed, Feb 19, 2025 at 12:25:51AM -0800, Kalesh Singh wrote:
> > > On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
> > > <lorenzo.stoakes@oracle.com> wrote:
> > > >
> > > > The guard regions feature was initially implemented to support anonymous
> > > > mappings only, excluding shmem.
> > > >
> > > > This was done such as to introduce the feature carefully and incrementally
> > > > and to be conservative when considering the various caveats and corner
> > > > cases that are applicable to file-backed mappings but not to anonymous
> > > > ones.
> > > >
> > > > Now this feature has landed in 6.13, it is time to revisit this and to
> > > > extend this functionality to file-backed and shmem mappings.
> > > >
> > > > In order to make this maximally useful, and since one may map file-backed
> > > > mappings read-only (for instance ELF images), we also remove the
> > > > restriction on read-only mappings and permit the establishment of guard
> > > > regions in any non-hugetlb, non-mlock()'d mapping.
> > >
> > > Hi Lorenzo,
> > >
> > > Thank you for your work on this.
> >
> > You're welcome.
> >
> > >
> > > Have we thought about how guard regions are represented in /proc/*/[s]maps?
> >
> > This is off-topic here but... Yes, extensively. No they do not appear
> > there.
> >
> > I thought you had attended LPC and my talk where I mentioned this
> > purposefully as a drawback?
> >
> > I went out of my way to advertise this limitation at the LPC talk, in the
> > original series, etc. so it's a little disappointing that this is being
> > brought up so late, but nobody else has raised objections to this issue so
> > I think in general it's not a limitation that matters in practice.
> >
> > >
> > > In the field, I've found that many applications read the ranges from
> > > /proc/self/[s]maps to determine what they can access (usually related
> > > to obfuscation techniques). If they don't know of the guard regions it
> > > would cause them to crash; I think that we'll need similar entries to
> > > PROT_NONE (---p) for these, and generally to maintain consistency
> > > between the behavior and what is being said from /proc/*/[s]maps.
> >
> > No, we cannot have these, sorry.
> >
> > Firstly /proc/$pid/[s]maps describes VMAs. The entire purpose of this
> > feature is to avoid having to accumulate VMAs for regions which are not
> > intended to be accessible.
> >
> > Secondly, there is no practical means for this to be accomplished in
> > /proc/$pid/maps in _any_ way - as no metadata relating to a VMA indicates
> > they have guard regions.
> >
> > This is intentional, because setting such metadata is simply not practical
> > - why? Because when you try to split the VMA, how do you know which bit
> > gets the metadata and which doesn't? You can't without _reading page
> > tables_.
> >
> > /proc/$pid/smaps _does_ read page tables, but we can't start pretending
> > VMAs exist when they don't, this would be completely inaccurate, would
> > break assumptions for things like mremap (which require a single VMA) and
> > would be unworkable.
> >
> > The best that _could_ be achieved is to have a marker in /proc/$pid/smaps
> > saying 'hey this region has guard regions somewhere'.
>
> And then simply expose it in /proc/$pid/pagemap, which is a better interface
> for this pte-level information inside of VMAs. We should still have a spare
> bit for that purpose in the pagemap entries.
Ah yeah thanks David forgot about that!
This is also a possibility if that'd solve your problems Kalesh?
This bit will be fought over haha
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-19 9:15 UTC (permalink / raw)
To: Lorenzo Stoakes, Kalesh Singh
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas
In-Reply-To: <45297010-a0a4-4a42-84e8-6f4764eab3b3@lucifer.local>
On 19.02.25 10:03, Lorenzo Stoakes wrote:
> On Wed, Feb 19, 2025 at 12:25:51AM -0800, Kalesh Singh wrote:
>> On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
>> <lorenzo.stoakes@oracle.com> wrote:
>>>
>>> The guard regions feature was initially implemented to support anonymous
>>> mappings only, excluding shmem.
>>>
>>> This was done such as to introduce the feature carefully and incrementally
>>> and to be conservative when considering the various caveats and corner
>>> cases that are applicable to file-backed mappings but not to anonymous
>>> ones.
>>>
>>> Now this feature has landed in 6.13, it is time to revisit this and to
>>> extend this functionality to file-backed and shmem mappings.
>>>
>>> In order to make this maximally useful, and since one may map file-backed
>>> mappings read-only (for instance ELF images), we also remove the
>>> restriction on read-only mappings and permit the establishment of guard
>>> regions in any non-hugetlb, non-mlock()'d mapping.
>>
>> Hi Lorenzo,
>>
>> Thank you for your work on this.
>
> You're welcome.
>
>>
>> Have we thought about how guard regions are represented in /proc/*/[s]maps?
>
> This is off-topic here but... Yes, extensively. No they do not appear
> there.
>
> I thought you had attended LPC and my talk where I mentioned this
> purposefully as a drawback?
>
> I went out of my way to advertise this limitation at the LPC talk, in the
> original series, etc. so it's a little disappointing that this is being
> brought up so late, but nobody else has raised objections to this issue so
> I think in general it's not a limitation that matters in practice.
>
>>
>> In the field, I've found that many applications read the ranges from
>> /proc/self/[s]maps to determine what they can access (usually related
>> to obfuscation techniques). If they don't know of the guard regions it
>> would cause them to crash; I think that we'll need similar entries to
>> PROT_NONE (---p) for these, and generally to maintain consistency
>> between the behavior and what is being said from /proc/*/[s]maps.
>
> No, we cannot have these, sorry.
>
> Firstly /proc/$pid/[s]maps describes VMAs. The entire purpose of this
> feature is to avoid having to accumulate VMAs for regions which are not
> intended to be accessible.
>
> Secondly, there is no practical means for this to be accomplished in
> /proc/$pid/maps in _any_ way - as no metadata relating to a VMA indicates
> they have guard regions.
>
> This is intentional, because setting such metadata is simply not practical
> - why? Because when you try to split the VMA, how do you know which bit
> gets the metadata and which doesn't? You can't without _reading page
> tables_.
>
> /proc/$pid/smaps _does_ read page tables, but we can't start pretending
> VMAs exist when they don't, this would be completely inaccurate, would
> break assumptions for things like mremap (which require a single VMA) and
> would be unworkable.
>
> The best that _could_ be achieved is to have a marker in /proc/$pid/smaps
> saying 'hey this region has guard regions somewhere'.
And then simply expose it in /proc/$pid/pagemap, which is a better
interface for this pte-level information inside of VMAs. We should still
have a spare bit for that purpose in the pagemap entries.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-19 9:15 UTC (permalink / raw)
To: Kalesh Singh
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
David Hildenbrand, linux-mm, linux-kernel, Shuah Khan,
linux-kselftest, linux-api, John Hubbard, Juan Yescas
In-Reply-To: <CAC_TJvfPNkJDWnG81GnJcFeMLYzN8=uM-oTrK6FKT7tD=E4TQg@mail.gmail.com>
On Wed, Feb 19, 2025 at 12:35:12AM -0800, Kalesh Singh wrote:
> On Wed, Feb 19, 2025 at 12:25 AM Kalesh Singh <kaleshsingh@google.com> wrote:
> >
> > On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
> > <lorenzo.stoakes@oracle.com> wrote:
> > >
> > > The guard regions feature was initially implemented to support anonymous
> > > mappings only, excluding shmem.
> > >
> > > This was done such as to introduce the feature carefully and incrementally
> > > and to be conservative when considering the various caveats and corner
> > > cases that are applicable to file-backed mappings but not to anonymous
> > > ones.
> > >
> > > Now this feature has landed in 6.13, it is time to revisit this and to
> > > extend this functionality to file-backed and shmem mappings.
> > >
> > > In order to make this maximally useful, and since one may map file-backed
> > > mappings read-only (for instance ELF images), we also remove the
> > > restriction on read-only mappings and permit the establishment of guard
> > > regions in any non-hugetlb, non-mlock()'d mapping.
> >
> > Hi Lorenzo,
> >
> > Thank you for your work on this.
> >
> > Have we thought about how guard regions are represented in /proc/*/[s]maps?
> >
> > In the field, I've found that many applications read the ranges from
> > /proc/self/[s]maps to determine what they can access (usually related
> > to obfuscation techniques). If they don't know of the guard regions it
> > would cause them to crash; I think that we'll need similar entries to
> > PROT_NONE (---p) for these, and generally to maintain consistency
> > between the behavior and what is being said from /proc/*/[s]maps.
>
> To clarify why the applications may not be aware of their guard
> regions -- in the case of the ELF mappings these PROT_NONE (guard
> regions) would be installed by the dynamic loader; or may be inherited
> from the parent (zygote in Android's case).
I'm still really confused as to how you propose to use guard regions in
this scenario. If it's trying to 'clone' some set of mappings somehow then
guard regions are just not compatible with your use case at all.
I'm not sure what would be honestly... you maybe could find a way to modify
the loader to interpret PROT_NONE mappings as a cue to use guard regions
instead if that worked?
You can't both not have a VMA (something that describes mapping with common
attributes) and have page table state along with somehow having something
that describes a mapping with said page table state :)
It's a sort of non-such-beast.
Everything is a trade-off, guard regions provide the option to retain state
regarding faulting behaviour in page tables rather the VMAs, entirely
opt-in and at the behest of the user.
Now extended to file-backed and shmem!
While I hoped it would help your scenario based on your talk at LPC, this
is very clearly an important change for guard regions (for shmem most
notably) and removes an important limitation and exposes the possibility of
using it for things such as ELF mappings for those who don't also require
specific [s]maps state (which explicitly NEEDS VMA metadata state).
I suspect you can find a way of making this work in your scenario
though.
An alternative if you're willing to wait for page table state to be
acquired is to expose a new /proc/$pid/... endpoint that explicitly outputs
guard regions.
Would this solve your problem...? I had always considered implementing this
if there was a demand.
Maybe /proc/$pid/gmaps? :P 'g' could mean guard regions or... ;)
>
> >
> > -- Kalesh
> >
> > >
> > > It is permissible to permit the establishment of guard regions in read-only
> > > mappings because the guard regions only reduce access to the mapping, and
> > > when removed simply reinstate the existing attributes of the underlying
> > > VMA, meaning no access violations can occur.
> > >
> > > While the change in kernel code introduced in this series is small, the
> > > majority of the effort here is spent in extending the testing to assert
> > > that the feature works correctly across numerous file-backed mapping
> > > scenarios.
> > >
> > > Every single guard region self-test performed against anonymous memory
> > > (which is relevant and not anon-only) has now been updated to also be
> > > performed against shmem and a mapping of a file in the working directory.
> > >
> > > This confirms that all cases also function correctly for file-backed guard
> > > regions.
> > >
> > > In addition a number of other tests are added for specific file-backed
> > > mapping scenarios.
> > >
> > > There are a number of other concerns that one might have with regard to
> > > guard regions, addressed below:
> > >
> > > Readahead
> > > ~~~~~~~~~
> > >
> > > Readahead is a process through which the page cache is populated on the
> > > assumption that sequential reads will occur, thus amortising I/O and,
> > > through a clever use of the PG_readahead folio flag establishing during
> > > major fault and checked upon minor fault, provides for asynchronous I/O to
> > > occur as dat is processed, reducing I/O stalls as data is faulted in.
> > >
> > > Guard regions do not alter this mechanism which operations at the folio and
> > > fault level, but do of course prevent the faulting of folios that would
> > > otherwise be mapped.
> > >
> > > In the instance of a major fault prior to a guard region, synchronous
> > > readahead will occur including populating folios in the page cache which
> > > the guard regions will, in the case of the mapping in question, prevent
> > > access to.
> > >
> > > In addition, if PG_readahead is placed in a folio that is now inaccessible,
> > > this will prevent asynchronous readahead from occurring as it would
> > > otherwise do.
> > >
> > > However, there are mechanisms for heuristically resetting this within
> > > readahead regardless, which will 'recover' correct readahead behaviour.
> > >
> > > Readahead presumes sequential data access, the presence of a guard region
> > > clearly indicates that, at least in the guard region, no such sequential
> > > access will occur, as it cannot occur there.
> > >
> > > So this should have very little impact on any real workload. The far more
> > > important point is as to whether readahead causes incorrect or
> > > inappropriate mapping of ranges disallowed by the presence of guard
> > > regions - this is not the case, as readahead does not 'pre-fault' memory in
> > > this fashion.
> > >
> > > At any rate, any mechanism which would attempt to do so would hit the usual
> > > page fault paths, which correctly handle PTE markers as with anonymous
> > > mappings.
> > >
> > > Fault-Around
> > > ~~~~~~~~~~~~
> > >
> > > The fault-around logic, in a similar vein to readahead, attempts to improve
> > > efficiency with regard to file-backed memory mappings, however it differs
> > > in that it does not try to fetch folios into the page cache that are about
> > > to be accessed, but rather pre-maps a range of folios around the faulting
> > > address.
> > >
> > > Guard regions making use of PTE markers makes this relatively trivial, as
> > > this case is already handled - see filemap_map_folio_range() and
> > > filemap_map_order0_folio() - in both instances, the solution is to simply
> > > keep the established page table mappings and let the fault handler take
> > > care of PTE markers, as per the comment:
> > >
> > > /*
> > > * NOTE: If there're PTE markers, we'll leave them to be
> > > * handled in the specific fault path, and it'll prohibit
> > > * the fault-around logic.
> > > */
> > >
> > > This works, as establishing guard regions results in page table mappings
> > > with PTE markers, and clearing them removes them.
> > >
> > > Truncation
> > > ~~~~~~~~~~
> > >
> > > File truncation will not eliminate existing guard regions, as the
> > > truncation operation will ultimately zap the range via
> > > unmap_mapping_range(), which specifically excludes PTE markers.
> > >
> > > Zapping
> > > ~~~~~~~
> > >
> > > Zapping is, as with anonymous mappings, handled by zap_nonpresent_ptes(),
> > > which specifically deals with guard entries, leaving them intact except in
> > > instances such as process teardown or munmap() where they need to be
> > > removed.
> > >
> > > Reclaim
> > > ~~~~~~~
> > >
> > > When reclaim is performed on file-backed folios, it ultimately invokes
> > > try_to_unmap_one() via the rmap. If the folio is non-large, then map_pte()
> > > will ultimately abort the operation for the guard region mapping. If large,
> > > then check_pte() will determine that this is a non-device private
> > > entry/device-exclusive entry 'swap' PTE and thus abort the operation in
> > > that instance.
> > >
> > > Therefore, no odd things happen in the instance of reclaim being attempted
> > > upon a file-backed guard region.
> > >
> > > Hole Punching
> > > ~~~~~~~~~~~~~
> > >
> > > This updates the page cache and ultimately invokes unmap_mapping_range(),
> > > which explicitly leaves PTE markers in place.
> > >
> > > Because the establishment of guard regions zapped any existing mappings to
> > > file-backed folios, once the guard regions are removed then the
> > > hole-punched region will be faulted in as usual and everything will behave
> > > as expected.
> > >
> > > Lorenzo Stoakes (4):
> > > mm: allow guard regions in file-backed and read-only mappings
> > > selftests/mm: rename guard-pages to guard-regions
> > > tools/selftests: expand all guard region tests to file-backed
> > > tools/selftests: add file/shmem-backed mapping guard region tests
> > >
> > > mm/madvise.c | 8 +-
> > > tools/testing/selftests/mm/.gitignore | 2 +-
> > > tools/testing/selftests/mm/Makefile | 2 +-
> > > .../mm/{guard-pages.c => guard-regions.c} | 921 ++++++++++++++++--
> > > 4 files changed, 821 insertions(+), 112 deletions(-)
> > > rename tools/testing/selftests/mm/{guard-pages.c => guard-regions.c} (58%)
> > >
> > > --
> > > 2.48.1
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-19 9:03 UTC (permalink / raw)
To: Kalesh Singh
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
David Hildenbrand, linux-mm, linux-kernel, Shuah Khan,
linux-kselftest, linux-api, John Hubbard, Juan Yescas
In-Reply-To: <CAC_TJveMB1_iAUt81D5-+z8gArbVcbfDM=djCZG_bRVaCEMRmg@mail.gmail.com>
On Wed, Feb 19, 2025 at 12:25:51AM -0800, Kalesh Singh wrote:
> On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > The guard regions feature was initially implemented to support anonymous
> > mappings only, excluding shmem.
> >
> > This was done such as to introduce the feature carefully and incrementally
> > and to be conservative when considering the various caveats and corner
> > cases that are applicable to file-backed mappings but not to anonymous
> > ones.
> >
> > Now this feature has landed in 6.13, it is time to revisit this and to
> > extend this functionality to file-backed and shmem mappings.
> >
> > In order to make this maximally useful, and since one may map file-backed
> > mappings read-only (for instance ELF images), we also remove the
> > restriction on read-only mappings and permit the establishment of guard
> > regions in any non-hugetlb, non-mlock()'d mapping.
>
> Hi Lorenzo,
>
> Thank you for your work on this.
You're welcome.
>
> Have we thought about how guard regions are represented in /proc/*/[s]maps?
This is off-topic here but... Yes, extensively. No they do not appear
there.
I thought you had attended LPC and my talk where I mentioned this
purposefully as a drawback?
I went out of my way to advertise this limitation at the LPC talk, in the
original series, etc. so it's a little disappointing that this is being
brought up so late, but nobody else has raised objections to this issue so
I think in general it's not a limitation that matters in practice.
>
> In the field, I've found that many applications read the ranges from
> /proc/self/[s]maps to determine what they can access (usually related
> to obfuscation techniques). If they don't know of the guard regions it
> would cause them to crash; I think that we'll need similar entries to
> PROT_NONE (---p) for these, and generally to maintain consistency
> between the behavior and what is being said from /proc/*/[s]maps.
No, we cannot have these, sorry.
Firstly /proc/$pid/[s]maps describes VMAs. The entire purpose of this
feature is to avoid having to accumulate VMAs for regions which are not
intended to be accessible.
Secondly, there is no practical means for this to be accomplished in
/proc/$pid/maps in _any_ way - as no metadata relating to a VMA indicates
they have guard regions.
This is intentional, because setting such metadata is simply not practical
- why? Because when you try to split the VMA, how do you know which bit
gets the metadata and which doesn't? You can't without _reading page
tables_.
/proc/$pid/smaps _does_ read page tables, but we can't start pretending
VMAs exist when they don't, this would be completely inaccurate, would
break assumptions for things like mremap (which require a single VMA) and
would be unworkable.
The best that _could_ be achieved is to have a marker in /proc/$pid/smaps
saying 'hey this region has guard regions somewhere'.
But I haven't seen any demand for this and presumably this wouldn't help
your imagined program?
I don't really understand your use case though, what programs would read
/proc/maps, then... try to use /proc/$pid/mem or whatnot to arbitrarily
read regions? Such applications would be in danger of SIGBUS in any case if
they were to read invalid portions of file-backed mappings, and have no way
of knowing this, so they seem fundamentally broken as it is?
>
> -- Kalesh
>
> >
> > It is permissible to permit the establishment of guard regions in read-only
> > mappings because the guard regions only reduce access to the mapping, and
> > when removed simply reinstate the existing attributes of the underlying
> > VMA, meaning no access violations can occur.
> >
> > While the change in kernel code introduced in this series is small, the
> > majority of the effort here is spent in extending the testing to assert
> > that the feature works correctly across numerous file-backed mapping
> > scenarios.
> >
> > Every single guard region self-test performed against anonymous memory
> > (which is relevant and not anon-only) has now been updated to also be
> > performed against shmem and a mapping of a file in the working directory.
> >
> > This confirms that all cases also function correctly for file-backed guard
> > regions.
> >
> > In addition a number of other tests are added for specific file-backed
> > mapping scenarios.
> >
> > There are a number of other concerns that one might have with regard to
> > guard regions, addressed below:
> >
> > Readahead
> > ~~~~~~~~~
> >
> > Readahead is a process through which the page cache is populated on the
> > assumption that sequential reads will occur, thus amortising I/O and,
> > through a clever use of the PG_readahead folio flag establishing during
> > major fault and checked upon minor fault, provides for asynchronous I/O to
> > occur as dat is processed, reducing I/O stalls as data is faulted in.
> >
> > Guard regions do not alter this mechanism which operations at the folio and
> > fault level, but do of course prevent the faulting of folios that would
> > otherwise be mapped.
> >
> > In the instance of a major fault prior to a guard region, synchronous
> > readahead will occur including populating folios in the page cache which
> > the guard regions will, in the case of the mapping in question, prevent
> > access to.
> >
> > In addition, if PG_readahead is placed in a folio that is now inaccessible,
> > this will prevent asynchronous readahead from occurring as it would
> > otherwise do.
> >
> > However, there are mechanisms for heuristically resetting this within
> > readahead regardless, which will 'recover' correct readahead behaviour.
> >
> > Readahead presumes sequential data access, the presence of a guard region
> > clearly indicates that, at least in the guard region, no such sequential
> > access will occur, as it cannot occur there.
> >
> > So this should have very little impact on any real workload. The far more
> > important point is as to whether readahead causes incorrect or
> > inappropriate mapping of ranges disallowed by the presence of guard
> > regions - this is not the case, as readahead does not 'pre-fault' memory in
> > this fashion.
> >
> > At any rate, any mechanism which would attempt to do so would hit the usual
> > page fault paths, which correctly handle PTE markers as with anonymous
> > mappings.
> >
> > Fault-Around
> > ~~~~~~~~~~~~
> >
> > The fault-around logic, in a similar vein to readahead, attempts to improve
> > efficiency with regard to file-backed memory mappings, however it differs
> > in that it does not try to fetch folios into the page cache that are about
> > to be accessed, but rather pre-maps a range of folios around the faulting
> > address.
> >
> > Guard regions making use of PTE markers makes this relatively trivial, as
> > this case is already handled - see filemap_map_folio_range() and
> > filemap_map_order0_folio() - in both instances, the solution is to simply
> > keep the established page table mappings and let the fault handler take
> > care of PTE markers, as per the comment:
> >
> > /*
> > * NOTE: If there're PTE markers, we'll leave them to be
> > * handled in the specific fault path, and it'll prohibit
> > * the fault-around logic.
> > */
> >
> > This works, as establishing guard regions results in page table mappings
> > with PTE markers, and clearing them removes them.
> >
> > Truncation
> > ~~~~~~~~~~
> >
> > File truncation will not eliminate existing guard regions, as the
> > truncation operation will ultimately zap the range via
> > unmap_mapping_range(), which specifically excludes PTE markers.
> >
> > Zapping
> > ~~~~~~~
> >
> > Zapping is, as with anonymous mappings, handled by zap_nonpresent_ptes(),
> > which specifically deals with guard entries, leaving them intact except in
> > instances such as process teardown or munmap() where they need to be
> > removed.
> >
> > Reclaim
> > ~~~~~~~
> >
> > When reclaim is performed on file-backed folios, it ultimately invokes
> > try_to_unmap_one() via the rmap. If the folio is non-large, then map_pte()
> > will ultimately abort the operation for the guard region mapping. If large,
> > then check_pte() will determine that this is a non-device private
> > entry/device-exclusive entry 'swap' PTE and thus abort the operation in
> > that instance.
> >
> > Therefore, no odd things happen in the instance of reclaim being attempted
> > upon a file-backed guard region.
> >
> > Hole Punching
> > ~~~~~~~~~~~~~
> >
> > This updates the page cache and ultimately invokes unmap_mapping_range(),
> > which explicitly leaves PTE markers in place.
> >
> > Because the establishment of guard regions zapped any existing mappings to
> > file-backed folios, once the guard regions are removed then the
> > hole-punched region will be faulted in as usual and everything will behave
> > as expected.
> >
> > Lorenzo Stoakes (4):
> > mm: allow guard regions in file-backed and read-only mappings
> > selftests/mm: rename guard-pages to guard-regions
> > tools/selftests: expand all guard region tests to file-backed
> > tools/selftests: add file/shmem-backed mapping guard region tests
> >
> > mm/madvise.c | 8 +-
> > tools/testing/selftests/mm/.gitignore | 2 +-
> > tools/testing/selftests/mm/Makefile | 2 +-
> > .../mm/{guard-pages.c => guard-regions.c} | 921 ++++++++++++++++--
> > 4 files changed, 821 insertions(+), 112 deletions(-)
> > rename tools/testing/selftests/mm/{guard-pages.c => guard-regions.c} (58%)
> >
> > --
> > 2.48.1
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Kalesh Singh @ 2025-02-19 8:35 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
David Hildenbrand, linux-mm, linux-kernel, Shuah Khan,
linux-kselftest, linux-api, John Hubbard, Juan Yescas
In-Reply-To: <CAC_TJveMB1_iAUt81D5-+z8gArbVcbfDM=djCZG_bRVaCEMRmg@mail.gmail.com>
On Wed, Feb 19, 2025 at 12:25 AM Kalesh Singh <kaleshsingh@google.com> wrote:
>
> On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > The guard regions feature was initially implemented to support anonymous
> > mappings only, excluding shmem.
> >
> > This was done such as to introduce the feature carefully and incrementally
> > and to be conservative when considering the various caveats and corner
> > cases that are applicable to file-backed mappings but not to anonymous
> > ones.
> >
> > Now this feature has landed in 6.13, it is time to revisit this and to
> > extend this functionality to file-backed and shmem mappings.
> >
> > In order to make this maximally useful, and since one may map file-backed
> > mappings read-only (for instance ELF images), we also remove the
> > restriction on read-only mappings and permit the establishment of guard
> > regions in any non-hugetlb, non-mlock()'d mapping.
>
> Hi Lorenzo,
>
> Thank you for your work on this.
>
> Have we thought about how guard regions are represented in /proc/*/[s]maps?
>
> In the field, I've found that many applications read the ranges from
> /proc/self/[s]maps to determine what they can access (usually related
> to obfuscation techniques). If they don't know of the guard regions it
> would cause them to crash; I think that we'll need similar entries to
> PROT_NONE (---p) for these, and generally to maintain consistency
> between the behavior and what is being said from /proc/*/[s]maps.
To clarify why the applications may not be aware of their guard
regions -- in the case of the ELF mappings these PROT_NONE (guard
regions) would be installed by the dynamic loader; or may be inherited
from the parent (zygote in Android's case).
>
> -- Kalesh
>
> >
> > It is permissible to permit the establishment of guard regions in read-only
> > mappings because the guard regions only reduce access to the mapping, and
> > when removed simply reinstate the existing attributes of the underlying
> > VMA, meaning no access violations can occur.
> >
> > While the change in kernel code introduced in this series is small, the
> > majority of the effort here is spent in extending the testing to assert
> > that the feature works correctly across numerous file-backed mapping
> > scenarios.
> >
> > Every single guard region self-test performed against anonymous memory
> > (which is relevant and not anon-only) has now been updated to also be
> > performed against shmem and a mapping of a file in the working directory.
> >
> > This confirms that all cases also function correctly for file-backed guard
> > regions.
> >
> > In addition a number of other tests are added for specific file-backed
> > mapping scenarios.
> >
> > There are a number of other concerns that one might have with regard to
> > guard regions, addressed below:
> >
> > Readahead
> > ~~~~~~~~~
> >
> > Readahead is a process through which the page cache is populated on the
> > assumption that sequential reads will occur, thus amortising I/O and,
> > through a clever use of the PG_readahead folio flag establishing during
> > major fault and checked upon minor fault, provides for asynchronous I/O to
> > occur as dat is processed, reducing I/O stalls as data is faulted in.
> >
> > Guard regions do not alter this mechanism which operations at the folio and
> > fault level, but do of course prevent the faulting of folios that would
> > otherwise be mapped.
> >
> > In the instance of a major fault prior to a guard region, synchronous
> > readahead will occur including populating folios in the page cache which
> > the guard regions will, in the case of the mapping in question, prevent
> > access to.
> >
> > In addition, if PG_readahead is placed in a folio that is now inaccessible,
> > this will prevent asynchronous readahead from occurring as it would
> > otherwise do.
> >
> > However, there are mechanisms for heuristically resetting this within
> > readahead regardless, which will 'recover' correct readahead behaviour.
> >
> > Readahead presumes sequential data access, the presence of a guard region
> > clearly indicates that, at least in the guard region, no such sequential
> > access will occur, as it cannot occur there.
> >
> > So this should have very little impact on any real workload. The far more
> > important point is as to whether readahead causes incorrect or
> > inappropriate mapping of ranges disallowed by the presence of guard
> > regions - this is not the case, as readahead does not 'pre-fault' memory in
> > this fashion.
> >
> > At any rate, any mechanism which would attempt to do so would hit the usual
> > page fault paths, which correctly handle PTE markers as with anonymous
> > mappings.
> >
> > Fault-Around
> > ~~~~~~~~~~~~
> >
> > The fault-around logic, in a similar vein to readahead, attempts to improve
> > efficiency with regard to file-backed memory mappings, however it differs
> > in that it does not try to fetch folios into the page cache that are about
> > to be accessed, but rather pre-maps a range of folios around the faulting
> > address.
> >
> > Guard regions making use of PTE markers makes this relatively trivial, as
> > this case is already handled - see filemap_map_folio_range() and
> > filemap_map_order0_folio() - in both instances, the solution is to simply
> > keep the established page table mappings and let the fault handler take
> > care of PTE markers, as per the comment:
> >
> > /*
> > * NOTE: If there're PTE markers, we'll leave them to be
> > * handled in the specific fault path, and it'll prohibit
> > * the fault-around logic.
> > */
> >
> > This works, as establishing guard regions results in page table mappings
> > with PTE markers, and clearing them removes them.
> >
> > Truncation
> > ~~~~~~~~~~
> >
> > File truncation will not eliminate existing guard regions, as the
> > truncation operation will ultimately zap the range via
> > unmap_mapping_range(), which specifically excludes PTE markers.
> >
> > Zapping
> > ~~~~~~~
> >
> > Zapping is, as with anonymous mappings, handled by zap_nonpresent_ptes(),
> > which specifically deals with guard entries, leaving them intact except in
> > instances such as process teardown or munmap() where they need to be
> > removed.
> >
> > Reclaim
> > ~~~~~~~
> >
> > When reclaim is performed on file-backed folios, it ultimately invokes
> > try_to_unmap_one() via the rmap. If the folio is non-large, then map_pte()
> > will ultimately abort the operation for the guard region mapping. If large,
> > then check_pte() will determine that this is a non-device private
> > entry/device-exclusive entry 'swap' PTE and thus abort the operation in
> > that instance.
> >
> > Therefore, no odd things happen in the instance of reclaim being attempted
> > upon a file-backed guard region.
> >
> > Hole Punching
> > ~~~~~~~~~~~~~
> >
> > This updates the page cache and ultimately invokes unmap_mapping_range(),
> > which explicitly leaves PTE markers in place.
> >
> > Because the establishment of guard regions zapped any existing mappings to
> > file-backed folios, once the guard regions are removed then the
> > hole-punched region will be faulted in as usual and everything will behave
> > as expected.
> >
> > Lorenzo Stoakes (4):
> > mm: allow guard regions in file-backed and read-only mappings
> > selftests/mm: rename guard-pages to guard-regions
> > tools/selftests: expand all guard region tests to file-backed
> > tools/selftests: add file/shmem-backed mapping guard region tests
> >
> > mm/madvise.c | 8 +-
> > tools/testing/selftests/mm/.gitignore | 2 +-
> > tools/testing/selftests/mm/Makefile | 2 +-
> > .../mm/{guard-pages.c => guard-regions.c} | 921 ++++++++++++++++--
> > 4 files changed, 821 insertions(+), 112 deletions(-)
> > rename tools/testing/selftests/mm/{guard-pages.c => guard-regions.c} (58%)
> >
> > --
> > 2.48.1
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Kalesh Singh @ 2025-02-19 8:25 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
David Hildenbrand, linux-mm, linux-kernel, Shuah Khan,
linux-kselftest, linux-api, John Hubbard, Juan Yescas
In-Reply-To: <cover.1739469950.git.lorenzo.stoakes@oracle.com>
On Thu, Feb 13, 2025 at 10:18 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> The guard regions feature was initially implemented to support anonymous
> mappings only, excluding shmem.
>
> This was done such as to introduce the feature carefully and incrementally
> and to be conservative when considering the various caveats and corner
> cases that are applicable to file-backed mappings but not to anonymous
> ones.
>
> Now this feature has landed in 6.13, it is time to revisit this and to
> extend this functionality to file-backed and shmem mappings.
>
> In order to make this maximally useful, and since one may map file-backed
> mappings read-only (for instance ELF images), we also remove the
> restriction on read-only mappings and permit the establishment of guard
> regions in any non-hugetlb, non-mlock()'d mapping.
Hi Lorenzo,
Thank you for your work on this.
Have we thought about how guard regions are represented in /proc/*/[s]maps?
In the field, I've found that many applications read the ranges from
/proc/self/[s]maps to determine what they can access (usually related
to obfuscation techniques). If they don't know of the guard regions it
would cause them to crash; I think that we'll need similar entries to
PROT_NONE (---p) for these, and generally to maintain consistency
between the behavior and what is being said from /proc/*/[s]maps.
-- Kalesh
>
> It is permissible to permit the establishment of guard regions in read-only
> mappings because the guard regions only reduce access to the mapping, and
> when removed simply reinstate the existing attributes of the underlying
> VMA, meaning no access violations can occur.
>
> While the change in kernel code introduced in this series is small, the
> majority of the effort here is spent in extending the testing to assert
> that the feature works correctly across numerous file-backed mapping
> scenarios.
>
> Every single guard region self-test performed against anonymous memory
> (which is relevant and not anon-only) has now been updated to also be
> performed against shmem and a mapping of a file in the working directory.
>
> This confirms that all cases also function correctly for file-backed guard
> regions.
>
> In addition a number of other tests are added for specific file-backed
> mapping scenarios.
>
> There are a number of other concerns that one might have with regard to
> guard regions, addressed below:
>
> Readahead
> ~~~~~~~~~
>
> Readahead is a process through which the page cache is populated on the
> assumption that sequential reads will occur, thus amortising I/O and,
> through a clever use of the PG_readahead folio flag establishing during
> major fault and checked upon minor fault, provides for asynchronous I/O to
> occur as dat is processed, reducing I/O stalls as data is faulted in.
>
> Guard regions do not alter this mechanism which operations at the folio and
> fault level, but do of course prevent the faulting of folios that would
> otherwise be mapped.
>
> In the instance of a major fault prior to a guard region, synchronous
> readahead will occur including populating folios in the page cache which
> the guard regions will, in the case of the mapping in question, prevent
> access to.
>
> In addition, if PG_readahead is placed in a folio that is now inaccessible,
> this will prevent asynchronous readahead from occurring as it would
> otherwise do.
>
> However, there are mechanisms for heuristically resetting this within
> readahead regardless, which will 'recover' correct readahead behaviour.
>
> Readahead presumes sequential data access, the presence of a guard region
> clearly indicates that, at least in the guard region, no such sequential
> access will occur, as it cannot occur there.
>
> So this should have very little impact on any real workload. The far more
> important point is as to whether readahead causes incorrect or
> inappropriate mapping of ranges disallowed by the presence of guard
> regions - this is not the case, as readahead does not 'pre-fault' memory in
> this fashion.
>
> At any rate, any mechanism which would attempt to do so would hit the usual
> page fault paths, which correctly handle PTE markers as with anonymous
> mappings.
>
> Fault-Around
> ~~~~~~~~~~~~
>
> The fault-around logic, in a similar vein to readahead, attempts to improve
> efficiency with regard to file-backed memory mappings, however it differs
> in that it does not try to fetch folios into the page cache that are about
> to be accessed, but rather pre-maps a range of folios around the faulting
> address.
>
> Guard regions making use of PTE markers makes this relatively trivial, as
> this case is already handled - see filemap_map_folio_range() and
> filemap_map_order0_folio() - in both instances, the solution is to simply
> keep the established page table mappings and let the fault handler take
> care of PTE markers, as per the comment:
>
> /*
> * NOTE: If there're PTE markers, we'll leave them to be
> * handled in the specific fault path, and it'll prohibit
> * the fault-around logic.
> */
>
> This works, as establishing guard regions results in page table mappings
> with PTE markers, and clearing them removes them.
>
> Truncation
> ~~~~~~~~~~
>
> File truncation will not eliminate existing guard regions, as the
> truncation operation will ultimately zap the range via
> unmap_mapping_range(), which specifically excludes PTE markers.
>
> Zapping
> ~~~~~~~
>
> Zapping is, as with anonymous mappings, handled by zap_nonpresent_ptes(),
> which specifically deals with guard entries, leaving them intact except in
> instances such as process teardown or munmap() where they need to be
> removed.
>
> Reclaim
> ~~~~~~~
>
> When reclaim is performed on file-backed folios, it ultimately invokes
> try_to_unmap_one() via the rmap. If the folio is non-large, then map_pte()
> will ultimately abort the operation for the guard region mapping. If large,
> then check_pte() will determine that this is a non-device private
> entry/device-exclusive entry 'swap' PTE and thus abort the operation in
> that instance.
>
> Therefore, no odd things happen in the instance of reclaim being attempted
> upon a file-backed guard region.
>
> Hole Punching
> ~~~~~~~~~~~~~
>
> This updates the page cache and ultimately invokes unmap_mapping_range(),
> which explicitly leaves PTE markers in place.
>
> Because the establishment of guard regions zapped any existing mappings to
> file-backed folios, once the guard regions are removed then the
> hole-punched region will be faulted in as usual and everything will behave
> as expected.
>
> Lorenzo Stoakes (4):
> mm: allow guard regions in file-backed and read-only mappings
> selftests/mm: rename guard-pages to guard-regions
> tools/selftests: expand all guard region tests to file-backed
> tools/selftests: add file/shmem-backed mapping guard region tests
>
> mm/madvise.c | 8 +-
> tools/testing/selftests/mm/.gitignore | 2 +-
> tools/testing/selftests/mm/Makefile | 2 +-
> .../mm/{guard-pages.c => guard-regions.c} | 921 ++++++++++++++++--
> 4 files changed, 821 insertions(+), 112 deletions(-)
> rename tools/testing/selftests/mm/{guard-pages.c => guard-regions.c} (58%)
>
> --
> 2.48.1
^ permalink raw reply
* Re: [PATCH] ntsync: Set the permissions to be 0666
From: Elizabeth Figura @ 2025-02-18 23:57 UTC (permalink / raw)
To: dri-devel, Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet,
Shuah Khan, Mike Lothian
Cc: Mike Lothian, linux-kernel, linux-api, wine-devel,
André Almeida, Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra,
Andy Lutomirski, Randy Dunlap, Ingo Molnar, Will Deacon,
Waiman Long, Boqun Feng
In-Reply-To: <20250214122759.2629-2-mike@fireburn.co.uk>
On Friday, 14 February 2025 06:28:00 CST Mike Lothian wrote:
> This allows ntsync to be usuable by non-root processes out of the box
>
> Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
> ---
> drivers/misc/ntsync.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> index 055395cde42b..586b86243e1d 100644
> --- a/drivers/misc/ntsync.c
> +++ b/drivers/misc/ntsync.c
> @@ -1208,6 +1208,7 @@ static struct miscdevice ntsync_misc = {
> .minor = MISC_DYNAMIC_MINOR,
> .name = NTSYNC_NAME,
> .fops = &ntsync_fops,
> + .mode = 0666, // Setting file permissions to 0666
> };
>
> module_misc_device(ntsync_misc);
>
Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com>
--
The comment seems rather redundant, but otherwise this is correct and prudent.
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-18 17:31 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <c0e079bd-a840-4240-93ae-0ee2755d425a@lucifer.local>
>> See mlock2();
>>
>> SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
>> {
>> vm_flags_t vm_flags = VM_LOCKED;
>>
>> if (flags & ~MLOCK_ONFAULT)
>> return -EINVAL;
>>
>> if (flags & MLOCK_ONFAULT)
>> vm_flags |= VM_LOCKONFAULT;
>>
>> return do_mlock(start, len, vm_flags);
>> }
>>
>>
>> VM_LOCKONFAULT always as VM_LOCKED set as well.
>
> OK cool, that makes sense.
>
> As with much kernel stuff, I knew this in the past. Then I forgot. Then I knew
> again, then... :P if only somebody would write it down in a book...
And if there would be such a book in the makings, I would consider
hurrying up and preordering it ;)
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-18 17:28 UTC (permalink / raw)
To: David Hildenbrand
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <b30a6306-d62b-4515-add8-4550d044501c@redhat.com>
On Tue, Feb 18, 2025 at 06:25:35PM +0100, David Hildenbrand wrote:
> > >
> > > QEMU, for example, will issue an mlockall(MCL_CURRENT | MCL_FUTURE); when
> > > requested to then exit(); if it fails.
> >
> > Hm under what circumstances? I use qemu extensively to test this stuff with
> > no issues. Unless you mean it's using it in the 'host' code somehow.
>
>
> -overcommit mem-lock=on
>
> or (legacy)
>
> -realtime mlock=on
>
> I think.
>
> [...]
Thanks
>
> > > >
> > > > It fails because it tries to 'touch' the memory, but 'touching' guard
> > > > region memory causes a segfault. This kind of breaks the idea of
> > > > mlock()'ing guard regions.
> > > >
> > > > I think adding workarounds to make this possible in any way is not really
> > > > worth it (and would probably be pretty gross).
> > > >
> > > > We already document that 'mlock()ing lightweight guard regions will fail'
> > > > as per man page so this is all in line with that.
> > >
> > > Right, and I claim that supporting VM_LOCKONFAULT might likely be as easy as
> > > allowing install/remove of guard regions when that flag is set.
> >
> > We already allow this flag! VM_LOCKED and VM_HUGETLB are the only flags we
> > disallow.
>
>
> See mlock2();
>
> SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
> {
> vm_flags_t vm_flags = VM_LOCKED;
>
> if (flags & ~MLOCK_ONFAULT)
> return -EINVAL;
>
> if (flags & MLOCK_ONFAULT)
> vm_flags |= VM_LOCKONFAULT;
>
> return do_mlock(start, len, vm_flags);
> }
>
>
> VM_LOCKONFAULT always as VM_LOCKED set as well.
OK cool, that makes sense.
As with much kernel stuff, I knew this in the past. Then I forgot. Then I knew
again, then... :P if only somebody would write it down in a book...
Yeah then that makes sense to check explicitly for (VM_LOCKED | VM_LOCKONFAULT)
in any MADV_GUARD_INSTALL_LOCKED variant as obviously this would be passively
excluded right now.
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-18 17:25 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <6eb33b5d-3040-4637-b627-48f8f78e4e28@lucifer.local>
>>
>> QEMU, for example, will issue an mlockall(MCL_CURRENT | MCL_FUTURE); when
>> requested to then exit(); if it fails.
>
> Hm under what circumstances? I use qemu extensively to test this stuff with
> no issues. Unless you mean it's using it in the 'host' code somehow.
-overcommit mem-lock=on
or (legacy)
-realtime mlock=on
I think.
[...]
>>>
>>> It fails because it tries to 'touch' the memory, but 'touching' guard
>>> region memory causes a segfault. This kind of breaks the idea of
>>> mlock()'ing guard regions.
>>>
>>> I think adding workarounds to make this possible in any way is not really
>>> worth it (and would probably be pretty gross).
>>>
>>> We already document that 'mlock()ing lightweight guard regions will fail'
>>> as per man page so this is all in line with that.
>>
>> Right, and I claim that supporting VM_LOCKONFAULT might likely be as easy as
>> allowing install/remove of guard regions when that flag is set.
>
> We already allow this flag! VM_LOCKED and VM_HUGETLB are the only flags we
> disallow.
See mlock2();
SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
{
vm_flags_t vm_flags = VM_LOCKED;
if (flags & ~MLOCK_ONFAULT)
return -EINVAL;
if (flags & MLOCK_ONFAULT)
vm_flags |= VM_LOCKONFAULT;
return do_mlock(start, len, vm_flags);
}
VM_LOCKONFAULT always as VM_LOCKED set as well.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-18 17:20 UTC (permalink / raw)
To: David Hildenbrand
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <a49d277e-128c-4853-bdeb-3a94134acbf6@redhat.com>
On Tue, Feb 18, 2025 at 06:14:00PM +0100, David Hildenbrand wrote:
> On 18.02.25 17:43, Lorenzo Stoakes wrote:
> > On Tue, Feb 18, 2025 at 04:20:18PM +0100, David Hildenbrand wrote:
> > > > Right yeah that'd be super weird. And I don't want to add that logic.
> > > >
> > > > > Also not sure what happens if one does an mlock()/mlockall() after
> > > > > already installing PTE markers.
> > > >
> > > > The existing logic already handles non-present cases by skipping them, in
> > > > mlock_pte_range():
> > > >
> > > > for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
> > > > ptent = ptep_get(pte);
> > > > if (!pte_present(ptent))
> > > > continue;
> > > >
> > > > ...
> > > > }
> > >
> > > I *think* that code only updates already-mapped folios, to properly call
> > > mlock_folio()/munlock_folio().
> >
> > Guard regions _are_ 'already mapped' :) so it leaves them in place.
>
> "mapped folios" -- there is no folio mapped. Yes, the VMA is in place.
We're engaging in a moot discussion on this I think but I mean it appears
to operate by walking page tables if they are populated, which they will be
for guard regions, but when it finds it's non-present it will skip.
This amounts to the same thing as not doing anything, obviously.
>
> >
> > do_mlock() -> apply_vma_lock_flags() -> mlock_fixup() -> mlock_vma_pages_range()
> > implies this will be invoked.
>
> Yes, to process any already mapped folios, to then continue population
> later.
>
> >
> > >
> > > It is not the code that populates pages on mlock()/mlockall(). I think all
> > > that goes via mm_populate()/__mm_populate(), where "ordinary GUP" should
> > > apply.
> >
> > OK I want to correct what I said earlier.
> >
> > Installing a guard region then attempting mlock() will result in an error. The
> > populate will -EFAULT and stop at the guard region, which causes mlock() to
> > error out.
>
> Right, that's my expectation.
OK good!
>
> >
> > This is a partial failure, so the VMA is split and has VM_LOCKED applied, but
> > the populate halts at the guard region.
> >
> > This is ok as per previous discussion on aggregate operation failure, there can
> > be no expectation of 'unwinding' of partially successful operations that form
> > part of a requested aggregate one.
> >
> > However, given there's stuff to clean up, and on error a user _may_ wish to then
> > remove guard regions and try again, I guess there's no harm in keeping the code
> > as it is where we allow MADV_GUARD_REMOVE even if VM_LOCKED is in place.
>
> Likely yes; it's all weird code.
>
> >
> > >
> > > See populate_vma_page_range(), especially also the VM_LOCKONFAULT handling.
> >
> > Yeah that code is horrible, you just reminded me of it... 'rightly or wrongly'
> > yeah wrongly, very wrongly...
> >
> > >
> > > >
> > > > Which covers off guard regions. Removing the guard regions after this will
> > > > leave you in a weird situation where these entries will be zapped... maybe
> > > > we need a patch to make MADV_GUARD_REMOVE check VM_LOCKED and in this case
> > > > also populate?
> > >
> > > Maybe? Or we say that it behaves like MADV_DONTNEED_LOCKED.
> >
> > See above, no we should not :P this is only good for cleanup after mlock()
> > failure, although no sane program should really be trying to do this, a sane
> > program would give up here (and it's a _programmatic error_ to try to mlock() a
> > range with guard regions).
> >>>> Somme apps use mlockall(), and it might be nice to just be able to use
> guard
> > > pages as if "Nothing happened".
> >
> > Sadly I think not given above :P
>
> QEMU, for example, will issue an mlockall(MCL_CURRENT | MCL_FUTURE); when
> requested to then exit(); if it fails.
Hm under what circumstances? I use qemu extensively to test this stuff with
no issues. Unless you mean it's using it in the 'host' code somehow.
>
> Assume glibc or any lib uses it, QEMU would have no real way of figuring
> that out or instructing offending libraries to disabled that, at least for
> now ...
>
> ... turning RT VMs less usable if any library uses guard regions. :(
>
This seems really stupid, to be honest. Unfortunately there's no way around
this, if software does stupid things then they get stupid prizes. There are
other ways mlock() and faulting in can fail too.
> There is upcoming support for MCL_ONFAULT in QEMU [1] (see below).
Good.
>
> [1] https://lkml.kernel.org/r/20250212173823.214429-3-peterx@redhat.com
>
> >
> > >
> > > E.g., QEMU has the option to use mlockall().
> > >
> > > >
> > > > Then again we're currently asymmetric as you can add them _before_
> > > > mlock()'ing...
> > >
> > > Right.
> > >
> > > --
> > > Cheers,
> > >
> > > David / dhildenb
> > >
> >
> > I think the _LOCKED idea is therefore kaput, because it just won't work
> > properly because populating guard regions fails.
>
> Right, I think basic VM_LOCKED is out of the picture. VM_LOCKONFAULT might
> be interesting, because we are skipping the population stage.
>
> >
> > It fails because it tries to 'touch' the memory, but 'touching' guard
> > region memory causes a segfault. This kind of breaks the idea of
> > mlock()'ing guard regions.
> >
> > I think adding workarounds to make this possible in any way is not really
> > worth it (and would probably be pretty gross).
> >
> > We already document that 'mlock()ing lightweight guard regions will fail'
> > as per man page so this is all in line with that.
>
> Right, and I claim that supporting VM_LOCKONFAULT might likely be as easy as
> allowing install/remove of guard regions when that flag is set.
We already allow this flag! VM_LOCKED and VM_HUGETLB are the only flags we
disallow.
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-18 17:14 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <62c0ba1c-7724-4033-b1de-d62a59751ca5@lucifer.local>
On 18.02.25 17:43, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 04:20:18PM +0100, David Hildenbrand wrote:
>>> Right yeah that'd be super weird. And I don't want to add that logic.
>>>
>>>> Also not sure what happens if one does an mlock()/mlockall() after
>>>> already installing PTE markers.
>>>
>>> The existing logic already handles non-present cases by skipping them, in
>>> mlock_pte_range():
>>>
>>> for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
>>> ptent = ptep_get(pte);
>>> if (!pte_present(ptent))
>>> continue;
>>>
>>> ...
>>> }
>>
>> I *think* that code only updates already-mapped folios, to properly call
>> mlock_folio()/munlock_folio().
>
> Guard regions _are_ 'already mapped' :) so it leaves them in place.
"mapped folios" -- there is no folio mapped. Yes, the VMA is in place.
>
> do_mlock() -> apply_vma_lock_flags() -> mlock_fixup() -> mlock_vma_pages_range()
> implies this will be invoked.
Yes, to process any already mapped folios, to then continue population
later.
>
>>
>> It is not the code that populates pages on mlock()/mlockall(). I think all
>> that goes via mm_populate()/__mm_populate(), where "ordinary GUP" should
>> apply.
>
> OK I want to correct what I said earlier.
>
> Installing a guard region then attempting mlock() will result in an error. The
> populate will -EFAULT and stop at the guard region, which causes mlock() to
> error out.
Right, that's my expectation.
>
> This is a partial failure, so the VMA is split and has VM_LOCKED applied, but
> the populate halts at the guard region.
>
> This is ok as per previous discussion on aggregate operation failure, there can
> be no expectation of 'unwinding' of partially successful operations that form
> part of a requested aggregate one.
>
> However, given there's stuff to clean up, and on error a user _may_ wish to then
> remove guard regions and try again, I guess there's no harm in keeping the code
> as it is where we allow MADV_GUARD_REMOVE even if VM_LOCKED is in place.
Likely yes; it's all weird code.
>
>>
>> See populate_vma_page_range(), especially also the VM_LOCKONFAULT handling.
>
> Yeah that code is horrible, you just reminded me of it... 'rightly or wrongly'
> yeah wrongly, very wrongly...
>
>>
>>>
>>> Which covers off guard regions. Removing the guard regions after this will
>>> leave you in a weird situation where these entries will be zapped... maybe
>>> we need a patch to make MADV_GUARD_REMOVE check VM_LOCKED and in this case
>>> also populate?
>>
>> Maybe? Or we say that it behaves like MADV_DONTNEED_LOCKED.
>
> See above, no we should not :P this is only good for cleanup after mlock()
> failure, although no sane program should really be trying to do this, a sane
> program would give up here (and it's a _programmatic error_ to try to mlock() a
> range with guard regions).
>>>> Somme apps use mlockall(), and it might be nice to just be able to
use guard
>> pages as if "Nothing happened".
>
> Sadly I think not given above :P
QEMU, for example, will issue an mlockall(MCL_CURRENT | MCL_FUTURE);
when requested to then exit(); if it fails.
Assume glibc or any lib uses it, QEMU would have no real way of figuring
that out or instructing offending libraries to disabled that, at least
for now ...
... turning RT VMs less usable if any library uses guard regions. :(
There is upcoming support for MCL_ONFAULT in QEMU [1] (see below).
[1] https://lkml.kernel.org/r/20250212173823.214429-3-peterx@redhat.com
>
>>
>> E.g., QEMU has the option to use mlockall().
>>
>>>
>>> Then again we're currently asymmetric as you can add them _before_
>>> mlock()'ing...
>>
>> Right.
>>
>> --
>> Cheers,
>>
>> David / dhildenb
>>
>
> I think the _LOCKED idea is therefore kaput, because it just won't work
> properly because populating guard regions fails.
Right, I think basic VM_LOCKED is out of the picture. VM_LOCKONFAULT
might be interesting, because we are skipping the population stage.
>
> It fails because it tries to 'touch' the memory, but 'touching' guard
> region memory causes a segfault. This kind of breaks the idea of
> mlock()'ing guard regions.
>
> I think adding workarounds to make this possible in any way is not really
> worth it (and would probably be pretty gross).
>
> We already document that 'mlock()ing lightweight guard regions will fail'
> as per man page so this is all in line with that.
Right, and I claim that supporting VM_LOCKONFAULT might likely be as
easy as allowing install/remove of guard regions when that flag is set.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: Lorenzo Stoakes @ 2025-02-18 17:04 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <d3da8c0e-bb9f-4832-afbe-ea862084339e@redhat.com>
On Tue, Feb 18, 2025 at 06:00:36PM +0100, David Hildenbrand wrote:
> On 18.02.25 17:49, Lorenzo Stoakes wrote:
> > On Tue, Feb 18, 2025 at 05:27:24PM +0100, David Hildenbrand wrote:
> > > On 18.02.25 17:21, Lorenzo Stoakes wrote:
> > > > On Tue, Feb 18, 2025 at 05:17:20PM +0100, David Hildenbrand wrote:
> > > > > On 18.02.25 17:12, Lorenzo Stoakes wrote:
> > > > > > On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
> > > > > > > On 13.02.25 19:17, Lorenzo Stoakes wrote:
> > > > > > > > There is no reason to disallow guard regions in file-backed mappings -
> > > > > > > > readahead and fault-around both function correctly in the presence of PTE
> > > > > > > > markers, equally other operations relating to memory-mapped files function
> > > > > > > > correctly.
> > > > > > > >
> > > > > > > > Additionally, read-only mappings if introducing guard-regions, only
> > > > > > > > restrict the mapping further, which means there is no violation of any
> > > > > > > > access rights by permitting this to be so.
> > > > > > > >
> > > > > > > > Removing this restriction allows for read-only mapped files (such as
> > > > > > > > executable files) correctly which would otherwise not be permitted.
> > > > > > > >
> > > > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > > > ---
> > > > > > > > mm/madvise.c | 8 +-------
> > > > > > > > 1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > > > > > > index 6ecead476a80..e01e93e179a8 100644
> > > > > > > > --- a/mm/madvise.c
> > > > > > > > +++ b/mm/madvise.c
> > > > > > > > @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
> > > > > > > > if (!allow_locked)
> > > > > > > > disallowed |= VM_LOCKED;
> > > > > > > > - if (!vma_is_anonymous(vma))
> > > > > > > > - return false;
> > > > > > > > -
> > > > > > > > - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
> > > > > > > > - return false;
> > > > > > > > -
> > > > > > > > - return true;
> > > > > > > > + return !(vma->vm_flags & disallowed);
> > > > > > > > }
> > > > > > > > static bool is_guard_pte_marker(pte_t ptent)
> > > > > > >
> > > > > > > Acked-by: David Hildenbrand <david@redhat.com>
> > > > > >
> > > > > > Thanks!
> > > > > >
> > > > > > >
> > > > > > > I assume these markers cannot completely prevent us from allocating
> > > > > > > pages/folios for these underlying file/pageache ranges of these markers in
> > > > > > > case of shmem during page faults, right?
> > > > > >
> > > > > > If the markers are in place, then page faulting will result in a
> > > > > > segfault. If we faulted in a shmem page then installed markers (which would
> > > > > > zap the range), then the page cache will be populated, but obviously
> > > > > > subject to standard reclaim.
> > > > >
> > > > > Well, yes, (a) if there is swap and (b), if the noswap option was not
> > > > > specified for tmpfs.
> > > > >
> > > >
> > > > Right, yeah if you don't have it set up such that dropping a reference to the
> > > > folio doesn't drop the page altogether.
> > > >
> > > > I think this matches expectation though in that you'd get the same results from
> > > > an MADV_DONTNEED followed by faulting the page again.
> > >
> > > It might make sense to document that: installing a guard behaves just like
> > > MADV_DONTNEED; in case of a file, that means that the pagecache is left
> > > untouched.
> >
> > More docs noooo! :P I will update the man pages when this is more obviously
> > heading for landing in 6.15 accordingly.
> >
> > Current man page documentation on this is:
> >
> > 'If the region maps memory pages those mappings will be replaced as part of
> > the operation'
> >
> > I think something like:
> >
> > 'If the region maps pages those mappings will be replaced as part of the
> > operation. When guard regions are removed via MADV_GUARD_REMOVE, faulting
> > in the page will behave as if that region had MADV_DONTNEED applied to it,
> > that is anonymous ranges will be backed by newly allocated zeroed pages and
> > file-backed ranges will be backed by the underlying file pages.'
> >
> > Probably something less wordy than this...
>
> Yeah, but sounds good to me.
>
> >
> > >
> > > >
> > > > > Okay, so installing a guard entry might require punshing a hole to get rid
> > > > > of any already-existing memory. But readahead (below) might mess it up.
> > > >
> > > > Only if you are so concerned about avoiding the page cache being populated there
> > > > that you want to do this :)
> > > >
> > > > Readahead I think will not readahead into a holepunched region as the hole
> > > > punching extends to the fs layer _I believe_ I have not checked the code for
> > > > this, but I believe it actually changes the underlying file too right to say
> > > > 'this part of the file is empty'?
> > >
> > > Well, we are talking about shmem here ... not your ordinary fs backed by an
> > > actual file :)
> >
> > I am talking about both, I multitask ;)
>
> For !shmem, we should indeed not be messing with a sparse file structure.
Ah right I get you, sorry missed your point here. Yeah MADV_REMOVE for shmem
will just explicitly drop the pages which doesn't have any underlying file to
impact as with actually file-backed memory.
>
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: David Hildenbrand @ 2025-02-18 17:00 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <195a2b04-2968-4a51-9308-855da9fc12ed@lucifer.local>
On 18.02.25 17:49, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 05:27:24PM +0100, David Hildenbrand wrote:
>> On 18.02.25 17:21, Lorenzo Stoakes wrote:
>>> On Tue, Feb 18, 2025 at 05:17:20PM +0100, David Hildenbrand wrote:
>>>> On 18.02.25 17:12, Lorenzo Stoakes wrote:
>>>>> On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
>>>>>> On 13.02.25 19:17, Lorenzo Stoakes wrote:
>>>>>>> There is no reason to disallow guard regions in file-backed mappings -
>>>>>>> readahead and fault-around both function correctly in the presence of PTE
>>>>>>> markers, equally other operations relating to memory-mapped files function
>>>>>>> correctly.
>>>>>>>
>>>>>>> Additionally, read-only mappings if introducing guard-regions, only
>>>>>>> restrict the mapping further, which means there is no violation of any
>>>>>>> access rights by permitting this to be so.
>>>>>>>
>>>>>>> Removing this restriction allows for read-only mapped files (such as
>>>>>>> executable files) correctly which would otherwise not be permitted.
>>>>>>>
>>>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>>> ---
>>>>>>> mm/madvise.c | 8 +-------
>>>>>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/mm/madvise.c b/mm/madvise.c
>>>>>>> index 6ecead476a80..e01e93e179a8 100644
>>>>>>> --- a/mm/madvise.c
>>>>>>> +++ b/mm/madvise.c
>>>>>>> @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
>>>>>>> if (!allow_locked)
>>>>>>> disallowed |= VM_LOCKED;
>>>>>>> - if (!vma_is_anonymous(vma))
>>>>>>> - return false;
>>>>>>> -
>>>>>>> - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
>>>>>>> - return false;
>>>>>>> -
>>>>>>> - return true;
>>>>>>> + return !(vma->vm_flags & disallowed);
>>>>>>> }
>>>>>>> static bool is_guard_pte_marker(pte_t ptent)
>>>>>>
>>>>>> Acked-by: David Hildenbrand <david@redhat.com>
>>>>>
>>>>> Thanks!
>>>>>
>>>>>>
>>>>>> I assume these markers cannot completely prevent us from allocating
>>>>>> pages/folios for these underlying file/pageache ranges of these markers in
>>>>>> case of shmem during page faults, right?
>>>>>
>>>>> If the markers are in place, then page faulting will result in a
>>>>> segfault. If we faulted in a shmem page then installed markers (which would
>>>>> zap the range), then the page cache will be populated, but obviously
>>>>> subject to standard reclaim.
>>>>
>>>> Well, yes, (a) if there is swap and (b), if the noswap option was not
>>>> specified for tmpfs.
>>>>
>>>
>>> Right, yeah if you don't have it set up such that dropping a reference to the
>>> folio doesn't drop the page altogether.
>>>
>>> I think this matches expectation though in that you'd get the same results from
>>> an MADV_DONTNEED followed by faulting the page again.
>>
>> It might make sense to document that: installing a guard behaves just like
>> MADV_DONTNEED; in case of a file, that means that the pagecache is left
>> untouched.
>
> More docs noooo! :P I will update the man pages when this is more obviously
> heading for landing in 6.15 accordingly.
>
> Current man page documentation on this is:
>
> 'If the region maps memory pages those mappings will be replaced as part of
> the operation'
>
> I think something like:
>
> 'If the region maps pages those mappings will be replaced as part of the
> operation. When guard regions are removed via MADV_GUARD_REMOVE, faulting
> in the page will behave as if that region had MADV_DONTNEED applied to it,
> that is anonymous ranges will be backed by newly allocated zeroed pages and
> file-backed ranges will be backed by the underlying file pages.'
>
> Probably something less wordy than this...
Yeah, but sounds good to me.
>
>>
>>>
>>>> Okay, so installing a guard entry might require punshing a hole to get rid
>>>> of any already-existing memory. But readahead (below) might mess it up.
>>>
>>> Only if you are so concerned about avoiding the page cache being populated there
>>> that you want to do this :)
>>>
>>> Readahead I think will not readahead into a holepunched region as the hole
>>> punching extends to the fs layer _I believe_ I have not checked the code for
>>> this, but I believe it actually changes the underlying file too right to say
>>> 'this part of the file is empty'?
>>
>> Well, we are talking about shmem here ... not your ordinary fs backed by an
>> actual file :)
>
> I am talking about both, I multitask ;)
For !shmem, we should indeed not be messing with a sparse file structure.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: Lorenzo Stoakes @ 2025-02-18 16:49 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <de79890e-86b5-4f43-8a25-1e50c3b1daea@redhat.com>
On Tue, Feb 18, 2025 at 05:27:24PM +0100, David Hildenbrand wrote:
> On 18.02.25 17:21, Lorenzo Stoakes wrote:
> > On Tue, Feb 18, 2025 at 05:17:20PM +0100, David Hildenbrand wrote:
> > > On 18.02.25 17:12, Lorenzo Stoakes wrote:
> > > > On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
> > > > > On 13.02.25 19:17, Lorenzo Stoakes wrote:
> > > > > > There is no reason to disallow guard regions in file-backed mappings -
> > > > > > readahead and fault-around both function correctly in the presence of PTE
> > > > > > markers, equally other operations relating to memory-mapped files function
> > > > > > correctly.
> > > > > >
> > > > > > Additionally, read-only mappings if introducing guard-regions, only
> > > > > > restrict the mapping further, which means there is no violation of any
> > > > > > access rights by permitting this to be so.
> > > > > >
> > > > > > Removing this restriction allows for read-only mapped files (such as
> > > > > > executable files) correctly which would otherwise not be permitted.
> > > > > >
> > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > ---
> > > > > > mm/madvise.c | 8 +-------
> > > > > > 1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > > > > index 6ecead476a80..e01e93e179a8 100644
> > > > > > --- a/mm/madvise.c
> > > > > > +++ b/mm/madvise.c
> > > > > > @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
> > > > > > if (!allow_locked)
> > > > > > disallowed |= VM_LOCKED;
> > > > > > - if (!vma_is_anonymous(vma))
> > > > > > - return false;
> > > > > > -
> > > > > > - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
> > > > > > - return false;
> > > > > > -
> > > > > > - return true;
> > > > > > + return !(vma->vm_flags & disallowed);
> > > > > > }
> > > > > > static bool is_guard_pte_marker(pte_t ptent)
> > > > >
> > > > > Acked-by: David Hildenbrand <david@redhat.com>
> > > >
> > > > Thanks!
> > > >
> > > > >
> > > > > I assume these markers cannot completely prevent us from allocating
> > > > > pages/folios for these underlying file/pageache ranges of these markers in
> > > > > case of shmem during page faults, right?
> > > >
> > > > If the markers are in place, then page faulting will result in a
> > > > segfault. If we faulted in a shmem page then installed markers (which would
> > > > zap the range), then the page cache will be populated, but obviously
> > > > subject to standard reclaim.
> > >
> > > Well, yes, (a) if there is swap and (b), if the noswap option was not
> > > specified for tmpfs.
> > >
> >
> > Right, yeah if you don't have it set up such that dropping a reference to the
> > folio doesn't drop the page altogether.
> >
> > I think this matches expectation though in that you'd get the same results from
> > an MADV_DONTNEED followed by faulting the page again.
>
> It might make sense to document that: installing a guard behaves just like
> MADV_DONTNEED; in case of a file, that means that the pagecache is left
> untouched.
More docs noooo! :P I will update the man pages when this is more obviously
heading for landing in 6.15 accordingly.
Current man page documentation on this is:
'If the region maps memory pages those mappings will be replaced as part of
the operation'
I think something like:
'If the region maps pages those mappings will be replaced as part of the
operation. When guard regions are removed via MADV_GUARD_REMOVE, faulting
in the page will behave as if that region had MADV_DONTNEED applied to it,
that is anonymous ranges will be backed by newly allocated zeroed pages and
file-backed ranges will be backed by the underlying file pages.'
Probably something less wordy than this...
>
> >
> > > Okay, so installing a guard entry might require punshing a hole to get rid
> > > of any already-existing memory. But readahead (below) might mess it up.
> >
> > Only if you are so concerned about avoiding the page cache being populated there
> > that you want to do this :)
> >
> > Readahead I think will not readahead into a holepunched region as the hole
> > punching extends to the fs layer _I believe_ I have not checked the code for
> > this, but I believe it actually changes the underlying file too right to say
> > 'this part of the file is empty'?
>
> Well, we are talking about shmem here ... not your ordinary fs backed by an
> actual file :)
I am talking about both, I multitask ;)
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-18 16:43 UTC (permalink / raw)
To: David Hildenbrand
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <0db666da-10d3-4b2c-9b33-781fb265343f@redhat.com>
On Tue, Feb 18, 2025 at 04:20:18PM +0100, David Hildenbrand wrote:
> > Right yeah that'd be super weird. And I don't want to add that logic.
> >
> > > Also not sure what happens if one does an mlock()/mlockall() after
> > > already installing PTE markers.
> >
> > The existing logic already handles non-present cases by skipping them, in
> > mlock_pte_range():
> >
> > for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
> > ptent = ptep_get(pte);
> > if (!pte_present(ptent))
> > continue;
> >
> > ...
> > }
>
> I *think* that code only updates already-mapped folios, to properly call
> mlock_folio()/munlock_folio().
Guard regions _are_ 'already mapped' :) so it leaves them in place.
do_mlock() -> apply_vma_lock_flags() -> mlock_fixup() -> mlock_vma_pages_range()
implies this will be invoked.
>
> It is not the code that populates pages on mlock()/mlockall(). I think all
> that goes via mm_populate()/__mm_populate(), where "ordinary GUP" should
> apply.
OK I want to correct what I said earlier.
Installing a guard region then attempting mlock() will result in an error. The
populate will -EFAULT and stop at the guard region, which causes mlock() to
error out.
This is a partial failure, so the VMA is split and has VM_LOCKED applied, but
the populate halts at the guard region.
This is ok as per previous discussion on aggregate operation failure, there can
be no expectation of 'unwinding' of partially successful operations that form
part of a requested aggregate one.
However, given there's stuff to clean up, and on error a user _may_ wish to then
remove guard regions and try again, I guess there's no harm in keeping the code
as it is where we allow MADV_GUARD_REMOVE even if VM_LOCKED is in place.
>
> See populate_vma_page_range(), especially also the VM_LOCKONFAULT handling.
Yeah that code is horrible, you just reminded me of it... 'rightly or wrongly'
yeah wrongly, very wrongly...
>
> >
> > Which covers off guard regions. Removing the guard regions after this will
> > leave you in a weird situation where these entries will be zapped... maybe
> > we need a patch to make MADV_GUARD_REMOVE check VM_LOCKED and in this case
> > also populate?
>
> Maybe? Or we say that it behaves like MADV_DONTNEED_LOCKED.
See above, no we should not :P this is only good for cleanup after mlock()
failure, although no sane program should really be trying to do this, a sane
program would give up here (and it's a _programmatic error_ to try to mlock() a
range with guard regions).
>
> >
> > Actually I think the simpler option is to just disallow MADV_GUARD_REMOVE
> > if you since locked the range? The code currently allows this on the
> > proviso that 'you aren't zapping locked mappings' but leaves the VMA in a
> > state such that some entries would not be locked.
> >
> > It'd be pretty weird to lock guard regions like this.
> >
> > Having said all that, given what you say below, maybe it's not an issue
> > after all?...
> >
> > >
> > > __mm_populate() would skip whole VMAs in case populate_vma_page_range()
> > > fails. And I would assume populate_vma_page_range() fails on the first
> > > guard when it triggers a page fault.
> > >
> > > OTOH, supporting the mlock-on-fault thingy should be easy. That's precisely where
> > > MADV_DONTNEED_LOCKED originates from:
> > >
> > > commit 9457056ac426e5ed0671356509c8dcce69f8dee0
> > > Author: Johannes Weiner <hannes@cmpxchg.org>
> > > Date: Thu Mar 24 18:14:12 2022 -0700
> > >
> > > mm: madvise: MADV_DONTNEED_LOCKED
> > > MADV_DONTNEED historically rejects mlocked ranges, but with MLOCK_ONFAULT
> > > and MCL_ONFAULT allowing to mlock without populating, there are valid use
> > > cases for depopulating locked ranges as well.
> >
> > ...Hm this seems to imply the current guard remove stuff isn't quite so
> > bad, so maybe the assumption that VM_LOCKED implies 'everything is
> > populated' isn't quite as stringent then.
>
> Right, with MCL_ONFAULT at least. Without MCL_ONFAULT, the assumption is
> that everything is populated (unless, apparently one uses
> MADV_DONTNEED_LOCKED or population failed, maybe).
>
> VM_LOCKONFAULT seems to be the sane case. I wonder why MADV_DONTNEED_LOCKED
> didn't explicitly check for that one ... maybe there is a history to that.
Yeah weird.
>
> >
> > The restriction is as simple as:
> >
> > if (behavior != MADV_DONTNEED_LOCKED)
> > forbidden |= VM_LOCKED;
> >
> > >
> > >
> > > Adding support for that would be indeed nice.
> >
> > I mean it's sort of maybe understandable why you'd want to MADV_DONTNEED
> > locked ranges, but I really don't understand why you'd want to add guard
> > regions to mlock()'ed regions?
>
> Somme apps use mlockall(), and it might be nice to just be able to use guard
> pages as if "Nothing happened".
Sadly I think not given above :P
>
> E.g., QEMU has the option to use mlockall().
>
> >
> > Then again we're currently asymmetric as you can add them _before_
> > mlock()'ing...
>
> Right.
>
> --
> Cheers,
>
> David / dhildenb
>
I think the _LOCKED idea is therefore kaput, because it just won't work
properly because populating guard regions fails.
It fails because it tries to 'touch' the memory, but 'touching' guard
region memory causes a segfault. This kind of breaks the idea of
mlock()'ing guard regions.
I think adding workarounds to make this possible in any way is not really
worth it (and would probably be pretty gross).
We already document that 'mlock()ing lightweight guard regions will fail'
as per man page so this is all in line with that.
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: David Hildenbrand @ 2025-02-18 16:27 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <d219ab1b-9fb1-45de-aa65-b6071d049dd1@lucifer.local>
On 18.02.25 17:21, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 05:17:20PM +0100, David Hildenbrand wrote:
>> On 18.02.25 17:12, Lorenzo Stoakes wrote:
>>> On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
>>>> On 13.02.25 19:17, Lorenzo Stoakes wrote:
>>>>> There is no reason to disallow guard regions in file-backed mappings -
>>>>> readahead and fault-around both function correctly in the presence of PTE
>>>>> markers, equally other operations relating to memory-mapped files function
>>>>> correctly.
>>>>>
>>>>> Additionally, read-only mappings if introducing guard-regions, only
>>>>> restrict the mapping further, which means there is no violation of any
>>>>> access rights by permitting this to be so.
>>>>>
>>>>> Removing this restriction allows for read-only mapped files (such as
>>>>> executable files) correctly which would otherwise not be permitted.
>>>>>
>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> ---
>>>>> mm/madvise.c | 8 +-------
>>>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/mm/madvise.c b/mm/madvise.c
>>>>> index 6ecead476a80..e01e93e179a8 100644
>>>>> --- a/mm/madvise.c
>>>>> +++ b/mm/madvise.c
>>>>> @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
>>>>> if (!allow_locked)
>>>>> disallowed |= VM_LOCKED;
>>>>> - if (!vma_is_anonymous(vma))
>>>>> - return false;
>>>>> -
>>>>> - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
>>>>> - return false;
>>>>> -
>>>>> - return true;
>>>>> + return !(vma->vm_flags & disallowed);
>>>>> }
>>>>> static bool is_guard_pte_marker(pte_t ptent)
>>>>
>>>> Acked-by: David Hildenbrand <david@redhat.com>
>>>
>>> Thanks!
>>>
>>>>
>>>> I assume these markers cannot completely prevent us from allocating
>>>> pages/folios for these underlying file/pageache ranges of these markers in
>>>> case of shmem during page faults, right?
>>>
>>> If the markers are in place, then page faulting will result in a
>>> segfault. If we faulted in a shmem page then installed markers (which would
>>> zap the range), then the page cache will be populated, but obviously
>>> subject to standard reclaim.
>>
>> Well, yes, (a) if there is swap and (b), if the noswap option was not
>> specified for tmpfs.
>>
>
> Right, yeah if you don't have it set up such that dropping a reference to the
> folio doesn't drop the page altogether.
>
> I think this matches expectation though in that you'd get the same results from
> an MADV_DONTNEED followed by faulting the page again.
It might make sense to document that: installing a guard behaves just
like MADV_DONTNEED; in case of a file, that means that the pagecache is
left untouched.
>
>> Okay, so installing a guard entry might require punshing a hole to get rid
>> of any already-existing memory. But readahead (below) might mess it up.
>
> Only if you are so concerned about avoiding the page cache being populated there
> that you want to do this :)
>
> Readahead I think will not readahead into a holepunched region as the hole
> punching extends to the fs layer _I believe_ I have not checked the code for
> this, but I believe it actually changes the underlying file too right to say
> 'this part of the file is empty'?
Well, we are talking about shmem here ... not your ordinary fs backed by
an actual file :)
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: Lorenzo Stoakes @ 2025-02-18 16:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <90a00957-02b5-440a-9168-de93c760fea7@redhat.com>
On Tue, Feb 18, 2025 at 05:17:20PM +0100, David Hildenbrand wrote:
> On 18.02.25 17:12, Lorenzo Stoakes wrote:
> > On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
> > > On 13.02.25 19:17, Lorenzo Stoakes wrote:
> > > > There is no reason to disallow guard regions in file-backed mappings -
> > > > readahead and fault-around both function correctly in the presence of PTE
> > > > markers, equally other operations relating to memory-mapped files function
> > > > correctly.
> > > >
> > > > Additionally, read-only mappings if introducing guard-regions, only
> > > > restrict the mapping further, which means there is no violation of any
> > > > access rights by permitting this to be so.
> > > >
> > > > Removing this restriction allows for read-only mapped files (such as
> > > > executable files) correctly which would otherwise not be permitted.
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > ---
> > > > mm/madvise.c | 8 +-------
> > > > 1 file changed, 1 insertion(+), 7 deletions(-)
> > > >
> > > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > > index 6ecead476a80..e01e93e179a8 100644
> > > > --- a/mm/madvise.c
> > > > +++ b/mm/madvise.c
> > > > @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
> > > > if (!allow_locked)
> > > > disallowed |= VM_LOCKED;
> > > > - if (!vma_is_anonymous(vma))
> > > > - return false;
> > > > -
> > > > - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
> > > > - return false;
> > > > -
> > > > - return true;
> > > > + return !(vma->vm_flags & disallowed);
> > > > }
> > > > static bool is_guard_pte_marker(pte_t ptent)
> > >
> > > Acked-by: David Hildenbrand <david@redhat.com>
> >
> > Thanks!
> >
> > >
> > > I assume these markers cannot completely prevent us from allocating
> > > pages/folios for these underlying file/pageache ranges of these markers in
> > > case of shmem during page faults, right?
> >
> > If the markers are in place, then page faulting will result in a
> > segfault. If we faulted in a shmem page then installed markers (which would
> > zap the range), then the page cache will be populated, but obviously
> > subject to standard reclaim.
>
> Well, yes, (a) if there is swap and (b), if the noswap option was not
> specified for tmpfs.
>
Right, yeah if you don't have it set up such that dropping a reference to the
folio doesn't drop the page altogether.
I think this matches expectation though in that you'd get the same results from
an MADV_DONTNEED followed by faulting the page again.
> Okay, so installing a guard entry might require punshing a hole to get rid
> of any already-existing memory. But readahead (below) might mess it up.
Only if you are so concerned about avoiding the page cache being populated there
that you want to do this :)
Readahead I think will not readahead into a holepunched region as the hole
punching extends to the fs layer _I believe_ I have not checked the code for
this, but I believe it actually changes the underlying file too right to say
'this part of the file is empty'?
(I did explicitly test hole punching with guard regions btw, by the by :)
>
> >
> > If we perform synchronous readahead prior to a guard region that includes
> > (partially or fully) a guard region we might major fault entries into the
> > page cache that are then not accessable _from that mapping_, this is rather
> > unavoidable as this doesn't account for page table mappings and should be
> > largely trivial overhead (also these folios are reclaimable).
>
> Right, that's what I had in mind: assume I have a single marker in a PMD,
> shmem might allocate a PMD THP to back that region, ignoring the marker
> hint. (so I think)
>
> Thanks!
>
> --
> Cheers,
>
> David / dhildenb
>
>
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: David Hildenbrand @ 2025-02-18 16:17 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <4d6d2942-10a1-46e8-93a6-7ce52b6af3ad@lucifer.local>
On 18.02.25 17:12, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
>> On 13.02.25 19:17, Lorenzo Stoakes wrote:
>>> There is no reason to disallow guard regions in file-backed mappings -
>>> readahead and fault-around both function correctly in the presence of PTE
>>> markers, equally other operations relating to memory-mapped files function
>>> correctly.
>>>
>>> Additionally, read-only mappings if introducing guard-regions, only
>>> restrict the mapping further, which means there is no violation of any
>>> access rights by permitting this to be so.
>>>
>>> Removing this restriction allows for read-only mapped files (such as
>>> executable files) correctly which would otherwise not be permitted.
>>>
>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> ---
>>> mm/madvise.c | 8 +-------
>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/mm/madvise.c b/mm/madvise.c
>>> index 6ecead476a80..e01e93e179a8 100644
>>> --- a/mm/madvise.c
>>> +++ b/mm/madvise.c
>>> @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
>>> if (!allow_locked)
>>> disallowed |= VM_LOCKED;
>>> - if (!vma_is_anonymous(vma))
>>> - return false;
>>> -
>>> - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
>>> - return false;
>>> -
>>> - return true;
>>> + return !(vma->vm_flags & disallowed);
>>> }
>>> static bool is_guard_pte_marker(pte_t ptent)
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>
> Thanks!
>
>>
>> I assume these markers cannot completely prevent us from allocating
>> pages/folios for these underlying file/pageache ranges of these markers in
>> case of shmem during page faults, right?
>
> If the markers are in place, then page faulting will result in a
> segfault. If we faulted in a shmem page then installed markers (which would
> zap the range), then the page cache will be populated, but obviously
> subject to standard reclaim.
Well, yes, (a) if there is swap and (b), if the noswap option was not
specified for tmpfs.
Okay, so installing a guard entry might require punshing a hole to get
rid of any already-existing memory. But readahead (below) might mess it up.
>
> If we perform synchronous readahead prior to a guard region that includes
> (partially or fully) a guard region we might major fault entries into the
> page cache that are then not accessable _from that mapping_, this is rather
> unavoidable as this doesn't account for page table mappings and should be
> largely trivial overhead (also these folios are reclaimable).
Right, that's what I had in mind: assume I have a single marker in a
PMD, shmem might allocate a PMD THP to back that region, ignoring the
marker hint. (so I think)
Thanks!
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: Lorenzo Stoakes @ 2025-02-18 16:12 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <6500a93f-aad1-4b21-a94e-feb493c344a3@redhat.com>
On Tue, Feb 18, 2025 at 05:01:16PM +0100, David Hildenbrand wrote:
> On 13.02.25 19:17, Lorenzo Stoakes wrote:
> > There is no reason to disallow guard regions in file-backed mappings -
> > readahead and fault-around both function correctly in the presence of PTE
> > markers, equally other operations relating to memory-mapped files function
> > correctly.
> >
> > Additionally, read-only mappings if introducing guard-regions, only
> > restrict the mapping further, which means there is no violation of any
> > access rights by permitting this to be so.
> >
> > Removing this restriction allows for read-only mapped files (such as
> > executable files) correctly which would otherwise not be permitted.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> > mm/madvise.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 6ecead476a80..e01e93e179a8 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
> > if (!allow_locked)
> > disallowed |= VM_LOCKED;
> > - if (!vma_is_anonymous(vma))
> > - return false;
> > -
> > - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
> > - return false;
> > -
> > - return true;
> > + return !(vma->vm_flags & disallowed);
> > }
> > static bool is_guard_pte_marker(pte_t ptent)
>
> Acked-by: David Hildenbrand <david@redhat.com>
Thanks!
>
> I assume these markers cannot completely prevent us from allocating
> pages/folios for these underlying file/pageache ranges of these markers in
> case of shmem during page faults, right?
If the markers are in place, then page faulting will result in a
segfault. If we faulted in a shmem page then installed markers (which would
zap the range), then the page cache will be populated, but obviously
subject to standard reclaim.
If we perform synchronous readahead prior to a guard region that includes
(partially or fully) a guard region we might major fault entries into the
page cache that are then not accessable _from that mapping_, this is rather
unavoidable as this doesn't account for page table mappings and should be
largely trivial overhead (also these folios are reclaimable).
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 1/4] mm: allow guard regions in file-backed and read-only mappings
From: David Hildenbrand @ 2025-02-18 16:01 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Suren Baghdasaryan, Liam R . Howlett, Matthew Wilcox,
Vlastimil Babka, Paul E . McKenney, Jann Horn, linux-mm,
linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <d885cb259174736c2830a5dfe07f81b214ef3faa.1739469950.git.lorenzo.stoakes@oracle.com>
On 13.02.25 19:17, Lorenzo Stoakes wrote:
> There is no reason to disallow guard regions in file-backed mappings -
> readahead and fault-around both function correctly in the presence of PTE
> markers, equally other operations relating to memory-mapped files function
> correctly.
>
> Additionally, read-only mappings if introducing guard-regions, only
> restrict the mapping further, which means there is no violation of any
> access rights by permitting this to be so.
>
> Removing this restriction allows for read-only mapped files (such as
> executable files) correctly which would otherwise not be permitted.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> mm/madvise.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 6ecead476a80..e01e93e179a8 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1051,13 +1051,7 @@ static bool is_valid_guard_vma(struct vm_area_struct *vma, bool allow_locked)
> if (!allow_locked)
> disallowed |= VM_LOCKED;
>
> - if (!vma_is_anonymous(vma))
> - return false;
> -
> - if ((vma->vm_flags & (VM_MAYWRITE | disallowed)) != VM_MAYWRITE)
> - return false;
> -
> - return true;
> + return !(vma->vm_flags & disallowed);
> }
>
> static bool is_guard_pte_marker(pte_t ptent)
Acked-by: David Hildenbrand <david@redhat.com>
I assume these markers cannot completely prevent us from allocating
pages/folios for these underlying file/pageache ranges of these markers
in case of shmem during page faults, right?
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-18 15:20 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <b5b9cfcb-341d-4a5a-a6b7-59526643ad71@lucifer.local>
> Right yeah that'd be super weird. And I don't want to add that logic.
>
>> Also not sure what happens if one does an mlock()/mlockall() after
>> already installing PTE markers.
>
> The existing logic already handles non-present cases by skipping them, in
> mlock_pte_range():
>
> for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
> ptent = ptep_get(pte);
> if (!pte_present(ptent))
> continue;
>
> ...
> }
I *think* that code only updates already-mapped folios, to properly call
mlock_folio()/munlock_folio().
It is not the code that populates pages on mlock()/mlockall(). I think
all that goes via mm_populate()/__mm_populate(), where "ordinary GUP"
should apply.
See populate_vma_page_range(), especially also the VM_LOCKONFAULT handling.
>
> Which covers off guard regions. Removing the guard regions after this will
> leave you in a weird situation where these entries will be zapped... maybe
> we need a patch to make MADV_GUARD_REMOVE check VM_LOCKED and in this case
> also populate?
Maybe? Or we say that it behaves like MADV_DONTNEED_LOCKED.
>
> Actually I think the simpler option is to just disallow MADV_GUARD_REMOVE
> if you since locked the range? The code currently allows this on the
> proviso that 'you aren't zapping locked mappings' but leaves the VMA in a
> state such that some entries would not be locked.
>
> It'd be pretty weird to lock guard regions like this.
>
> Having said all that, given what you say below, maybe it's not an issue
> after all?...
>
>>
>> __mm_populate() would skip whole VMAs in case populate_vma_page_range()
>> fails. And I would assume populate_vma_page_range() fails on the first
>> guard when it triggers a page fault.
>>
>> OTOH, supporting the mlock-on-fault thingy should be easy. That's precisely where
>> MADV_DONTNEED_LOCKED originates from:
>>
>> commit 9457056ac426e5ed0671356509c8dcce69f8dee0
>> Author: Johannes Weiner <hannes@cmpxchg.org>
>> Date: Thu Mar 24 18:14:12 2022 -0700
>>
>> mm: madvise: MADV_DONTNEED_LOCKED
>> MADV_DONTNEED historically rejects mlocked ranges, but with MLOCK_ONFAULT
>> and MCL_ONFAULT allowing to mlock without populating, there are valid use
>> cases for depopulating locked ranges as well.
>
> ...Hm this seems to imply the current guard remove stuff isn't quite so
> bad, so maybe the assumption that VM_LOCKED implies 'everything is
> populated' isn't quite as stringent then.
Right, with MCL_ONFAULT at least. Without MCL_ONFAULT, the assumption is
that everything is populated (unless, apparently one uses
MADV_DONTNEED_LOCKED or population failed, maybe).
VM_LOCKONFAULT seems to be the sane case. I wonder why
MADV_DONTNEED_LOCKED didn't explicitly check for that one ... maybe
there is a history to that.
>
> The restriction is as simple as:
>
> if (behavior != MADV_DONTNEED_LOCKED)
> forbidden |= VM_LOCKED;
>
>>
>>
>> Adding support for that would be indeed nice.
>
> I mean it's sort of maybe understandable why you'd want to MADV_DONTNEED
> locked ranges, but I really don't understand why you'd want to add guard
> regions to mlock()'ed regions?
Somme apps use mlockall(), and it might be nice to just be able to use
guard pages as if "Nothing happened".
E.g., QEMU has the option to use mlockall().
>
> Then again we're currently asymmetric as you can add them _before_
> mlock()'ing...
Right.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: Lorenzo Stoakes @ 2025-02-18 14:53 UTC (permalink / raw)
To: David Hildenbrand
Cc: Vlastimil Babka, Andrew Morton, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <37b606be-f1ef-4abf-83ff-c1f34567568e@redhat.com>
On Tue, Feb 18, 2025 at 03:35:19PM +0100, David Hildenbrand wrote:
> On 18.02.25 14:05, Lorenzo Stoakes wrote:
> > On Tue, Feb 18, 2025 at 01:12:05PM +0100, Vlastimil Babka wrote:
> > > On 2/13/25 19:16, Lorenzo Stoakes wrote:
> > > > The guard regions feature was initially implemented to support anonymous
> > > > mappings only, excluding shmem.
> > > >
> > > > This was done such as to introduce the feature carefully and incrementally
> > > > and to be conservative when considering the various caveats and corner
> > > > cases that are applicable to file-backed mappings but not to anonymous
> > > > ones.
> > > >
> > > > Now this feature has landed in 6.13, it is time to revisit this and to
> > > > extend this functionality to file-backed and shmem mappings.
> > > >
> > > > In order to make this maximally useful, and since one may map file-backed
> > > > mappings read-only (for instance ELF images), we also remove the
> > > > restriction on read-only mappings and permit the establishment of guard
> > > > regions in any non-hugetlb, non-mlock()'d mapping.
> > >
> > > Do you plan to address mlock later too? I guess somebody might find use for
> > > those. Is there some fundamental issue or just that we need to define some
> > > good semantics for corner cases? (i.e. if pages are already populated in the
> > > mlocked area, discarding them by replacing with guard pte's goes against
> > > that, so do we allow it or not?).
> >
> > Yeah that's the fundamental issue with mlock, it does not interact with the
> > zapping part of MADV_GUARD_INSTALL, and that is why we disallow it (but not so
> > for MADV_GUARD_REMOVE, as if a VMA that contains guard regions is locked
> > _afterwards_ there will be no zapping).
> >
> > We could potentially expose an equivalent, as there are for other flags, a
> > _LOCKED variant of the madvise() flag, like MADV_GUARD_INSTALL_LOCKED to make
> > this explicit.
> >
> > That is probably the most sensible option, if there is a need for this!
>
> mlock is weird, because it assumes that memory will be faulted in in the whole VMA.
>
> You'd likely have to populate + mlock the page when removing the guard.
Right yeah that'd be super weird. And I don't want to add that logic.
> Also not sure what happens if one does an mlock()/mlockall() after
> already installing PTE markers.
The existing logic already handles non-present cases by skipping them, in
mlock_pte_range():
for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
ptent = ptep_get(pte);
if (!pte_present(ptent))
continue;
...
}
Which covers off guard regions. Removing the guard regions after this will
leave you in a weird situation where these entries will be zapped... maybe
we need a patch to make MADV_GUARD_REMOVE check VM_LOCKED and in this case
also populate?
Actually I think the simpler option is to just disallow MADV_GUARD_REMOVE
if you since locked the range? The code currently allows this on the
proviso that 'you aren't zapping locked mappings' but leaves the VMA in a
state such that some entries would not be locked.
It'd be pretty weird to lock guard regions like this.
Having said all that, given what you say below, maybe it's not an issue
after all?...
>
> __mm_populate() would skip whole VMAs in case populate_vma_page_range()
> fails. And I would assume populate_vma_page_range() fails on the first
> guard when it triggers a page fault.
>
> OTOH, supporting the mlock-on-fault thingy should be easy. That's precisely where
> MADV_DONTNEED_LOCKED originates from:
>
> commit 9457056ac426e5ed0671356509c8dcce69f8dee0
> Author: Johannes Weiner <hannes@cmpxchg.org>
> Date: Thu Mar 24 18:14:12 2022 -0700
>
> mm: madvise: MADV_DONTNEED_LOCKED
> MADV_DONTNEED historically rejects mlocked ranges, but with MLOCK_ONFAULT
> and MCL_ONFAULT allowing to mlock without populating, there are valid use
> cases for depopulating locked ranges as well.
...Hm this seems to imply the current guard remove stuff isn't quite so
bad, so maybe the assumption that VM_LOCKED implies 'everything is
populated' isn't quite as stringent then.
The restriction is as simple as:
if (behavior != MADV_DONTNEED_LOCKED)
forbidden |= VM_LOCKED;
>
>
> Adding support for that would be indeed nice.
I mean it's sort of maybe understandable why you'd want to MADV_DONTNEED
locked ranges, but I really don't understand why you'd want to add guard
regions to mlock()'ed regions?
Then again we're currently asymmetric as you can add them _before_
mlock()'ing...
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply
* Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings
From: David Hildenbrand @ 2025-02-18 14:35 UTC (permalink / raw)
To: Lorenzo Stoakes, Vlastimil Babka
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Paul E . McKenney, Jann Horn, linux-mm,
linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <8d643393-ddc0-490d-8fad-ad0b2720afb1@lucifer.local>
On 18.02.25 14:05, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 01:12:05PM +0100, Vlastimil Babka wrote:
>> On 2/13/25 19:16, Lorenzo Stoakes wrote:
>>> The guard regions feature was initially implemented to support anonymous
>>> mappings only, excluding shmem.
>>>
>>> This was done such as to introduce the feature carefully and incrementally
>>> and to be conservative when considering the various caveats and corner
>>> cases that are applicable to file-backed mappings but not to anonymous
>>> ones.
>>>
>>> Now this feature has landed in 6.13, it is time to revisit this and to
>>> extend this functionality to file-backed and shmem mappings.
>>>
>>> In order to make this maximally useful, and since one may map file-backed
>>> mappings read-only (for instance ELF images), we also remove the
>>> restriction on read-only mappings and permit the establishment of guard
>>> regions in any non-hugetlb, non-mlock()'d mapping.
>>
>> Do you plan to address mlock later too? I guess somebody might find use for
>> those. Is there some fundamental issue or just that we need to define some
>> good semantics for corner cases? (i.e. if pages are already populated in the
>> mlocked area, discarding them by replacing with guard pte's goes against
>> that, so do we allow it or not?).
>
> Yeah that's the fundamental issue with mlock, it does not interact with the
> zapping part of MADV_GUARD_INSTALL, and that is why we disallow it (but not so
> for MADV_GUARD_REMOVE, as if a VMA that contains guard regions is locked
> _afterwards_ there will be no zapping).
>
> We could potentially expose an equivalent, as there are for other flags, a
> _LOCKED variant of the madvise() flag, like MADV_GUARD_INSTALL_LOCKED to make
> this explicit.
>
> That is probably the most sensible option, if there is a need for this!
mlock is weird, because it assumes that memory will be faulted in in the whole VMA.
You'd likely have to populate + mlock the page when removing the guard.
Also not sure what happens if one does an mlock()/mlockall() after
already installing PTE markers.
__mm_populate() would skip whole VMAs in case populate_vma_page_range()
fails. And I would assume populate_vma_page_range() fails on the first
guard when it triggers a page fault.
OTOH, supporting the mlock-on-fault thingy should be easy. That's precisely where
MADV_DONTNEED_LOCKED originates from:
commit 9457056ac426e5ed0671356509c8dcce69f8dee0
Author: Johannes Weiner <hannes@cmpxchg.org>
Date: Thu Mar 24 18:14:12 2022 -0700
mm: madvise: MADV_DONTNEED_LOCKED
MADV_DONTNEED historically rejects mlocked ranges, but with MLOCK_ONFAULT
and MCL_ONFAULT allowing to mlock without populating, there are valid use
cases for depopulating locked ranges as well.
Adding support for that would be indeed nice.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 4/4] tools/selftests: add file/shmem-backed mapping guard region tests
From: Vlastimil Babka @ 2025-02-18 14:18 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Suren Baghdasaryan, Liam R . Howlett, Matthew Wilcox,
Paul E . McKenney, Jann Horn, David Hildenbrand, linux-mm,
linux-kernel, Shuah Khan, linux-kselftest, linux-api,
John Hubbard, Juan Yescas, Kalesh Singh
In-Reply-To: <90c16bec5fcaafcd1700dfa3e9988c3e1aa9ac1d.1739469950.git.lorenzo.stoakes@oracle.com>
On 2/13/25 19:17, Lorenzo Stoakes wrote:
> Extend the guard region self tests to explicitly assert that guard regions
> work correctly for functionality specific to file-backed and shmem
> mappings.
>
> In addition to testing all of the existing guard region functionality that
> is currently tested against anonymous mappings against file-backed and
> shmem mappings (except those which are exclusive to anonymous mapping), we
> now also:
>
> * Test that MADV_SEQUENTIAL does not cause unexpected readahead behaviour.
> * Test that MAP_PRIVATE behaves as expected with guard regions installed in
> both a shared and private mapping of an fd.
> * Test that a read-only file can correctly establish guard regions.
> * Test a probable fault-around case does not interfere with guard regions
> (or vice-versa).
> * Test that truncation does not eliminate guard regions.
> * Test that hole punching functions as expected in the presence of guard
> regions.
> * Test that a read-only mapping of a memfd write sealed mapping can have
> guard regions established within it and function correctly without
> violation of the seal.
> * Test that guard regions installed into a mapping of the anonymous zero
> page function correctly.
Impressive.
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox