All of lore.kernel.org
 help / color / mirror / Atom feed
* + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
@ 2023-09-29 19:08 Andrew Morton
  2023-10-05  8:12 ` David Hildenbrand
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2023-09-29 19:08 UTC (permalink / raw)
  To: mm-commits, ziy, yuzhao, ying.huang, willy, vbabka, shy828301,
	rientjes, mcgrof, kirill.shutemov, jhubbard, itaru.kitayama,
	hughd, fengwei.yin, david, catalin.marinas, anshuman.khandual,
	ryan.roberts, akpm


The patch titled
     Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
has been added to the -mm mm-unstable branch.  Its filename is
     mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will later appear in the mm-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Ryan Roberts <ryan.roberts@arm.com>
Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
Date: Fri, 29 Sep 2023 12:44:13 +0100

In preparation for anonymous large folio support, improve
folio_add_new_anon_rmap() to allow a non-pmd-mappable, large folio to be
passed to it.  In this case, all contained pages are accounted using the
order-0 folio (or base page) scheme.

Link: https://lkml.kernel.org/r/20230929114421.3761121-3-ryan.roberts@arm.com
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: Yu Zhao <yuzhao@google.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/rmap.c |   26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

--- a/mm/rmap.c~mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap
+++ a/mm/rmap.c
@@ -1321,26 +1321,40 @@ void page_add_anon_rmap(struct page *pag
  * This means the inc-and-test can be bypassed.
  * The folio does not have to be locked.
  *
- * If the folio is large, it is accounted as a THP.  As the folio
+ * If the folio is pmd-mappable, it is accounted as a THP.  As the folio
  * is new, it's assumed to be mapped exclusively by a single process.
  */
 void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
 		unsigned long address)
 {
-	int nr;
+	int nr = folio_nr_pages(folio);
 
-	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
+	VM_BUG_ON_VMA(address < vma->vm_start ||
+			address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
 	__folio_set_swapbacked(folio);
 
-	if (likely(!folio_test_pmd_mappable(folio))) {
+	if (likely(!folio_test_large(folio))) {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_mapcount, 0);
-		nr = 1;
+		__page_set_anon_rmap(folio, &folio->page, vma, address, 1);
+	} else if (!folio_test_pmd_mappable(folio)) {
+		int i;
+
+		for (i = 0; i < nr; i++) {
+			struct page *page = folio_page(folio, i);
+
+			/* increment count (starts at -1) */
+			atomic_set(&page->_mapcount, 0);
+			__page_set_anon_rmap(folio, page, vma,
+					address + (i << PAGE_SHIFT), 1);
+		}
+
+		atomic_set(&folio->_nr_pages_mapped, nr);
 	} else {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_entire_mapcount, 0);
 		atomic_set(&folio->_nr_pages_mapped, COMPOUND_MAPPED);
-		nr = folio_nr_pages(folio);
+		__page_set_anon_rmap(folio, &folio->page, vma, address, 1);
 		__lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr);
 	}
 
_

Patches currently in -mm which might be from ryan.roberts@arm.com are

mm-hugetlb-add-huge-page-size-param-to-set_huge_pte_at.patch
arm64-hugetlb-fix-set_huge_pte_at-to-work-with-all-swap-entries.patch
mm-allow-deferred-splitting-of-arbitrary-anon-large-folios.patch
mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
mm-thp-account-pte-mapped-anonymous-thp-usage.patch
mm-thp-introduce-anon_orders-and-anon_always_mask-sysfs-files.patch
mm-thp-extend-thp-to-allocate-anonymous-large-folios.patch
mm-thp-add-recommend-option-for-anon_orders.patch
arm64-mm-override-arch_wants_pte_order.patch
selftests-mm-cow-generalize-do_run_with_thp-helper.patch
selftests-mm-cow-add-tests-for-small-order-anon-thp.patch


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-09-29 19:08 + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch Andrew Morton
@ 2023-10-05  8:12 ` David Hildenbrand
  2023-10-05 19:23   ` Yu Zhao
  0 siblings, 1 reply; 10+ messages in thread
From: David Hildenbrand @ 2023-10-05  8:12 UTC (permalink / raw)
  To: Andrew Morton, mm-commits, ziy, yuzhao, ying.huang, willy,
	vbabka, shy828301, rientjes, mcgrof, kirill.shutemov, jhubbard,
	itaru.kitayama, hughd, fengwei.yin, catalin.marinas,
	anshuman.khandual, ryan.roberts

On 29.09.23 21:08, Andrew Morton wrote:
> 
> The patch titled
>       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
> has been added to the -mm mm-unstable branch.  Its filename is
>       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
> 

Andrew, please don't move these patches to stable before we have a 
couple of acks from people involved in the discussions.

I'm confident that we'll get them into 6.8. Not so sure about 6.7.

Besides, there are still other things to be sorted out to make this 
actually consumable: As the cover letter reads:

"
NOTE: These changes should not be merged until the prerequisites are 
complete.
These are in progress and tracked at [7].
"

Again, I am happy to see them in mm-unstable early.

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-05  8:12 ` David Hildenbrand
@ 2023-10-05 19:23   ` Yu Zhao
  2023-10-06  8:53     ` Ryan Roberts
  2023-10-06 19:06     ` David Hildenbrand
  0 siblings, 2 replies; 10+ messages in thread
From: Yu Zhao @ 2023-10-05 19:23 UTC (permalink / raw)
  To: David Hildenbrand, ryan.roberts, Andrew Morton
  Cc: mm-commits, ziy, ying.huang, willy, vbabka, shy828301, rientjes,
	mcgrof, kirill.shutemov, jhubbard, itaru.kitayama, hughd,
	fengwei.yin, catalin.marinas, anshuman.khandual

On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 29.09.23 21:08, Andrew Morton wrote:
> >
> > The patch titled
> >       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
> > has been added to the -mm mm-unstable branch.  Its filename is
> >       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
> >
>
> Andrew, please don't move these patches to stable before we have a
> couple of acks from people involved in the discussions.
>
> I'm confident that we'll get them into 6.8.

I am not :)

As I repeated, the priorities seem wrong to me. There are many pending
work items that don't involve any ABI changes, and IMO, we should
focus on them before we even start discussing ABI changes.

I've been quiet  because I don't think opinion based discussions are
essential to nailing down ABIs. As I suggested, I prefer a Kconfig
option as the first step so that people can play with it. After we
have collected field data, then we'd be better at coming up with
suitable ABIs.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-05 19:23   ` Yu Zhao
@ 2023-10-06  8:53     ` Ryan Roberts
  2023-10-06 15:24       ` Yu Zhao
  2023-10-06 19:06     ` David Hildenbrand
  1 sibling, 1 reply; 10+ messages in thread
From: Ryan Roberts @ 2023-10-06  8:53 UTC (permalink / raw)
  To: Yu Zhao, David Hildenbrand, Andrew Morton
  Cc: mm-commits, ziy, ying.huang, willy, vbabka, shy828301, rientjes,
	mcgrof, kirill.shutemov, jhubbard, itaru.kitayama, hughd,
	fengwei.yin, catalin.marinas, anshuman.khandual

On 05/10/2023 20:23, Yu Zhao wrote:
> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 29.09.23 21:08, Andrew Morton wrote:
>>>
>>> The patch titled
>>>       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
>>>
>>
>> Andrew, please don't move these patches to stable before we have a
>> couple of acks from people involved in the discussions.
>>
>> I'm confident that we'll get them into 6.8.
> 
> I am not :)

I don't find that comment particularly helpful; Are you implying a NAK? If so,
some rationale and actionable suggestions would help.

> 
> As I repeated, the priorities seem wrong to me. There are many pending
> work items that don't involve any ABI changes

Are you referring to the prerequisite list [1]? If so, then its my understanding
that these work items are all in progress:

  - David is working on "shared vs exclusive mappings"
  - Zi Yan has posted an RFC for compaction
  - Yin Fengwei's mlock series is now in mm-stable
  - Yin Fengwei's madvise series is in 6.6
  - I've reworked and posted a series for deferred_split_folio; although I've
    deprioritied it because you said it wasn't really a pre-requisite.
  - numa balancing depends on David's "shared vs exclusive mappings" work
  - I've started looking at "large folios in swap cache" in the background,
    because I'm seeing some slow down with large folios, but we also agreed that
    wasn't a prerequisite

Or if that's not what you are referring to, then perhaps you are referring to
your comments against v5, which I thought were addressed in v6:

  - Removal of the hardcoded order policy [2]; which is removed and replaced by
    the flexibility of user-selectable orders (this has proven very useful for
    my testing)

, and IMO, we should
> focus on them before we even start discussing ABI changes.

I don't see why the prerequisites can't be done in parallel. I don't see any
dependency between them and the ABI.


> 
> I've been quiet  because I don't think opinion based discussions are
> essential to nailing down ABIs. 

We had 2 mm alignment meetings focussing on this topic, where the resounding
conclusion that I heard was that at minimum, we need a way to enable/disable
this at runtime. I'll admit that what I've posted goes a bit further than that,
but I was hoping we could argue about that in the context of the patch set. But
I definitely think we have moved on from the "we don't need any user ABI for
now" position.

As I suggested, I prefer a Kconfig
> option as the first step so that people can play with it. After we
> have collected field data, then we'd be better at coming up with
> suitable ABIs.

If you want to make it a compile time option only, and use it for testing, then
why do you need it upstream at all? Why not just take the patches and apply
them? Then we could use your data to help guide the upstreaming (I have asked
this before, but never saw a response [3]). Personally though, I would rather
not let perfection get in the way of good enough - I can see real world wins
with this and would prefer to start getting it into mainline incrementally,
sooner rather than later.

Thanks,
Ryan


[1] https://lore.kernel.org/linux-mm/f8d47176-03a8-99bf-a813-b5942830fd73@arm.com/
[2]
https://lore.kernel.org/linux-mm/CAOUHufbUGwc2XvZOBmTCzMsOHxP-eLB60EdysKYzrkRMScOyMg@mail.gmail.com/
[3] https://lore.kernel.org/linux-mm/e80cd2e6-6f7c-4337-a170-152926863290@arm.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-06  8:53     ` Ryan Roberts
@ 2023-10-06 15:24       ` Yu Zhao
  2023-10-06 19:08         ` David Hildenbrand
  2023-10-09  7:59         ` Ryan Roberts
  0 siblings, 2 replies; 10+ messages in thread
From: Yu Zhao @ 2023-10-06 15:24 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: David Hildenbrand, Andrew Morton, mm-commits, ziy, ying.huang,
	willy, vbabka, shy828301, rientjes, mcgrof, kirill.shutemov,
	jhubbard, itaru.kitayama, hughd, fengwei.yin, catalin.marinas,
	anshuman.khandual

On Fri, Oct 6, 2023 at 2:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 05/10/2023 20:23, Yu Zhao wrote:
> > On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 29.09.23 21:08, Andrew Morton wrote:
> >>>
> >>> The patch titled
> >>>       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
> >>> has been added to the -mm mm-unstable branch.  Its filename is
> >>>       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
> >>>
> >>
> >> Andrew, please don't move these patches to stable before we have a
> >> couple of acks from people involved in the discussions.
> >>
> >> I'm confident that we'll get them into 6.8.
> >
> > I am not :)
>
> I don't find that comment particularly helpful; Are you implying a NAK? If so,
> some rationale and actionable suggestions would help.

From my POV, there have been some random changes between each version
of your patchset:
1. v3 *introduced* a boot parameter `flexthp_unhinted_max`, which
   *disappeared* in later versions.
2. v4 *introduced* a random constant ANON_FOLIO_MAX_ORDER_UNHINTED,
   which *disappeared* in v6.
3. v5 is really close to what I think can be merged -- my only
   objection is to ANON_FOLIO_MAX_ORDER_UNHINTED, I've never heard
   *a good reason* why that random constant should sit in core mm.
4. v6 *introduced* a sysfs ABI -- we had a couple of alignment meetings
   about how it might look but there was *never a consensus*.

FWICT, you haven't made up your mind. So I'm not convinced this series
is close to being ready to introduce an ABI. This is NOT a NAK, just a
ton of concerns with your current approach.

And I'm not sure how I could be more clear on how we can move forward
progress [1][2][3]. Some suggestions would also help.

[1] v3 https://lore.kernel.org/linux-mm/CAOUHufaDfJwF_-zb6zV5COG-KaaGcSyrNmbaEzaWz2UjcGGgHQ@mail.gmail.com/
[2] v4 https://lore.kernel.org/linux-mm/CAOUHufackQzy+yXOzaej+G6DNYK-k9GAUHAK6Vq79BFHr7KwAQ@mail.gmail.com/
[3] v5 https://lore.kernel.org/linux-mm/CAOUHufbUGwc2XvZOBmTCzMsOHxP-eLB60EdysKYzrkRMScOyMg@mail.gmail.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-05 19:23   ` Yu Zhao
  2023-10-06  8:53     ` Ryan Roberts
@ 2023-10-06 19:06     ` David Hildenbrand
  1 sibling, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2023-10-06 19:06 UTC (permalink / raw)
  To: Yu Zhao, ryan.roberts, Andrew Morton
  Cc: mm-commits, ziy, ying.huang, willy, vbabka, shy828301, rientjes,
	mcgrof, kirill.shutemov, jhubbard, itaru.kitayama, hughd,
	fengwei.yin, catalin.marinas, anshuman.khandual

