Linux userland API discussions
 help / color / mirror / Atom feed
* 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: 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: 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: 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: 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: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: 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 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: 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: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: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: 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: 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: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: 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: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: 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: 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: 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: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: 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: 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: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: David Hildenbrand @ 2024-07-11 20:59 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: <CAOUHufZuMdN31WnbwctyFv+o8nAfVBaiHZa9Ud_cz6QAoNQHxw@mail.gmail.com>

On 11.07.24 21:58, Yu Zhao wrote:
> 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().
> 

Ahh, got it, thanks!

-- 
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 20:59 UTC (permalink / raw)
  To: Jason A. Donenfeld, Yu Zhao
  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: <ZpA-iuAItDxBSfBS@zx2c4.com>

On 11.07.24 22:20, Jason A. Donenfeld wrote:
> 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.

Yes!

-- 
Cheers,

David / dhildenb


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox