* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 20:20 UTC (permalink / raw)
To: Yu Zhao
Cc: David Hildenbrand, Linus Torvalds, linux-kernel, patches, tglx,
linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <CAOUHufYsxCb=taWWfUbuzi1Hmmug=ThQMoTjsxrtFkt=UXEu6w@mail.gmail.com>
Hi David,
On Thu, Jul 11, 2024 at 01:49:42PM -0600, Yu Zhao wrote:
> On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@redhat.com> wrote:
> > > - /* dirty lazyfree */
> > > - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
> > > - success = lru_gen_del_folio(lruvec, folio, true);
> > > - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> > > - folio_set_swapbacked(folio);
> > > - lruvec_add_folio_tail(lruvec, folio);
> > > - return true;
> > > - }
> This is an optimization to avoid an unnecessary trip to
> shrink_folio_list(), so it's safe to delete the entire 'if' block, and
> that would be preferable than leaving a dangling 'if'.
Alright, I'll just remove that entire chunk then, for v+1 of this patch?
That sounds prettttty okay.
Jason
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 20:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Hildenbrand, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpA7gvIvxNRkd6hp@zx2c4.com>
On Thu, Jul 11, 2024 at 10:07:30PM +0200, Jason A. Donenfeld wrote:
> Hi Linus,
>
> On Thu, Jul 11, 2024 at 10:57:17AM -0700, Linus Torvalds wrote:
> > May I suggest a slightly different approach: do what we did for "pte_mkwrite()".
> >
> > It needed the vma too, for not too dissimilar reasons: special dirty
> > bit handling for the shadow stack. See
>
> Thanks for the suggestion. That seems pretty clean.
>
> It still needs to avoid setting swapbacked in the first place, but
> ensuring that it's never dirty means it won't get turned back on.
>
> The first patch renames pte_dirty() to pte_dirty_novma(). The second
> patch adds an inline function, pte_dirty(pte, vma) that just forwards
> the pte to pte_dirty_novma(), and then converts callers that have a vma
> available to pass to call pte_dirty(). And then the VM_DROPPABLE patch
> simply adds the `&& !(vma->vm_flags & VM_DROPPABLE)` condition to
> pte_dirty().
>
> I put these in https://git.zx2c4.com/linux-rng/log/ per usual, and I'll
> post a new version to the list not before long (unless objections).
Oh, I didn't catch upthread in time (my mail flow is based on `lei
up`, which I guess I should run at greater frequency). It seems like we
apparently might go in a different direction.
I'll move that to https://git.zx2c4.com/linux-rng/log/?h=jd/pte_dirty in
case it's useful later, though.
Jason
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 20:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Hildenbrand, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <CAHk-=whGE_w46zVk=7S0zOcWv4Dp3EYtuJtzU92ab3pSnnmpHw@mail.gmail.com>
Hi Linus,
On Thu, Jul 11, 2024 at 10:57:17AM -0700, Linus Torvalds wrote:
> May I suggest a slightly different approach: do what we did for "pte_mkwrite()".
>
> It needed the vma too, for not too dissimilar reasons: special dirty
> bit handling for the shadow stack. See
Thanks for the suggestion. That seems pretty clean.
It still needs to avoid setting swapbacked in the first place, but
ensuring that it's never dirty means it won't get turned back on.
The first patch renames pte_dirty() to pte_dirty_novma(). The second
patch adds an inline function, pte_dirty(pte, vma) that just forwards
the pte to pte_dirty_novma(), and then converts callers that have a vma
available to pass to call pte_dirty(). And then the VM_DROPPABLE patch
simply adds the `&& !(vma->vm_flags & VM_DROPPABLE)` condition to
pte_dirty().
I put these in https://git.zx2c4.com/linux-rng/log/ per usual, and I'll
post a new version to the list not before long (unless objections).
Jason
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Yu Zhao @ 2024-07-11 19:58 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason A. Donenfeld, Linus Torvalds, linux-kernel, patches, tglx,
linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <da3ea234-d6dd-4809-b2f5-fbfedacb9748@redhat.com>
On Thu, Jul 11, 2024 at 1:53 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 11.07.24 21:49, Yu Zhao wrote:
> > On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 11.07.24 21:18, David Hildenbrand wrote:
> >>> On 11.07.24 20:56, David Hildenbrand wrote:
> >>>> On 11.07.24 20:54, Jason A. Donenfeld wrote:
> >>>>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
> >>>>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
> >>>>>>
> >>>>>> It should be set for THP/large folios.
> >>>>>
> >>>>> And it's tested too, apparently.
> >>>>>
> >>>>> Okay, well, how disappointing is this below? Because I'm running out of
> >>>>> tricks for flag reuse.
> >>>>>
> >>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> >>>>> index b9e914e1face..c1ea49a7f198 100644
> >>>>> --- a/include/linux/page-flags.h
> >>>>> +++ b/include/linux/page-flags.h
> >>>>> @@ -110,6 +110,7 @@ enum pageflags {
> >>>>> PG_workingset,
> >>>>> PG_error,
> >>>>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
> >>>>> + PG_owner_priv_2,
> >>>>
> >>>> Oh no, no new page flags please :)
> >>>>
> >>>> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
> >>>> always return false for these special VMAs.
> >>>
> >>> ... or look into removing that one case that gives us headake.
> >>>
> >>> No idea what would happen if we do the following:
> >>>
> >>> CCing Yu Zhao.
> >>>
> >>> diff --git a/mm/vmscan.c b/mm/vmscan.c
> >>> index 0761f91b407f..d1dfbd4fd38d 100644
> >>> --- a/mm/vmscan.c
> >>> +++ b/mm/vmscan.c
> >>> @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> >>> return true;
> >>> }
> >>>
> >>> - /* dirty lazyfree */
> >>> - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
> >>> - success = lru_gen_del_folio(lruvec, folio, true);
> >>> - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> >>> - folio_set_swapbacked(folio);
> >>> - lruvec_add_folio_tail(lruvec, folio);
> >>> - return true;
> >>> - }
> >>> + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
> >>> + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
> >>> + return false;
> >
> > This is an optimization to avoid an unnecessary trip to
> > shrink_folio_list(), so it's safe to delete the entire 'if' block, and
> > that would be preferable than leaving a dangling 'if'.
>
> Great, thanks.
>
> >
> >> Note that something is unclear to me: are we maybe running into that
> >> code also if folio_set_swapbacked() is already set and we are not in the
> >> lazyfree path (in contrast to what is documented)?
> >
> > Not sure what you mean: either rmap sees pte_dirty() and does
> > folio_mark_dirty() and then folio_set_swapbacked(); or MGLRU does the
> > same sequence, with the first two steps in walk_pte_range() and the
> > last one here.
>
> Let me rephrase:
>
> Checking for lazyfree is
>
> "folio_test_anon(folio) && !folio_test_swapbacked(folio)"
>
> Testing for dirtied lazyfree is
>
> "folio_test_anon(folio) && !folio_test_swapbacked(folio) &&
> folio_test)dirty(folio)"
>
> So I'm wondering about the missing folio_test_swapbacked() test.
It's not missing: type == LRU_GEN_FILE means folio_is_file_lru(),
which in turn means !folio_test_swapbacked().
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 19:53 UTC (permalink / raw)
To: Yu Zhao
Cc: Jason A. Donenfeld, Linus Torvalds, linux-kernel, patches, tglx,
linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <CAOUHufYsxCb=taWWfUbuzi1Hmmug=ThQMoTjsxrtFkt=UXEu6w@mail.gmail.com>
On 11.07.24 21:49, Yu Zhao wrote:
> On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 11.07.24 21:18, David Hildenbrand wrote:
>>> On 11.07.24 20:56, David Hildenbrand wrote:
>>>> On 11.07.24 20:54, Jason A. Donenfeld wrote:
>>>>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
>>>>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
>>>>>>
>>>>>> It should be set for THP/large folios.
>>>>>
>>>>> And it's tested too, apparently.
>>>>>
>>>>> Okay, well, how disappointing is this below? Because I'm running out of
>>>>> tricks for flag reuse.
>>>>>
>>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>>>> index b9e914e1face..c1ea49a7f198 100644
>>>>> --- a/include/linux/page-flags.h
>>>>> +++ b/include/linux/page-flags.h
>>>>> @@ -110,6 +110,7 @@ enum pageflags {
>>>>> PG_workingset,
>>>>> PG_error,
>>>>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
>>>>> + PG_owner_priv_2,
>>>>
>>>> Oh no, no new page flags please :)
>>>>
>>>> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
>>>> always return false for these special VMAs.
>>>
>>> ... or look into removing that one case that gives us headake.
>>>
>>> No idea what would happen if we do the following:
>>>
>>> CCing Yu Zhao.
>>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 0761f91b407f..d1dfbd4fd38d 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
>>> return true;
>>> }
>>>
>>> - /* dirty lazyfree */
>>> - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
>>> - success = lru_gen_del_folio(lruvec, folio, true);
>>> - VM_WARN_ON_ONCE_FOLIO(!success, folio);
>>> - folio_set_swapbacked(folio);
>>> - lruvec_add_folio_tail(lruvec, folio);
>>> - return true;
>>> - }
>>> + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
>>> + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
>>> + return false;
>
> This is an optimization to avoid an unnecessary trip to
> shrink_folio_list(), so it's safe to delete the entire 'if' block, and
> that would be preferable than leaving a dangling 'if'.
Great, thanks.
>
>> Note that something is unclear to me: are we maybe running into that
>> code also if folio_set_swapbacked() is already set and we are not in the
>> lazyfree path (in contrast to what is documented)?
>
> Not sure what you mean: either rmap sees pte_dirty() and does
> folio_mark_dirty() and then folio_set_swapbacked(); or MGLRU does the
> same sequence, with the first two steps in walk_pte_range() and the
> last one here.
Let me rephrase:
Checking for lazyfree is
"folio_test_anon(folio) && !folio_test_swapbacked(folio)"
Testing for dirtied lazyfree is
"folio_test_anon(folio) && !folio_test_swapbacked(folio) &&
folio_test)dirty(folio)"
So I'm wondering about the missing folio_test_swapbacked() test.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Yu Zhao @ 2024-07-11 19:52 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason A. Donenfeld, Linus Torvalds, linux-kernel, patches, tglx,
linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <CAOUHufYsxCb=taWWfUbuzi1Hmmug=ThQMoTjsxrtFkt=UXEu6w@mail.gmail.com>
On Thu, Jul 11, 2024 at 1:49 PM Yu Zhao <yuzhao@google.com> wrote:
>
> On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@redhat.com> wrote:
> >
> > On 11.07.24 21:18, David Hildenbrand wrote:
> > > On 11.07.24 20:56, David Hildenbrand wrote:
> > >> On 11.07.24 20:54, Jason A. Donenfeld wrote:
> > >>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
> > >>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
> > >>>>
> > >>>> It should be set for THP/large folios.
> > >>>
> > >>> And it's tested too, apparently.
> > >>>
> > >>> Okay, well, how disappointing is this below? Because I'm running out of
> > >>> tricks for flag reuse.
> > >>>
> > >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > >>> index b9e914e1face..c1ea49a7f198 100644
> > >>> --- a/include/linux/page-flags.h
> > >>> +++ b/include/linux/page-flags.h
> > >>> @@ -110,6 +110,7 @@ enum pageflags {
> > >>> PG_workingset,
> > >>> PG_error,
> > >>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
> > >>> + PG_owner_priv_2,
> > >>
> > >> Oh no, no new page flags please :)
> > >>
> > >> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
> > >> always return false for these special VMAs.
> > >
> > > ... or look into removing that one case that gives us headake.
> > >
> > > No idea what would happen if we do the following:
> > >
> > > CCing Yu Zhao.
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 0761f91b407f..d1dfbd4fd38d 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> > > return true;
> > > }
> > >
> > > - /* dirty lazyfree */
> > > - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
> > > - success = lru_gen_del_folio(lruvec, folio, true);
> > > - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> > > - folio_set_swapbacked(folio);
> > > - lruvec_add_folio_tail(lruvec, folio);
> > > - return true;
> > > - }
> > > + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
> > > + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
> > > + return false;
>
> This is an optimization to avoid an unnecessary trip to
> shrink_folio_list(), so it's safe to delete the entire 'if' block, and
> that would be preferable than leaving a dangling 'if'.
>
> > Note that something is unclear to me: are we maybe running into that
> > code also if folio_set_swapbacked() is already set and we are not in the
> > lazyfree path (in contrast to what is documented)?
>
> Not sure what you mean: either rmap sees pte_dirty() and does
> folio_mark_dirty() and then folio_set_swapbacked(); or MGLRU does the
> same sequence, with the first two steps in walk_pte_range() and the
> last one here.
Rationale: rmap is expensive (cache unfriendly) and MGLRU tries to
avoid using it.
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Yu Zhao @ 2024-07-11 19:49 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason A. Donenfeld, Linus Torvalds, linux-kernel, patches, tglx,
linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <2c464271-1c61-4cd8-bd4e-4bd8aa01fa00@redhat.com>
On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 11.07.24 21:18, David Hildenbrand wrote:
> > On 11.07.24 20:56, David Hildenbrand wrote:
> >> On 11.07.24 20:54, Jason A. Donenfeld wrote:
> >>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
> >>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
> >>>>
> >>>> It should be set for THP/large folios.
> >>>
> >>> And it's tested too, apparently.
> >>>
> >>> Okay, well, how disappointing is this below? Because I'm running out of
> >>> tricks for flag reuse.
> >>>
> >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> >>> index b9e914e1face..c1ea49a7f198 100644
> >>> --- a/include/linux/page-flags.h
> >>> +++ b/include/linux/page-flags.h
> >>> @@ -110,6 +110,7 @@ enum pageflags {
> >>> PG_workingset,
> >>> PG_error,
> >>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
> >>> + PG_owner_priv_2,
> >>
> >> Oh no, no new page flags please :)
> >>
> >> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
> >> always return false for these special VMAs.
> >
> > ... or look into removing that one case that gives us headake.
> >
> > No idea what would happen if we do the following:
> >
> > CCing Yu Zhao.
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 0761f91b407f..d1dfbd4fd38d 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> > return true;
> > }
> >
> > - /* dirty lazyfree */
> > - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
> > - success = lru_gen_del_folio(lruvec, folio, true);
> > - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> > - folio_set_swapbacked(folio);
> > - lruvec_add_folio_tail(lruvec, folio);
> > - return true;
> > - }
> > + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
> > + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
> > + return false;
This is an optimization to avoid an unnecessary trip to
shrink_folio_list(), so it's safe to delete the entire 'if' block, and
that would be preferable than leaving a dangling 'if'.
> Note that something is unclear to me: are we maybe running into that
> code also if folio_set_swapbacked() is already set and we are not in the
> lazyfree path (in contrast to what is documented)?
Not sure what you mean: either rmap sees pte_dirty() and does
folio_mark_dirty() and then folio_set_swapbacked(); or MGLRU does the
same sequence, with the first two steps in walk_pte_range() and the
last one here.
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 19:22 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jason A. Donenfeld, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <CAHk-=wjVc6cpSCJwAqrhPvwBbcQEOL2TEnCELfadhA=n1GN4Ww@mail.gmail.com>
On 11.07.24 21:17, Linus Torvalds wrote:
> On Thu, 11 Jul 2024 at 12:08, David Hildenbrand <david@redhat.com> wrote:
>>
>> We also have these folio_mark_dirty() calls, for example in
>> unpin_user_pages_dirty_lock(). Hm ... so preventing the folio from
>> getting dirtied is likely shaky.
>
> I do wonder if we should just disallow page pinning for these pages
> entirely. When the page can get replaced by zeroes at any time,
> pinning it doesn't make much sense.
>
> Except we do have that whole "fast" case that intentionally doesn't
> take locks and doesn't have a vma. Darn.
Yeah, and I think it should all be simpler; we shouldn't have to
special-case these cases everywhere.
Maybe we can just find a way to not do *folio_set_swapbacked() without a
VMA.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 19:20 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm, Yu Zhao
In-Reply-To: <1c8632b4-06a5-49da-be0c-6fc7ac2b3257@redhat.com>
On 11.07.24 21:18, David Hildenbrand wrote:
> On 11.07.24 20:56, David Hildenbrand wrote:
>> On 11.07.24 20:54, Jason A. Donenfeld wrote:
>>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
>>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
>>>>
>>>> It should be set for THP/large folios.
>>>
>>> And it's tested too, apparently.
>>>
>>> Okay, well, how disappointing is this below? Because I'm running out of
>>> tricks for flag reuse.
>>>
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index b9e914e1face..c1ea49a7f198 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -110,6 +110,7 @@ enum pageflags {
>>> PG_workingset,
>>> PG_error,
>>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
>>> + PG_owner_priv_2,
>>
>> Oh no, no new page flags please :)
>>
>> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
>> always return false for these special VMAs.
>
> ... or look into removing that one case that gives us headake.
>
> No idea what would happen if we do the following:
>
> CCing Yu Zhao.
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 0761f91b407f..d1dfbd4fd38d 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> return true;
> }
>
> - /* dirty lazyfree */
> - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
> - success = lru_gen_del_folio(lruvec, folio, true);
> - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> - folio_set_swapbacked(folio);
> - lruvec_add_folio_tail(lruvec, folio);
> - return true;
> - }
> + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
> + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
> + return false;
Note that something is unclear to me: are we maybe running into that
code also if folio_set_swapbacked() is already set and we are not in the
lazyfree path (in contrast to what is documented)?
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 19:18 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm, Yu Zhao
In-Reply-To: <443146f4-9db8-4a19-91f1-b6822fad8ce8@redhat.com>
On 11.07.24 20:56, David Hildenbrand wrote:
> On 11.07.24 20:54, Jason A. Donenfeld wrote:
>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
>>>> And PG_large_rmappable seems to only be used for hugetlb branches.
>>>
>>> It should be set for THP/large folios.
>>
>> And it's tested too, apparently.
>>
>> Okay, well, how disappointing is this below? Because I'm running out of
>> tricks for flag reuse.
>>
>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>> index b9e914e1face..c1ea49a7f198 100644
>> --- a/include/linux/page-flags.h
>> +++ b/include/linux/page-flags.h
>> @@ -110,6 +110,7 @@ enum pageflags {
>> PG_workingset,
>> PG_error,
>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
>> + PG_owner_priv_2,
>
> Oh no, no new page flags please :)
>
> Maybe just follow what Linux suggested: pass vma to pte_dirty() and
> always return false for these special VMAs.
... or look into removing that one case that gives us headake.
No idea what would happen if we do the following:
CCing Yu Zhao.
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0761f91b407f..d1dfbd4fd38d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
return true;
}
- /* dirty lazyfree */
- if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
- success = lru_gen_del_folio(lruvec, folio, true);
- VM_WARN_ON_ONCE_FOLIO(!success, folio);
- folio_set_swapbacked(folio);
- lruvec_add_folio_tail(lruvec, folio);
- return true;
- }
+ /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */
+ if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio))
+ return false;
--
Cheers,
David / dhildenb
^ permalink raw reply related
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Linus Torvalds @ 2024-07-11 19:17 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason A. Donenfeld, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <37da7835-0d76-463e-b074-455e405b138b@redhat.com>
On Thu, 11 Jul 2024 at 12:08, David Hildenbrand <david@redhat.com> wrote:
>
> We also have these folio_mark_dirty() calls, for example in
> unpin_user_pages_dirty_lock(). Hm ... so preventing the folio from
> getting dirtied is likely shaky.
I do wonder if we should just disallow page pinning for these pages
entirely. When the page can get replaced by zeroes at any time,
pinning it doesn't make much sense.
Except we do have that whole "fast" case that intentionally doesn't
take locks and doesn't have a vma. Darn.
Linus
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 19:07 UTC (permalink / raw)
To: Linus Torvalds, Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <CAHk-=whGE_w46zVk=7S0zOcWv4Dp3EYtuJtzU92ab3pSnnmpHw@mail.gmail.com>
On 11.07.24 19:57, Linus Torvalds wrote:
> On Thu, 11 Jul 2024 at 10:09, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> When I was working on this patchset this year with the syscall, this is
>> similar somewhat to the initial approach I was taking with setting up a
>> special mapping. It turned into kind of a mess and I couldn't get it
>> working. There's a lot of functionality built around anonymous pages
>> that would need to be duplicated (I think?).
>
> Yeah, I was kind of assuming that. You'd need to handle VM_DROPPABLE
> in the fault path specially, the way we currently split up based on
> vma_is_anonymous(), eg
>
> if (vma_is_anonymous(vmf->vma))
> return do_anonymous_page(vmf);
> else
> return do_fault(vmf);
>
> in do_pte_missing() etc.
>
> I don't actually think it would be too hard, but it's a more
> "conceptual" change, and it's probably not worth it.
>
>> Alright, an hour later of fiddling, and it doesn't actually work (yet?)
>> -- the selftest fails. A diff follows below.
>
> May I suggest a slightly different approach: do what we did for "pte_mkwrite()".
>
> It needed the vma too, for not too dissimilar reasons: special dirty
> bit handling for the shadow stack. See
>
> bb3aadf7d446 ("x86/mm: Start actually marking _PAGE_SAVED_DIRTY")
> b497e52ddb2a ("x86/mm: Teach pte_mkwrite() about stack memory")
>
> and now we have "pte_mkwrite_novma()" with the old semantics for the
> legacy cases that didn't get converted - whether it's because the
> architecture doesn't have the issue, or because it's a kernel pte.
>
> And the conversion was actually quite pain-free, because we have
>
> #ifndef pte_mkwrite
> static inline pte_t pte_mkwrite(pte_t pte, struct vm_area_struct *vma)
> {
> return pte_mkwrite_novma(pte);
> }
> #endif
>
> so all any architecture that didn't want this needed to do was to
> rename their pte_mkwrite() to pte_mkwrite_novma() and they were done.
> In fact, that was done first as basically semantically no-op patches:
>
> 2f0584f3f4bd ("mm: Rename arch pte_mkwrite()'s to pte_mkwrite_novma()")
> 6ecc21bb432d ("mm: Move pte/pmd_mkwrite() callers with no VMA to _novma()")
> 161e393c0f63 ("mm: Make pte_mkwrite() take a VMA")
>
> which made this all very pain-free (and was largely a sed script, I think).
>
>> - !pte_dirty(pte) && !PageDirty(page))
>> + !pte_dirty(pte) && !PageDirty(page) &&
>> + !(vma->vm_flags & VM_DROPPABLE))
>
> So instead of this kind of thing, we'd have
>
>> - !pte_dirty(pte) && !PageDirty(page))
>> + !pte_dirty(pte, vma) && !PageDirty(page) &&
>
> and the advantage here is that you can't miss anybody by mistake. The
> compiler will be very unhappy if you don't pass in the vma, and then
> any places that would be converted to "pte_dirty_novma()"
>
> We don't actually have all that many users of pte_dirty(), so it
> doesn't look too nasty. And if we make the pte_dirty() semantics
> depend on the vma, I really think we should do it the same way we did
> pte_mkwrite().
We also have these folio_mark_dirty() calls, for example in
unpin_user_pages_dirty_lock(). Hm ... so preventing the folio from
getting dirtied is likely shaky.
I guess we need a way to just reliably identify these folios :/.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 18:56 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAqbh3TnB9hIRRh@zx2c4.com>
On 11.07.24 20:54, Jason A. Donenfeld wrote:
> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
>>> And PG_large_rmappable seems to only be used for hugetlb branches.
>>
>> It should be set for THP/large folios.
>
> And it's tested too, apparently.
>
> Okay, well, how disappointing is this below? Because I'm running out of
> tricks for flag reuse.
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index b9e914e1face..c1ea49a7f198 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -110,6 +110,7 @@ enum pageflags {
> PG_workingset,
> PG_error,
> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
> + PG_owner_priv_2,
Oh no, no new page flags please :)
Maybe just follow what Linux suggested: pass vma to pte_dirty() and
always return false for these special VMAs.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 18:54 UTC (permalink / raw)
To: David Hildenbrand
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <8586b19c-2e14-4164-888f-8c3b86f3f963@redhat.com>
On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote:
> > And PG_large_rmappable seems to only be used for hugetlb branches.
>
> It should be set for THP/large folios.
And it's tested too, apparently.
Okay, well, how disappointing is this below? Because I'm running out of
tricks for flag reuse.
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index b9e914e1face..c1ea49a7f198 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -110,6 +110,7 @@ enum pageflags {
PG_workingset,
PG_error,
PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/
+ PG_owner_priv_2,
PG_arch_1,
PG_reserved,
PG_private, /* If pagecache, has fs-private data */
@@ -190,6 +191,9 @@ enum pageflags {
/* At least one page in this folio has the hwpoison flag set */
PG_has_hwpoisoned = PG_error,
PG_large_rmappable = PG_workingset, /* anon or file-backed */
+
+ /* Zero page under memory pressure. */
+ PG_droppable = PG_owner_priv_2,
};
#define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1)
@@ -549,6 +553,8 @@ PAGEFLAG(Private, private, PF_ANY)
PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
+PAGEFLAG(OwnerPriv2, owner_priv_2, PF_ANY)
+ TESTCLEARFLAG(OwnerPriv2, owner_priv_2, PF_ANY)
/*
* Only test-and-set exist for PG_writeback. The unconditional operators are
@@ -640,6 +646,8 @@ FOLIO_TEST_CLEAR_FLAG_FALSE(young)
FOLIO_FLAG_FALSE(idle)
#endif
+FOLIO_FLAG(droppable, FOLIO_SECOND_PAGE)
+
/*
* PageReported() is used to track reported free pages within the Buddy
* allocator. We can use the non-atomic version of the test and set
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index b63d211bd141..986551588805 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -108,6 +108,7 @@
DEF_PAGEFLAG_NAME(active), \
DEF_PAGEFLAG_NAME(workingset), \
DEF_PAGEFLAG_NAME(owner_priv_1), \
+ DEF_PAGEFLAG_NAME(owner_priv_2), \
DEF_PAGEFLAG_NAME(arch_1), \
DEF_PAGEFLAG_NAME(reserved), \
DEF_PAGEFLAG_NAME(private), \
diff --git a/mm/rmap.c b/mm/rmap.c
index 1f9b5a9cb121..73b4052b2f82 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
*/
if (!(vma->vm_flags & VM_DROPPABLE))
__folio_set_swapbacked(folio);
+ else
+ folio_set_droppable(folio);
__folio_set_anon(folio, vma, address, true);
if (likely(!folio_test_large(folio))) {
@@ -1852,7 +1854,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* ones can be dropped even if they've
* been dirtied.
*/
- (vma->vm_flags & VM_DROPPABLE))) {
+ folio_test_droppable(folio))) {
dec_mm_counter(mm, MM_ANONPAGES);
goto discard;
}
@@ -1866,7 +1868,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* Unlike MADV_FREE mappings, VM_DROPPABLE ones
* never get swap backed on failure to drop.
*/
- if (!(vma->vm_flags & VM_DROPPABLE))
+ if (!folio_test_droppable(folio))
folio_set_swapbacked(folio);
ret = false;
page_vma_mapped_walk_done(&pvmw);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2e34de9cd0d4..41340f2a12c7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4266,7 +4266,8 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
}
/* dirty lazyfree */
- if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
+ if (type == LRU_GEN_FILE && folio_test_anon(folio) &&
+ folio_test_dirty(folio) && !folio_test_droppable(folio)) {
success = lru_gen_del_folio(lruvec, folio, true);
VM_WARN_ON_ONCE_FOLIO(!success, folio);
folio_set_swapbacked(folio);
^ permalink raw reply related
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 18:24 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAfigBHfHdVeyNO@zx2c4.com>
On 11.07.24 20:08, Jason A. Donenfeld wrote:
> On Thu, Jul 11, 2024 at 07:56:39PM +0200, Jason A. Donenfeld wrote:
>> On Thu, Jul 11, 2024 at 07:54:34PM +0200, Jason A. Donenfeld wrote:
>>> On Thu, Jul 11, 2024 at 07:27:27PM +0200, David Hildenbrand wrote:
>>>>> PG_owner_priv_1 maps to PG_swapcache? :)
>>>>
>>>> Maybe the combination !swapbacked && swapcache could be used to indicate
>>>> such folios. (we will never set swapbacked)
>>>>
>>>> But likely we have to be a bit careful here. We don't want
>>>> folio_test_swapcache() to return for folios that ... are not in the
>>>> swapcache.
>>>
>>> I was thinking that too, but I'm afraid it's going to be another
>>> whack-a-mole nightmare. Even for things like task_mmu in procfs that
>>> show stats, that's going to be wonky.
>>>
>>> Any other flags we can overload that aren't going to be already used in
>>> our case?
>>
>> PG_error / folio_set_error seems unused in the non-IO case.
>
Note that Willy is about to remove PG_error IIRC.
> And PG_large_rmappable seems to only be used for hugetlb branches.
It should be set for THP/large folios.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 18:08 UTC (permalink / raw)
To: David Hildenbrand
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAc118_U7p3u2gZ@zx2c4.com>
On Thu, Jul 11, 2024 at 07:56:39PM +0200, Jason A. Donenfeld wrote:
> On Thu, Jul 11, 2024 at 07:54:34PM +0200, Jason A. Donenfeld wrote:
> > On Thu, Jul 11, 2024 at 07:27:27PM +0200, David Hildenbrand wrote:
> > > > PG_owner_priv_1 maps to PG_swapcache? :)
> > >
> > > Maybe the combination !swapbacked && swapcache could be used to indicate
> > > such folios. (we will never set swapbacked)
> > >
> > > But likely we have to be a bit careful here. We don't want
> > > folio_test_swapcache() to return for folios that ... are not in the
> > > swapcache.
> >
> > I was thinking that too, but I'm afraid it's going to be another
> > whack-a-mole nightmare. Even for things like task_mmu in procfs that
> > show stats, that's going to be wonky.
> >
> > Any other flags we can overload that aren't going to be already used in
> > our case?
>
> PG_error / folio_set_error seems unused in the non-IO case.
And PG_large_rmappable seems to only be used for hugetlb branches.
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index b9e914e1face..7fdc03197438 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -190,6 +190,7 @@ enum pageflags {
/* At least one page in this folio has the hwpoison flag set */
PG_has_hwpoisoned = PG_error,
PG_large_rmappable = PG_workingset, /* anon or file-backed */
+ PG_droppable = PG_error, /* anon droppable, not hugetlb */
};
#define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1)
@@ -640,6 +641,8 @@ FOLIO_TEST_CLEAR_FLAG_FALSE(young)
FOLIO_FLAG_FALSE(idle)
#endif
+FOLIO_FLAG(droppable, FOLIO_SECOND_PAGE)
+
/*
* PageReported() is used to track reported free pages within the Buddy
* allocator. We can use the non-atomic version of the test and set
diff --git a/mm/rmap.c b/mm/rmap.c
index 1f9b5a9cb121..73b4052b2f82 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
*/
if (!(vma->vm_flags & VM_DROPPABLE))
__folio_set_swapbacked(folio);
+ else
+ folio_set_droppable(folio);
__folio_set_anon(folio, vma, address, true);
if (likely(!folio_test_large(folio))) {
@@ -1852,7 +1854,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* ones can be dropped even if they've
* been dirtied.
*/
- (vma->vm_flags & VM_DROPPABLE))) {
+ folio_test_droppable(folio))) {
dec_mm_counter(mm, MM_ANONPAGES);
goto discard;
}
@@ -1866,7 +1868,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* Unlike MADV_FREE mappings, VM_DROPPABLE ones
* never get swap backed on failure to drop.
*/
- if (!(vma->vm_flags & VM_DROPPABLE))
+ if (!folio_test_droppable(folio))
folio_set_swapbacked(folio);
ret = false;
page_vma_mapped_walk_done(&pvmw);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2e34de9cd0d4..41340f2a12c7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4266,7 +4266,8 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
}
/* dirty lazyfree */
- if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
+ if (type == LRU_GEN_FILE && folio_test_anon(folio) &&
+ folio_test_dirty(folio) && !folio_test_droppable(folio)) {
success = lru_gen_del_folio(lruvec, folio, true);
VM_WARN_ON_ONCE_FOLIO(!success, folio);
folio_set_swapbacked(folio);
^ permalink raw reply related
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Linus Torvalds @ 2024-07-11 17:57 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: David Hildenbrand, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAR0CgLc28gEkV3@zx2c4.com>
On Thu, 11 Jul 2024 at 10:09, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> When I was working on this patchset this year with the syscall, this is
> similar somewhat to the initial approach I was taking with setting up a
> special mapping. It turned into kind of a mess and I couldn't get it
> working. There's a lot of functionality built around anonymous pages
> that would need to be duplicated (I think?).
Yeah, I was kind of assuming that. You'd need to handle VM_DROPPABLE
in the fault path specially, the way we currently split up based on
vma_is_anonymous(), eg
if (vma_is_anonymous(vmf->vma))
return do_anonymous_page(vmf);
else
return do_fault(vmf);
in do_pte_missing() etc.
I don't actually think it would be too hard, but it's a more
"conceptual" change, and it's probably not worth it.
> Alright, an hour later of fiddling, and it doesn't actually work (yet?)
> -- the selftest fails. A diff follows below.
May I suggest a slightly different approach: do what we did for "pte_mkwrite()".
It needed the vma too, for not too dissimilar reasons: special dirty
bit handling for the shadow stack. See
bb3aadf7d446 ("x86/mm: Start actually marking _PAGE_SAVED_DIRTY")
b497e52ddb2a ("x86/mm: Teach pte_mkwrite() about stack memory")
and now we have "pte_mkwrite_novma()" with the old semantics for the
legacy cases that didn't get converted - whether it's because the
architecture doesn't have the issue, or because it's a kernel pte.
And the conversion was actually quite pain-free, because we have
#ifndef pte_mkwrite
static inline pte_t pte_mkwrite(pte_t pte, struct vm_area_struct *vma)
{
return pte_mkwrite_novma(pte);
}
#endif
so all any architecture that didn't want this needed to do was to
rename their pte_mkwrite() to pte_mkwrite_novma() and they were done.
In fact, that was done first as basically semantically no-op patches:
2f0584f3f4bd ("mm: Rename arch pte_mkwrite()'s to pte_mkwrite_novma()")
6ecc21bb432d ("mm: Move pte/pmd_mkwrite() callers with no VMA to _novma()")
161e393c0f63 ("mm: Make pte_mkwrite() take a VMA")
which made this all very pain-free (and was largely a sed script, I think).
> - !pte_dirty(pte) && !PageDirty(page))
> + !pte_dirty(pte) && !PageDirty(page) &&
> + !(vma->vm_flags & VM_DROPPABLE))
So instead of this kind of thing, we'd have
> - !pte_dirty(pte) && !PageDirty(page))
> + !pte_dirty(pte, vma) && !PageDirty(page) &&
and the advantage here is that you can't miss anybody by mistake. The
compiler will be very unhappy if you don't pass in the vma, and then
any places that would be converted to "pte_dirty_novma()"
We don't actually have all that many users of pte_dirty(), so it
doesn't look too nasty. And if we make the pte_dirty() semantics
depend on the vma, I really think we should do it the same way we did
pte_mkwrite().
Long-term, maybe we should just aim to always pass in the vma to the
pte_xyz() functions, but...
Linus
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 17:56 UTC (permalink / raw)
To: David Hildenbrand
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAcWvij59AzUD9u@zx2c4.com>
On Thu, Jul 11, 2024 at 07:54:34PM +0200, Jason A. Donenfeld wrote:
> On Thu, Jul 11, 2024 at 07:27:27PM +0200, David Hildenbrand wrote:
> > > PG_owner_priv_1 maps to PG_swapcache? :)
> >
> > Maybe the combination !swapbacked && swapcache could be used to indicate
> > such folios. (we will never set swapbacked)
> >
> > But likely we have to be a bit careful here. We don't want
> > folio_test_swapcache() to return for folios that ... are not in the
> > swapcache.
>
> I was thinking that too, but I'm afraid it's going to be another
> whack-a-mole nightmare. Even for things like task_mmu in procfs that
> show stats, that's going to be wonky.
>
> Any other flags we can overload that aren't going to be already used in
> our case?
PG_error / folio_set_error seems unused in the non-IO case.
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 17:54 UTC (permalink / raw)
To: David Hildenbrand
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <54b6de32-f127-4928-9f4a-acb8653e5c81@redhat.com>
On Thu, Jul 11, 2024 at 07:27:27PM +0200, David Hildenbrand wrote:
> > PG_owner_priv_1 maps to PG_swapcache? :)
>
> Maybe the combination !swapbacked && swapcache could be used to indicate
> such folios. (we will never set swapbacked)
>
> But likely we have to be a bit careful here. We don't want
> folio_test_swapcache() to return for folios that ... are not in the
> swapcache.
I was thinking that too, but I'm afraid it's going to be another
whack-a-mole nightmare. Even for things like task_mmu in procfs that
show stats, that's going to be wonky.
Any other flags we can overload that aren't going to be already used in
our case?
Jason
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 17:49 UTC (permalink / raw)
To: David Hildenbrand
Cc: Linus Torvalds, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <98798483-dfcd-451e-94bb-57d830bf68d8@redhat.com>
On Thu, Jul 11, 2024 at 07:24:33PM +0200, David Hildenbrand wrote:
> On 11.07.24 19:17, Jason A. Donenfeld wrote:
> > On Thu, Jul 11, 2024 at 07:09:36PM +0200, Jason A. Donenfeld wrote:
> >> So, hmm... The swapbacked thing really seemed so simple... I wonder if
> >> there's a way of recovering that.
> >
> > Not wanting to introduce a new bitflag, I went looking and noticed this:
> >
> > /*
> > * Private page markings that may be used by the filesystem that owns the page
> > * for its own purposes.
> > * - PG_private and PG_private_2 cause release_folio() and co to be invoked
> > */
> > PAGEFLAG(Private, private, PF_ANY)
> > PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
> > PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
> > TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
> >
> > The below +4/-1 diff is pretty hacky and might be illegal in the state
> > of California, but I think it does work. The idea is that if that bit is
> > normally only used for filesystems, then in the anonymous case, it's
> > free to be used for this.
> >
> > Any opinions about this, or a suggestion on how to do that in a less
> > ugly way?
> >
> > Jason
> >
> >
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 1f9b5a9cb121..090554277e4a 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
> > */
> > if (!(vma->vm_flags & VM_DROPPABLE))
> > __folio_set_swapbacked(folio);
> > + else
> > + folio_set_owner_priv_1(folio);
>
>
> PG_owner_priv_1 maps to PG_swapcache? :)
Oh, drat, it looks like this overloading is nothing new then.
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 17:27 UTC (permalink / raw)
To: Jason A. Donenfeld, Linus Torvalds
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <98798483-dfcd-451e-94bb-57d830bf68d8@redhat.com>
On 11.07.24 19:24, David Hildenbrand wrote:
> On 11.07.24 19:17, Jason A. Donenfeld wrote:
>> On Thu, Jul 11, 2024 at 07:09:36PM +0200, Jason A. Donenfeld wrote:
>>> So, hmm... The swapbacked thing really seemed so simple... I wonder if
>>> there's a way of recovering that.
>>
>> Not wanting to introduce a new bitflag, I went looking and noticed this:
>>
>> /*
>> * Private page markings that may be used by the filesystem that owns the page
>> * for its own purposes.
>> * - PG_private and PG_private_2 cause release_folio() and co to be invoked
>> */
>> PAGEFLAG(Private, private, PF_ANY)
>> PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
>> PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
>> TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
>>
>> The below +4/-1 diff is pretty hacky and might be illegal in the state
>> of California, but I think it does work. The idea is that if that bit is
>> normally only used for filesystems, then in the anonymous case, it's
>> free to be used for this.
>>
>> Any opinions about this, or a suggestion on how to do that in a less
>> ugly way?
>>
>> Jason
>>
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 1f9b5a9cb121..090554277e4a 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
>> */
>> if (!(vma->vm_flags & VM_DROPPABLE))
>> __folio_set_swapbacked(folio);
>> + else
>> + folio_set_owner_priv_1(folio);
>
>
> PG_owner_priv_1 maps to PG_swapcache? :)
Maybe the combination !swapbacked && swapcache could be used to indicate
such folios. (we will never set swapbacked)
But likely we have to be a bit careful here. We don't want
folio_test_swapcache() to return for folios that ... are not in the
swapcache.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-11 17:24 UTC (permalink / raw)
To: Jason A. Donenfeld, Linus Torvalds
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <ZpATx21F_01SBRnO@zx2c4.com>
On 11.07.24 19:17, Jason A. Donenfeld wrote:
> On Thu, Jul 11, 2024 at 07:09:36PM +0200, Jason A. Donenfeld wrote:
>> So, hmm... The swapbacked thing really seemed so simple... I wonder if
>> there's a way of recovering that.
>
> Not wanting to introduce a new bitflag, I went looking and noticed this:
>
> /*
> * Private page markings that may be used by the filesystem that owns the page
> * for its own purposes.
> * - PG_private and PG_private_2 cause release_folio() and co to be invoked
> */
> PAGEFLAG(Private, private, PF_ANY)
> PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
> PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
> TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
>
> The below +4/-1 diff is pretty hacky and might be illegal in the state
> of California, but I think it does work. The idea is that if that bit is
> normally only used for filesystems, then in the anonymous case, it's
> free to be used for this.
>
> Any opinions about this, or a suggestion on how to do that in a less
> ugly way?
>
> Jason
>
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1f9b5a9cb121..090554277e4a 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
> */
> if (!(vma->vm_flags & VM_DROPPABLE))
> __folio_set_swapbacked(folio);
> + else
> + folio_set_owner_priv_1(folio);
PG_owner_priv_1 maps to PG_swapcache? :)
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 17:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Hildenbrand, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <ZpAR0CgLc28gEkV3@zx2c4.com>
On Thu, Jul 11, 2024 at 07:09:36PM +0200, Jason A. Donenfeld wrote:
> So, hmm... The swapbacked thing really seemed so simple... I wonder if
> there's a way of recovering that.
Not wanting to introduce a new bitflag, I went looking and noticed this:
/*
* Private page markings that may be used by the filesystem that owns the page
* for its own purposes.
* - PG_private and PG_private_2 cause release_folio() and co to be invoked
*/
PAGEFLAG(Private, private, PF_ANY)
PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
The below +4/-1 diff is pretty hacky and might be illegal in the state
of California, but I think it does work. The idea is that if that bit is
normally only used for filesystems, then in the anonymous case, it's
free to be used for this.
Any opinions about this, or a suggestion on how to do that in a less
ugly way?
Jason
diff --git a/mm/rmap.c b/mm/rmap.c
index 1f9b5a9cb121..090554277e4a 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1403,6 +1403,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
*/
if (!(vma->vm_flags & VM_DROPPABLE))
__folio_set_swapbacked(folio);
+ else
+ folio_set_owner_priv_1(folio);
__folio_set_anon(folio, vma, address, true);
if (likely(!folio_test_large(folio))) {
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2e34de9cd0d4..398b46027e8f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4266,7 +4266,8 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
}
/* dirty lazyfree */
- if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) {
+ if (type == LRU_GEN_FILE && folio_test_anon(folio) &&
+ folio_test_dirty(folio) && !folio_test_owner_priv_1(folio)) {
success = lru_gen_del_folio(lruvec, folio, true);
VM_WARN_ON_ONCE_FOLIO(!success, folio);
folio_set_swapbacked(folio);
^ permalink raw reply related
* Re: [PATCH v22 1/4] mm: add MAP_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-11 17:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Hildenbrand, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand, linux-mm
In-Reply-To: <CAHk-=wh=vzhiDSNaLJdmjkhLqevB8+rhE49pqh0uBwhsV=1ccQ@mail.gmail.com>
Hi Linus, David,
On Wed, Jul 10, 2024 at 10:07:03PM -0700, Linus Torvalds wrote:
> The other approach might be to just let all the dirty handling happen
> - make droppable pages have a "page->mapping" (and not be anonymous),
> and have the mapping->a_ops->writepage() just always return success
> immediately.
When I was working on this patchset this year with the syscall, this is
similar somewhat to the initial approach I was taking with setting up a
special mapping. It turned into kind of a mess and I couldn't get it
working. There's a lot of functionality built around anonymous pages
that would need to be duplicated (I think?). I'll revisit it if need be,
but let's see if I can make avoiding the dirty bit propagation work.
> It's mainly the pte_dirty games in mm/vmscan.c that does it
> (walk_pte_range), but also the tear-down in mm/memory.c
> (zap_present_folio_ptes). Possibly others that I didn't think of.
>
> Both do have access to the vma, although in the case of
> walk_pte_range() we don't actually pass it down because we haven't
> needed it).
Actually, it's there hanging out in args->vma, and the function makes
use of that member already. So not so bad.
>
> There's also page_vma_mkclean_one(), try_to_unmap_one() and
> try_to_migrate_one(). And possibly many others I haven't even thought
> about.
>
> So quite a few places that do that "transfer dirty bit from pte to folio".
Alright, an hour later of fiddling, and it doesn't actually work (yet?)
-- the selftest fails. A diff follows below.
So, hmm... The swapbacked thing really seemed so simple... I wonder if
there's a way of recovering that.
Jason
diff --git a/mm/gup.c b/mm/gup.c
index ca0f5cedce9b..38745cc4fa06 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -990,7 +990,8 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
}
if (flags & FOLL_TOUCH) {
if ((flags & FOLL_WRITE) &&
- !pte_dirty(pte) && !PageDirty(page))
+ !pte_dirty(pte) && !PageDirty(page) &&
+ !(vma->vm_flags & VM_DROPPABLE))
set_page_dirty(page);
/*
* pte_mkyoung() would be more correct here, but atomic care
diff --git a/mm/ksm.c b/mm/ksm.c
index 34c4820e0d3d..2401fc4203ba 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1339,7 +1339,7 @@ static int write_protect_page(struct vm_area_struct *vma, struct folio *folio,
goto out_unlock;
}
- if (pte_dirty(entry))
+ if (pte_dirty(entry) && !(vma->vm_flags & VM_DROPPABLE))
folio_mark_dirty(folio);
entry = pte_mkclean(entry);
@@ -1518,7 +1518,7 @@ static int try_to_merge_one_page(struct vm_area_struct *vma,
* Page reclaim just frees a clean page with no dirty
* ptes: make sure that the ksm page would be swapped.
*/
- if (!PageDirty(page))
+ if (!PageDirty(page) && !(vma->vm_flags & VM_DROPPABLE))
SetPageDirty(page);
err = 0;
} else if (pages_identical(page, kpage))
diff --git a/mm/memory.c b/mm/memory.c
index d10e616d7389..6a02d16309be 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1479,7 +1479,7 @@ static __always_inline void zap_present_folio_ptes(struct mmu_gather *tlb,
if (!folio_test_anon(folio)) {
ptent = get_and_clear_full_ptes(mm, addr, pte, nr, tlb->fullmm);
- if (pte_dirty(ptent)) {
+ if (pte_dirty(ptent) && !(vma->vm_flags & VM_DROPPABLE)) {
folio_mark_dirty(folio);
if (tlb_delay_rmap(tlb)) {
delay_rmap = true;
@@ -6140,7 +6140,8 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
if (write) {
copy_to_user_page(vma, page, addr,
maddr + offset, buf, bytes);
- set_page_dirty_lock(page);
+ if (!(vma->vm_flags & VM_DROPPABLE))
+ set_page_dirty_lock(page);
} else {
copy_from_user_page(vma, page, addr,
buf, maddr + offset, bytes);
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index aecc71972a87..72d3f8eaae6e 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -216,7 +216,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
migrate->cpages++;
/* Set the dirty flag on the folio now the pte is gone. */
- if (pte_dirty(pte))
+ if (pte_dirty(pte) && !(vma->vm_flags & VM_DROPPABLE))
folio_mark_dirty(folio);
/* Setup special migration page table entry */
diff --git a/mm/rmap.c b/mm/rmap.c
index 1f9b5a9cb121..1688d06bb617 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1397,12 +1397,7 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
VM_BUG_ON_VMA(address < vma->vm_start ||
address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
- /*
- * VM_DROPPABLE mappings don't swap; instead they're just dropped when
- * under memory pressure.
- */
- if (!(vma->vm_flags & VM_DROPPABLE))
- __folio_set_swapbacked(folio);
+ __folio_set_swapbacked(folio);
__folio_set_anon(folio, vma, address, true);
if (likely(!folio_test_large(folio))) {
@@ -1777,7 +1772,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
pte_install_uffd_wp_if_needed(vma, address, pvmw.pte, pteval);
/* Set the dirty flag on the folio now the pte is gone. */
- if (pte_dirty(pteval))
+ if (pte_dirty(pteval) && !(vma->vm_flags & VM_DROPPABLE))
folio_mark_dirty(folio);
/* Update high watermark before we lower rss */
@@ -1822,7 +1817,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
}
/* MADV_FREE page check */
- if (!folio_test_swapbacked(folio)) {
+ if (!folio_test_swapbacked(folio) || (vma->vm_flags & VM_DROPPABLE)) {
int ref_count, map_count;
/*
@@ -1846,13 +1841,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* plus the rmap(s) (dropped by discard:).
*/
if (ref_count == 1 + map_count &&
- (!folio_test_dirty(folio) ||
- /*
- * Unlike MADV_FREE mappings, VM_DROPPABLE
- * ones can be dropped even if they've
- * been dirtied.
- */
- (vma->vm_flags & VM_DROPPABLE))) {
+ !folio_test_dirty(folio)) {
dec_mm_counter(mm, MM_ANONPAGES);
goto discard;
}
@@ -1862,12 +1851,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* discarded. Remap the page to page table.
*/
set_pte_at(mm, address, pvmw.pte, pteval);
- /*
- * Unlike MADV_FREE mappings, VM_DROPPABLE ones
- * never get swap backed on failure to drop.
- */
- if (!(vma->vm_flags & VM_DROPPABLE))
- folio_set_swapbacked(folio);
+ folio_set_swapbacked(folio);
ret = false;
page_vma_mapped_walk_done(&pvmw);
break;
@@ -2151,7 +2135,7 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
}
/* Set the dirty flag on the folio now the pte is gone. */
- if (pte_dirty(pteval))
+ if (pte_dirty(pteval) && !(vma->vm_flags & VM_DROPPABLE))
folio_mark_dirty(folio);
/* Update high watermark before we lower rss */
@@ -2397,7 +2381,7 @@ static bool page_make_device_exclusive_one(struct folio *folio,
pteval = ptep_clear_flush(vma, address, pvmw.pte);
/* Set the dirty flag on the folio now the pte is gone. */
- if (pte_dirty(pteval))
+ if (pte_dirty(pteval) && !(vma->vm_flags & VM_DROPPABLE))
folio_mark_dirty(folio);
/*
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2e34de9cd0d4..cf5b26bd067a 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3396,6 +3396,7 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
walk->mm_stats[MM_LEAF_YOUNG]++;
if (pte_dirty(ptent) && !folio_test_dirty(folio) &&
+ !(args->vma->vm_flags & VM_DROPPABLE) &&
!(folio_test_anon(folio) && folio_test_swapbacked(folio) &&
!folio_test_swapcache(folio)))
folio_mark_dirty(folio);
@@ -3476,6 +3477,7 @@ static void walk_pmd_range_locked(pud_t *pud, unsigned long addr, struct vm_area
walk->mm_stats[MM_LEAF_YOUNG]++;
if (pmd_dirty(pmd[i]) && !folio_test_dirty(folio) &&
+ !(vma->vm_flags && VM_DROPPABLE) &&
!(folio_test_anon(folio) && folio_test_swapbacked(folio) &&
!folio_test_swapcache(folio)))
folio_mark_dirty(folio);
@@ -4076,6 +4078,7 @@ void lru_gen_look_around(struct page_vma_mapped_walk *pvmw)
young++;
if (pte_dirty(ptent) && !folio_test_dirty(folio) &&
+ !(vma->vm_flags & VM_DROPPABLE) &&
!(folio_test_anon(folio) && folio_test_swapbacked(folio) &&
!folio_test_swapcache(folio)))
folio_mark_dirty(folio);
^ permalink raw reply related
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-11 8:57 UTC (permalink / raw)
To: Kees Cook
Cc: Jeff Xu, Steve Dower, Al Viro, Christian Brauner, Linus Torvalds,
Paul Moore, Theodore Ts'o, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Grubb,
Thibaut Sautereau, Vincent Strubel, Xiaoming Ni, Yin Fengwei,
kernel-hardening, linux-api, linux-fsdevel, linux-integrity,
linux-kernel, linux-security-module
In-Reply-To: <202407100921.687BE1A6@keescook>
On Wed, Jul 10, 2024 at 09:26:14AM -0700, Kees Cook wrote:
> On Wed, Jul 10, 2024 at 11:58:25AM +0200, Mickaël Salaün wrote:
> > Here is another proposal:
> >
> > We can change a bit the semantic by making it the norm to always check
> > file executability with AT_CHECK, and using the securebits to restrict
> > file interpretation and/or command injection (e.g. user supplied shell
> > commands). Non-executable checked files can be reported/logged at the
> > kernel level, with audit, configured by sysadmins.
> >
> > New securebits (feel free to propose better names):
> >
> > - SECBIT_EXEC_RESTRICT_FILE: requires AT_CHECK to pass.
>
> Would you want the enforcement of this bit done by userspace or the
> kernel?
>
> IIUC, userspace would always perform AT_CHECK regardless of
> SECBIT_EXEC_RESTRICT_FILE, and then which would happen?
>
> 1) userspace would ignore errors from AT_CHECK when
> SECBIT_EXEC_RESTRICT_FILE is unset
Yes, that's the idea.
>
> or
>
> 2) kernel would allow all AT_CHECK when SECBIT_EXEC_RESTRICT_FILE is
> unset
>
> I suspect 1 is best and what you intend, given that
> SECBIT_EXEC_DENY_INTERACTIVE can only be enforced by userspace.
Indeed. We don't want AT_CHECK's behavior to change according to
securebits.
>
> > - SECBIT_EXEC_DENY_INTERACTIVE: deny any command injection via
> > command line arguments, environment variables, or configuration files.
> > This should be ignored by dynamic linkers. We could also have an
> > allow-list of shells for which this bit is not set, managed by an
> > LSM's policy, if the native securebits scoping approach is not enough.
> >
> > Different modes for script interpreters:
> >
> > 1. RESTRICT_FILE=0 DENY_INTERACTIVE=0 (default)
> > Always interpret scripts, and allow arbitrary user commands.
> > => No threat, everyone and everything is trusted, but we can get
> > ahead of potential issues with logs to prepare for a migration to a
> > restrictive mode.
> >
> > 2. RESTRICT_FILE=1 DENY_INTERACTIVE=0
> > Deny script interpretation if they are not executable, and allow
> > arbitrary user commands.
> > => Threat: (potential) malicious scripts run by trusted (and not
> > fooled) users. That could protect against unintended script
> > executions (e.g. sh /tmp/*.sh).
> > ==> Makes sense for (semi-restricted) user sessions.
> >
> > 3. RESTRICT_FILE=1 DENY_INTERACTIVE=1
> > Deny script interpretation if they are not executable, and also deny
> > any arbitrary user commands.
> > => Threat: malicious scripts run by untrusted users.
> > ==> Makes sense for system services executing scripts.
> >
> > 4. RESTRICT_FILE=0 DENY_INTERACTIVE=1
> > Always interpret scripts, but deny arbitrary user commands.
> > => Goal: monitor/measure/assess script content (e.g. with IMA/EVM) in
> > a system where the access rights are not (yet) ready. Arbitrary
> > user commands would be much more difficult to monitor.
> > ==> First step of restricting system services that should not
> > directly pass arbitrary commands to shells.
>
> I like these bits!
Good! Jeff, Steve, Florian, Matt, others, what do you think?
^ 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