On 05.10.23 21:23, Yu Zhao wrote:
> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 29.09.23 21:08, Andrew Morton wrote:
>>>
>>> The patch titled
>>>        Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>        mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
>>>
>>
>> Andrew, please don't move these patches to stable before we have a
>> couple of acks from people involved in the discussions.
>>
>> I'm confident that we'll get them into 6.8.
> 
> I am not :)

Well, there is still quite some time for 6.8, not so much for 6.7 :) 
Even if we end up with something minimal.

But I'm afraid (and previously expressed) a pure kconfig won't be 
sufficient, at least not for distributions.

I did not look into the details of the latest proposal, unfortunately, 
but I'll spoiler that exposing orders and bitmaps to users is not what I 
would immediately classify as a nice interface :)

Anyhow, let's not get distracted ...

> 
> As I repeated, the priorities seem wrong to me. There are many pending
> work items that don't involve any ABI changes, and IMO, we should
> focus on them before we even start discussing ABI changes.

I tend to agree: part of me being slower on feedback to the latest 
series is that I rather spent time trying to sort out some of the 
pending work items -- well, one, to be precise, which is challenging enough.

That's also one of the reason I don't see a need to rush this in: it 
would be in a state where no distribution could consider enabling it. So 
as a distribution guy, I don't see that much value in that ;)

One could certainly say that some of the pending items are not relevant 
for this to get merged and used to some degree. But again, from a 
distribution aspect, I think some of the pending items are a *requirement*.

> 
> I've been quiet  because I don't think opinion based discussions are
> essential to nailing down ABIs. As I suggested, I prefer a Kconfig

Please don't be quiet.

> option as the first step so that people can play with it. After we
> have collected field data, then we'd be better at coming up with
> suitable ABIs.

As I expressed, for me a magical toggle to enable/disable it (and if 
enabled, obey the existing THP rules) would be good enough for the first 
shot. It won't be sufficient for some workloads we discussed in the 
meetings, though.

So I see how Ryan tried to find something that is more configurable. 
Ideally, we'd probably have something that easily lets one enable a sane 
default (which we can tweak as we go), but still let people that want to 
configure it, just configure it. IMHO, nothing wrong with that. I might 
have some ideas, but again, lower priority for me at this point.

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-06 15:24       ` Yu Zhao
@ 2023-10-06 19:08         ` David Hildenbrand
  2023-10-09  7:59         ` Ryan Roberts
  1 sibling, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2023-10-06 19:08 UTC (permalink / raw)
  To: Yu Zhao, Ryan Roberts
  Cc: Andrew Morton, mm-commits, ziy, ying.huang, willy, vbabka,
	shy828301, rientjes, mcgrof, kirill.shutemov, jhubbard,
	itaru.kitayama, hughd, fengwei.yin, catalin.marinas,
	anshuman.khandual

On 06.10.23 17:24, Yu Zhao wrote:
> On Fri, Oct 6, 2023 at 2:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 05/10/2023 20:23, Yu Zhao wrote:
>>> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>> On 29.09.23 21:08, Andrew Morton wrote:
>>>>>
>>>>> The patch titled
>>>>>        Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
>>>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>>>        mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
>>>>>
>>>>
>>>> Andrew, please don't move these patches to stable before we have a
>>>> couple of acks from people involved in the discussions.
>>>>
>>>> I'm confident that we'll get them into 6.8.
>>>
>>> I am not :)
>>
>> I don't find that comment particularly helpful; Are you implying a NAK? If so,
>> some rationale and actionable suggestions would help.
> 
>  From my POV, there have been some random changes between each version
> of your patchset:

There have been significant changes, which is one reason why I would 
prefer this to actually get reviewed and ack'ed ;)

Sorry for stepping on the brake pedal ...

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
  2023-10-06 15:24       ` Yu Zhao
  2023-10-06 19:08         ` David Hildenbrand
@ 2023-10-09  7:59         ` Ryan Roberts
  1 sibling, 0 replies; 10+ messages in thread
From: Ryan Roberts @ 2023-10-09  7:59 UTC (permalink / raw)
  To: Yu Zhao
  Cc: David Hildenbrand, Andrew Morton, mm-commits, ziy, ying.huang,
	willy, vbabka, shy828301, rientjes, mcgrof, kirill.shutemov,
	jhubbard, itaru.kitayama, hughd, fengwei.yin, catalin.marinas,
	anshuman.khandual

On 06/10/2023 16:24, Yu Zhao wrote:
> On Fri, Oct 6, 2023 at 2:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 05/10/2023 20:23, Yu Zhao wrote:
>>> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>> On 29.09.23 21:08, Andrew Morton wrote:
>>>>>
>>>>> The patch titled
>>>>>       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
>>>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>>>       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
>>>>>
>>>>
>>>> Andrew, please don't move these patches to stable before we have a
>>>> couple of acks from people involved in the discussions.
>>>>
>>>> I'm confident that we'll get them into 6.8.
>>>
>>> I am not :)
>>
>> I don't find that comment particularly helpful; Are you implying a NAK? If so,
>> some rationale and actionable suggestions would help.
> 
> From my POV, there have been some random changes between each version
> of your patchset:
> 1. v3 *introduced* a boot parameter `flexthp_unhinted_max`, which
>    *disappeared* in later versions.
> 2. v4 *introduced* a random constant ANON_FOLIO_MAX_ORDER_UNHINTED,
>    which *disappeared* in v6.
> 3. v5 is really close to what I think can be merged -- my only
>    objection is to ANON_FOLIO_MAX_ORDER_UNHINTED, I've never heard
>    *a good reason* why that random constant should sit in core mm.
> 4. v6 *introduced* a sysfs ABI -- we had a couple of alignment meetings
>    about how it might look but there was *never a consensus*.

A couple of points; It was my understanding that the preferred way of working
was to turn up with real code, that could be used as context for discussion.
That's the approach I've been taking. If this is not how I should be doing it,
then perhaps you could point me to the preferred approach?

For me, none of these changes are "random"; they are all attempts to solve the
problem of internal fragmentation when the HW-preferred order is "too big" to
have any reasonable confidence that we won't be wasting loads of memory. And
1,2,3 was trying to do this without introducing any ABI, as per your preference.
The reasons for their removal (and moving on to another approach) was due to the
feedback that you linked below.

I appreciate in hindsight, that you have found this meandering frustrating. But
if nothing else, it has been beneficial to help me understand the expected
standards - hopefully I'm learning.

> 
> FWICT, you haven't made up your mind. So I'm not convinced this series
> is close to being ready to introduce an ABI. This is NOT a NAK, just a
> ton of concerns with your current approach.
> 
> And I'm not sure how I could be more clear on how we can move forward
> progress [1][2][3]. Some suggestions would also help.

I appreciate the feedback you have given so far, and as far as I'm concerned,
I've listened to it; the changes you enumerated above are a direct results of
your feedback in the links below. And I do hope you continue to provide feedback
and help guide me towards a better solution.

But, there are questions I asked in the previous email which you did not answer.
If you are able to answer them, that would help me be better aligned with your
thinking. Namely:

  - You said the priorities are wrong and there are many pending work items I
    should be looking at first - what were you referring to here?
  - If you want a compile time option for testing, what is preventing you doing
    testing by merging in the patches? What is the benefit of it being upstream?

I see that both you and David have now responded directly to the series - thanks
to both of you. I will read and respond to that shortly, and let's continue the
conversation!


Thanks,
Ryan


> 
> [1] v3 https://lore.kernel.org/linux-mm/CAOUHufaDfJwF_-zb6zV5COG-KaaGcSyrNmbaEzaWz2UjcGGgHQ@mail.gmail.com/
> [2] v4 https://lore.kernel.org/linux-mm/CAOUHufackQzy+yXOzaej+G6DNYK-k9GAUHAK6Vq79BFHr7KwAQ@mail.gmail.com/
> [3] v5 https://lore.kernel.org/linux-mm/CAOUHufbUGwc2XvZOBmTCzMsOHxP-eLB60EdysKYzrkRMScOyMg@mail.gmail.com/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
@ 2023-12-07 22:04 Andrew Morton
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2023-12-07 22:04 UTC (permalink / raw)
  To: mm-commits, ziy, yuzhao, ying.huang, willy, wangkefeng.wang,
	v-songbaohua, vbabka, shy828301, rientjes, mcgrof,
	kirill.shutemov, jhubbard, itaru.kitayama, hughd, fengwei.yin,
	david, catalin.marinas, apopple, anshuman.khandual, ryan.roberts,
	akpm


The patch titled
     Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
has been added to the -mm mm-unstable branch.  Its filename is
     mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will later appear in the mm-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Ryan Roberts <ryan.roberts@arm.com>
Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
Date: Thu, 7 Dec 2023 16:12:03 +0000

In preparation for supporting anonymous multi-size THP, improve
folio_add_new_anon_rmap() to allow a non-pmd-mappable, large folio to be
passed to it.  In this case, all contained pages are accounted using the
order-0 folio (or base page) scheme.

Link: https://lkml.kernel.org/r/20231207161211.2374093-3-ryan.roberts@arm.com
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: Yu Zhao <yuzhao@google.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Barry Song <v-songbaohua@oppo.com>
Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Tested-by: John Hubbard <jhubbard@nvidia.com>
Cc: Alistair Popple <apopple@nvidia.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: David Rientjes <rientjes@google.com>
Cc: "Huang, Ying" <ying.huang@intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/rmap.c |   28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

--- a/mm/rmap.c~mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap
+++ a/mm/rmap.c
@@ -1352,32 +1352,44 @@ void page_add_anon_rmap(struct page *pag
  * This means the inc-and-test can be bypassed.
  * The folio does not have to be locked.
  *
- * If the folio is large, it is accounted as a THP.  As the folio
+ * If the folio is pmd-mappable, it is accounted as a THP.  As the folio
  * is new, it's assumed to be mapped exclusively by a single process.
  */
 void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
 		unsigned long address)
 {
-	int nr;
+	int nr = folio_nr_pages(folio);
 
-	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
+	VM_BUG_ON_VMA(address < vma->vm_start ||
+			address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
 	__folio_set_swapbacked(folio);
+	__folio_set_anon(folio, vma, address, true);
 
-	if (likely(!folio_test_pmd_mappable(folio))) {
+	if (likely(!folio_test_large(folio))) {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_mapcount, 0);
-		nr = 1;
+		SetPageAnonExclusive(&folio->page);
+	} else if (!folio_test_pmd_mappable(folio)) {
+		int i;
+
+		for (i = 0; i < nr; i++) {
+			struct page *page = folio_page(folio, i);
+
+			/* increment count (starts at -1) */
+			atomic_set(&page->_mapcount, 0);
+			SetPageAnonExclusive(page);
+		}
+
+		atomic_set(&folio->_nr_pages_mapped, nr);
 	} else {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_entire_mapcount, 0);
 		atomic_set(&folio->_nr_pages_mapped, COMPOUND_MAPPED);
-		nr = folio_nr_pages(folio);
+		SetPageAnonExclusive(&folio->page);
 		__lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr);
 	}
 
 	__lruvec_stat_mod_folio(folio, NR_ANON_MAPPED, nr);
-	__folio_set_anon(folio, vma, address, true);
-	SetPageAnonExclusive(&folio->page);
 }
 
 /**
_

Patches currently in -mm which might be from ryan.roberts@arm.com are

mm-readahead-do-not-allow-order-1-folio.patch
mm-allow-deferred-splitting-of-arbitrary-anon-large-folios.patch
mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
mm-thp-introduce-multi-size-thp-sysfs-interface.patch
mm-thp-support-allocation-of-anonymous-multi-size-thp.patch
selftests-mm-kugepaged-restore-thp-settings-at-exit.patch
selftests-mm-factor-out-thp-settings-management.patch
selftests-mm-support-multi-size-thp-interface-in-thp_settings.patch
selftests-mm-khugepaged-enlighten-for-multi-size-thp.patch
selftests-mm-cow-generalize-do_run_with_thp-helper.patch
selftests-mm-cow-add-tests-for-anonymous-multi-size-thp.patch


^ permalink raw reply	[flat|nested] 10+ messages in thread

* + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch
@ 2023-08-10 16:31 Andrew Morton
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2023-08-10 16:31 UTC (permalink / raw)
  To: mm-commits, ziy, yuzhao, ying.huang, willy, shy828301, mcgrof,
	kirill.shutemov, itaru.kitayama, fengwei.yin, david,
	catalin.marinas, anshuman.khandual, ryan.roberts, akpm


The patch titled
     Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
has been added to the -mm mm-unstable branch.  Its filename is
     mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch

This patch will later appear in the mm-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Ryan Roberts <ryan.roberts@arm.com>
Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
Date: Thu, 10 Aug 2023 15:29:39 +0100

In preparation for LARGE_ANON_FOLIO support, improve
folio_add_new_anon_rmap() to allow a non-pmd-mappable, large folio to be
passed to it.  In this case, all contained pages are accounted using the
order-0 folio (or base page) scheme.

Link: https://lkml.kernel.org/r/20230810142942.3169679-3-ryan.roberts@arm.com
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: Yu Zhao <yuzhao@google.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Huang, Ying <ying.huang@intel.com>
Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/rmap.c |   27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

--- a/mm/rmap.c~mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap
+++ a/mm/rmap.c
@@ -1266,31 +1266,44 @@ void page_add_anon_rmap(struct page *pag
  * This means the inc-and-test can be bypassed.
  * The folio does not have to be locked.
  *
- * If the folio is large, it is accounted as a THP.  As the folio
+ * If the folio is pmd-mappable, it is accounted as a THP.  As the folio
  * is new, it's assumed to be mapped exclusively by a single process.
  */
 void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
 		unsigned long address)
 {
-	int nr;
+	int nr = folio_nr_pages(folio);
 
-	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
+	VM_BUG_ON_VMA(address < vma->vm_start ||
+			address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
 	__folio_set_swapbacked(folio);
 
-	if (likely(!folio_test_pmd_mappable(folio))) {
+	if (likely(!folio_test_large(folio))) {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_mapcount, 0);
-		nr = 1;
+		__page_set_anon_rmap(folio, &folio->page, vma, address, 1);
+	} else if (!folio_test_pmd_mappable(folio)) {
+		int i;
+
+		for (i = 0; i < nr; i++) {
+			struct page *page = folio_page(folio, i);
+
+			/* increment count (starts at -1) */
+			atomic_set(&page->_mapcount, 0);
+			__page_set_anon_rmap(folio, page, vma,
+					address + (i << PAGE_SHIFT), 1);
+		}
+
+		atomic_set(&folio->_nr_pages_mapped, nr);
 	} else {
 		/* increment count (starts at -1) */
 		atomic_set(&folio->_entire_mapcount, 0);
 		atomic_set(&folio->_nr_pages_mapped, COMPOUND_MAPPED);
-		nr = folio_nr_pages(folio);
+		__page_set_anon_rmap(folio, &folio->page, vma, address, 1);
 		__lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr);
 	}
 
 	__lruvec_stat_mod_folio(folio, NR_ANON_MAPPED, nr);
-	__page_set_anon_rmap(folio, &folio->page, vma, address, 1);
 }
 
 /**
_

Patches currently in -mm which might be from ryan.roberts@arm.com are

selftests-line-buffer-test-programs-stdout.patch
selftests-line-buffer-test-programs-stdout-fix.patch
selftests-mm-skip-soft-dirty-tests-on-arm64.patch
selftests-mm-enable-mrelease_test-for-arm64.patch
selftests-mm-fix-thuge-gen-test-bugs.patch
selftests-mm-va_high_addr_switch-should-skip-unsupported-arm64-configs.patch
selftests-mm-make-migration-test-robust-to-failure.patch
selftests-mm-optionally-pass-duration-to-transhuge-stress.patch
selftests-mm-run-all-tests-from-run_vmtestssh.patch
mm-allow-deferred-splitting-of-arbitrary-large-anon-folios.patch
mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
mm-large_anon_folio-for-improved-performance.patch
selftests-mm-cow-generalize-do_run_with_thp-helper.patch
selftests-mm-cow-add-large-anon-folio-tests.patch


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-12-07 22:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-29 19:08 + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch Andrew Morton
2023-10-05  8:12 ` David Hildenbrand
2023-10-05 19:23   ` Yu Zhao
2023-10-06  8:53     ` Ryan Roberts
2023-10-06 15:24       ` Yu Zhao
2023-10-06 19:08         ` David Hildenbrand
2023-10-09  7:59         ` Ryan Roberts
2023-10-06 19:06     ` David Hildenbrand
  -- strict thread matches above, loose matches on Subject: below --
2023-12-07 22:04 Andrew Morton
2023-08-10 16:31 Andrew Morton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.