[v2,0/5] variable-order, large folios for anonymous memory

  Hi All,

This is v2 of a series to implement variable order, large folios for anonymous
memory. The objective of this is to improve performance by allocating larger
chunks of memory during anonymous page faults. See [1] for background.

I've significantly reworked and simplified the patch set based on comments from
Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
VARIABLE_THP, on Yu's advice.

The last patch is for arm64 to explicitly override the default
arch_wants_pte_order() and is intended as an example. If this series is accepted
I suggest taking the first 4 patches through the mm tree and the arm64 change
could be handled through the arm64 tree separately. Neither has any build
dependency on the other.

The one area where I haven't followed Yu's advice is in the determination of the
size of folio to use. It was suggested that I have a single preferred large
order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
being existing overlapping populated PTEs, etc) then fallback immediately to
order-0. It turned out that this approach caused a performance regression in the
Speedometer benchmark. With my v1 patch, there were significant quantities of
memory which could not be placed in the 64K bucket and were instead being
allocated for the 32K and 16K buckets. With the proposed simplification, that
memory ended up using the 4K bucket, so page faults increased by 2.75x compared
to the v1 patch (although due to the 64K bucket, this number is still a bit
lower than the baseline). So instead, I continue to calculate a folio order that
is somewhere between the preferred order and 0. (See below for more details).

The patches are based on top of v6.4 plus Matthew Wilcox's set_ptes() series
[2], which is a hard dependency. I have a branch at [3].


Changes since v1 [1]
--------------------

  - removed changes to arch-dependent vma_alloc_zeroed_movable_folio()
  - replaced with arch-independent alloc_anon_folio()
      - follows THP allocation approach
  - no longer retry with intermediate orders if allocation fails
      - fallback directly to order-0
  - remove folio_add_new_anon_rmap_range() patch
      - instead add its new functionality to folio_add_new_anon_rmap()
  - remove batch-zap pte mappings optimization patch
      - remove enabler folio_remove_rmap_range() patch too
      - These offer real perf improvement so will submit separately
  - simplify Kconfig
      - single FLEXIBLE_THP option, which is independent of arch
      - depends on TRANSPARENT_HUGEPAGE
      - when enabled default to max anon folio size of 64K unless arch
        explicitly overrides
  - simplify changes to do_anonymous_page():
      - no more retry loop


Performance
-----------

Below results show 3 benchmarks; kernel compilation with 8 jobs, kernel
compilation with 80 jobs, and speedometer 2.0 (a javascript benchmark running in
Chromium). All cases are running on Ampere Altra with 1 NUMA node enabled,
Ubuntu 22.04 and XFS filesystem. Each benchmark is repeated 15 times over 5
reboots and averaged.

'anonfolio-lkml-v1' is the v1 patchset at [1]. 'anonfolio-lkml-v2' is this v2
patchset. 'anonfolio-lkml-v2-simple-order' is anonfolio-lkml-v2 but with the
order selection simplification that Yu Zhao suggested - I'm trying to justify
here why I did not follow the advice.


Kernel compilation with 8 jobs:

| kernel                         |   real-time |   kern-time |   user-time |
|:-------------------------------|------------:|------------:|------------:|
| baseline-4k                    |        0.0% |        0.0% |        0.0% |
| anonfolio-lkml-v1              |       -5.3% |      -42.9% |       -0.6% |
| anonfolio-lkml-v2-simple-order |       -4.4% |      -36.5% |       -0.4% |
| anonfolio-lkml-v2              |       -4.8% |      -38.6% |       -0.6% |

We can see that the simple-order approach is responsible for a regression of
0.4%.


Kernel compilation with 80 jobs:

| kernel                         |   real-time |   kern-time |   user-time |
|:-------------------------------|------------:|------------:|------------:|
| baseline-4k                    |        0.0% |        0.0% |        0.0% |
| anonfolio-lkml-v1              |       -4.6% |      -45.7% |        1.4% |
| anonfolio-lkml-v2-simple-order |       -4.7% |      -40.2% |       -0.1% |
| anonfolio-lkml-v2              |       -5.0% |      -42.6% |       -0.3% |

simple-order costs 0.3 % here. v2 is actually performing higher than v1 due to
fixing the v1 regression on user-time.


Speedometer 2.0:

| kernel                         |   runs_per_min |
|:-------------------------------|---------------:|
| baseline-4k                    |           0.0% |
| anonfolio-lkml-v1              |           0.7% |
| anonfolio-lkml-v2-simple-order |          -0.9% |
| anonfolio-lkml-v2              |           0.5% |

simple-order regresses performance by 0.9% vs the baseline, for a total negative
swing of 1.6% vs v1. This is fixed by keeping the more complex order selection
mechanism from v1.


The remaining (kernel time) performance gap between v1 and v2 for the above
benchmarks is due to the removal of the "batch zap" patch in v2. Adding that
back in gives us the performance back. I intend to submit that as a separate
series once this series is accepted.


[1] https://lore.kernel.org/linux-mm/20230626171430.3167004-1-ryan.roberts@arm.com/
[2] https://lore.kernel.org/linux-mm/20230315051444.3229621-1-willy@infradead.org/
[3] https://gitlab.arm.com/linux-arm/linux-rr/-/tree/features/granule_perf/anonfolio-lkml_v2

Thanks,
Ryan


Ryan Roberts (5):
  mm: Non-pmd-mappable, large folios for folio_add_new_anon_rmap()
  mm: Allow deferred splitting of arbitrary large anon folios
  mm: Default implementation of arch_wants_pte_order()
  mm: FLEXIBLE_THP for improved performance
  arm64: mm: Override arch_wants_pte_order()

 arch/arm64/Kconfig               |  12 +++
 arch/arm64/include/asm/pgtable.h |   4 +
 arch/arm64/mm/mmu.c              |   8 ++
 include/linux/pgtable.h          |  13 +++
 mm/Kconfig                       |  10 ++
 mm/memory.c                      | 168 ++++++++++++++++++++++++++++---
 mm/rmap.c                        |  28 ++++--
 7 files changed, 222 insertions(+), 21 deletions(-)

--
2.25.1
  

On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> Hi All,
>
> This is v2 of a series to implement variable order, large folios for anonymous
> memory. The objective of this is to improve performance by allocating larger
> chunks of memory during anonymous page faults. See [1] for background.

Thanks for the quick response!

> I've significantly reworked and simplified the patch set based on comments from
> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
> VARIABLE_THP, on Yu's advice.
>
> The last patch is for arm64 to explicitly override the default
> arch_wants_pte_order() and is intended as an example. If this series is accepted
> I suggest taking the first 4 patches through the mm tree and the arm64 change
> could be handled through the arm64 tree separately. Neither has any build
> dependency on the other.
>
> The one area where I haven't followed Yu's advice is in the determination of the
> size of folio to use. It was suggested that I have a single preferred large
> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
> being existing overlapping populated PTEs, etc) then fallback immediately to
> order-0. It turned out that this approach caused a performance regression in the
> Speedometer benchmark.

I suppose it's regression against the v1, not the unpatched kernel.

> With my v1 patch, there were significant quantities of
> memory which could not be placed in the 64K bucket and were instead being
> allocated for the 32K and 16K buckets. With the proposed simplification, that
> memory ended up using the 4K bucket, so page faults increased by 2.75x compared
> to the v1 patch (although due to the 64K bucket, this number is still a bit
> lower than the baseline). So instead, I continue to calculate a folio order that
> is somewhere between the preferred order and 0. (See below for more details).

I suppose the benchmark wasn't running under memory pressure, which is
uncommon for client devices. It could be easier the other way around:
using 32/16KB shows regression whereas order-0 shows better
performance under memory pressure.

I'm not sure we should use v1 as the baseline. Unpatched kernel sounds
more reasonable at this point. If 32/16KB is proven to be better in
most scenarios including under memory pressure, we can reintroduce
that policy. I highly doubt this is the case: we tried 16KB base page
size on client devices, and overall, the regressions outweighs the
benefits.

> The patches are based on top of v6.4 plus Matthew Wilcox's set_ptes() series
> [2], which is a hard dependency. I have a branch at [3].

It's not clear to me why [2] is a hard dependency.

It seems to me we are getting close and I was hoping we could get into
mm-unstable soon without depending on other series...
  

On 7/4/2023 10:18 AM, Yu Zhao wrote:
> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> Hi All,
>>
>> This is v2 of a series to implement variable order, large folios for anonymous
>> memory. The objective of this is to improve performance by allocating larger
>> chunks of memory during anonymous page faults. See [1] for background.
> 
> Thanks for the quick response!
> 
>> I've significantly reworked and simplified the patch set based on comments from
>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
>> VARIABLE_THP, on Yu's advice.
>>
>> The last patch is for arm64 to explicitly override the default
>> arch_wants_pte_order() and is intended as an example. If this series is accepted
>> I suggest taking the first 4 patches through the mm tree and the arm64 change
>> could be handled through the arm64 tree separately. Neither has any build
>> dependency on the other.
>>
>> The one area where I haven't followed Yu's advice is in the determination of the
>> size of folio to use. It was suggested that I have a single preferred large
>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
>> being existing overlapping populated PTEs, etc) then fallback immediately to
>> order-0. It turned out that this approach caused a performance regression in the
>> Speedometer benchmark.
> 
> I suppose it's regression against the v1, not the unpatched kernel.
From the performance data Ryan shared, it's against unpatched kernel:

Speedometer 2.0:

| kernel                         |   runs_per_min |
|:-------------------------------|---------------:|
| baseline-4k                    |           0.0% |
| anonfolio-lkml-v1              |           0.7% |
| anonfolio-lkml-v2-simple-order |          -0.9% |
| anonfolio-lkml-v2              |           0.5% |


What if we use 32K or 16K instead of 64K as default anonymous folio size? I suspect
this app may have 32K or 16K anon folio as sweet spot.


Regards
Yin, Fengwei

> 
>> With my v1 patch, there were significant quantities of
>> memory which could not be placed in the 64K bucket and were instead being
>> allocated for the 32K and 16K buckets. With the proposed simplification, that
>> memory ended up using the 4K bucket, so page faults increased by 2.75x compared
>> to the v1 patch (although due to the 64K bucket, this number is still a bit
>> lower than the baseline). So instead, I continue to calculate a folio order that
>> is somewhere between the preferred order and 0. (See below for more details).
> 
> I suppose the benchmark wasn't running under memory pressure, which is
> uncommon for client devices. It could be easier the other way around:
> using 32/16KB shows regression whereas order-0 shows better
> performance under memory pressure.
> 
> I'm not sure we should use v1 as the baseline. Unpatched kernel sounds
> more reasonable at this point. If 32/16KB is proven to be better in
> most scenarios including under memory pressure, we can reintroduce
> that policy. I highly doubt this is the case: we tried 16KB base page
> size on client devices, and overall, the regressions outweighs the
> benefits.
> 
>> The patches are based on top of v6.4 plus Matthew Wilcox's set_ptes() series
>> [2], which is a hard dependency. I have a branch at [3].
> 
> It's not clear to me why [2] is a hard dependency.
> 
> It seems to me we are getting close and I was hoping we could get into
> mm-unstable soon without depending on other series...
>
  

On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>
> On 7/4/2023 10:18 AM, Yu Zhao wrote:
> > On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>
> >> Hi All,
> >>
> >> This is v2 of a series to implement variable order, large folios for anonymous
> >> memory. The objective of this is to improve performance by allocating larger
> >> chunks of memory during anonymous page faults. See [1] for background.
> >
> > Thanks for the quick response!
> >
> >> I've significantly reworked and simplified the patch set based on comments from
> >> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
> >> VARIABLE_THP, on Yu's advice.
> >>
> >> The last patch is for arm64 to explicitly override the default
> >> arch_wants_pte_order() and is intended as an example. If this series is accepted
> >> I suggest taking the first 4 patches through the mm tree and the arm64 change
> >> could be handled through the arm64 tree separately. Neither has any build
> >> dependency on the other.
> >>
> >> The one area where I haven't followed Yu's advice is in the determination of the
> >> size of folio to use. It was suggested that I have a single preferred large
> >> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
> >> being existing overlapping populated PTEs, etc) then fallback immediately to
> >> order-0. It turned out that this approach caused a performance regression in the
> >> Speedometer benchmark.
> >
> > I suppose it's regression against the v1, not the unpatched kernel.
> From the performance data Ryan shared, it's against unpatched kernel:
>
> Speedometer 2.0:
>
> | kernel                         |   runs_per_min |
> |:-------------------------------|---------------:|
> | baseline-4k                    |           0.0% |
> | anonfolio-lkml-v1              |           0.7% |
> | anonfolio-lkml-v2-simple-order |          -0.9% |
> | anonfolio-lkml-v2              |           0.5% |

I see. Thanks.

A couple of questions:
1. Do we have a stddev?
2. Do we have a theory why it regressed?
Assuming no bugs, I don't see how a real regression could happen --
falling back to order-0 isn't different from the original behavior.
Ryan, could you `perf record` and `cat /proc/vmstat` and share them?
  

On 04/07/2023 08:11, Yu Zhao wrote:
> On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>>
>> On 7/4/2023 10:18 AM, Yu Zhao wrote:
>>> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> This is v2 of a series to implement variable order, large folios for anonymous
>>>> memory. The objective of this is to improve performance by allocating larger
>>>> chunks of memory during anonymous page faults. See [1] for background.
>>>
>>> Thanks for the quick response!
>>>
>>>> I've significantly reworked and simplified the patch set based on comments from
>>>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
>>>> VARIABLE_THP, on Yu's advice.
>>>>
>>>> The last patch is for arm64 to explicitly override the default
>>>> arch_wants_pte_order() and is intended as an example. If this series is accepted
>>>> I suggest taking the first 4 patches through the mm tree and the arm64 change
>>>> could be handled through the arm64 tree separately. Neither has any build
>>>> dependency on the other.
>>>>
>>>> The one area where I haven't followed Yu's advice is in the determination of the
>>>> size of folio to use. It was suggested that I have a single preferred large
>>>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
>>>> being existing overlapping populated PTEs, etc) then fallback immediately to
>>>> order-0. It turned out that this approach caused a performance regression in the
>>>> Speedometer benchmark.
>>>
>>> I suppose it's regression against the v1, not the unpatched kernel.
>> From the performance data Ryan shared, it's against unpatched kernel:
>>
>> Speedometer 2.0:
>>
>> | kernel                         |   runs_per_min |
>> |:-------------------------------|---------------:|
>> | baseline-4k                    |           0.0% |
>> | anonfolio-lkml-v1              |           0.7% |
>> | anonfolio-lkml-v2-simple-order |          -0.9% |
>> | anonfolio-lkml-v2              |           0.5% |
> 
> I see. Thanks.
> 
> A couple of questions:
> 1. Do we have a stddev?

| kernel                    |   mean_abs |   std_abs |   mean_rel |   std_rel |
|:------------------------- |-----------:|----------:|-----------:|----------:|
| baseline-4k               |      117.4 |       0.8 |       0.0% |      0.7% |
| anonfolio-v1              |      118.2 |         1 |       0.7% |      0.9% |
| anonfolio-v2-simple-order |      116.4 |       1.1 |      -0.9% |      0.9% |
| anonfolio-v2              |        118 |       1.2 |       0.5% |      1.0% |

This is with 3 runs per reboot across 5 reboots, with first run after reboot
trimmed (it's always a bit slower, I assume due to cold page cache). So 10 data
points per kernel in total.

I've rerun the test multiple times and see similar results each time.

I've also run anonfolio-v2 with Kconfig FLEXIBLE_THP=disabled and in this case I
see the same performance as baseline-4k.


> 2. Do we have a theory why it regressed?

I have a woolly hypothesis; I think Chromium is doing mmap/munmap in ways that
mean when we fault, order-4 is often too big to fit in the VMA. So we fallback
to order-0. I guess this is happening so often for this workload that the cost
of doing the checks and fallback is outweighing the benefit of the memory that
does end up with order-4 folios.

I've sampled the memory in each bucket (once per second) while running and its
roughly:

64K: 25%
32K: 15%
16K: 15%
4K: 45%

32K and 16K obviously fold into the 4K bucket with anonfolio-v2-simple-order.
But potentially, I suspect there is lots of mmap/unmap for the smaller sizes and
the 64K contents is more static - that's just a guess though.

> Assuming no bugs, I don't see how a real regression could happen --
> falling back to order-0 isn't different from the original behavior.
> Ryan, could you `perf record` and `cat /proc/vmstat` and share them?

I can, but it will have to be a bit later in the week. I'll do some more test
runs overnight so we have a larger number of runs - hopefully that might tell us
that this is noise to a certain extent.

I'd still like to hear a clear technical argument for why the bin-packing
approach is not the correct one!

Thanks,
Ryan
  

On 7/4/23 23:36, Ryan Roberts wrote:
> On 04/07/2023 08:11, Yu Zhao wrote:
>> On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>>>
>>> On 7/4/2023 10:18 AM, Yu Zhao wrote:
>>>> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>>
>>>>> Hi All,
>>>>>
>>>>> This is v2 of a series to implement variable order, large folios for anonymous
>>>>> memory. The objective of this is to improve performance by allocating larger
>>>>> chunks of memory during anonymous page faults. See [1] for background.
>>>>
>>>> Thanks for the quick response!
>>>>
>>>>> I've significantly reworked and simplified the patch set based on comments from
>>>>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
>>>>> VARIABLE_THP, on Yu's advice.
>>>>>
>>>>> The last patch is for arm64 to explicitly override the default
>>>>> arch_wants_pte_order() and is intended as an example. If this series is accepted
>>>>> I suggest taking the first 4 patches through the mm tree and the arm64 change
>>>>> could be handled through the arm64 tree separately. Neither has any build
>>>>> dependency on the other.
>>>>>
>>>>> The one area where I haven't followed Yu's advice is in the determination of the
>>>>> size of folio to use. It was suggested that I have a single preferred large
>>>>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
>>>>> being existing overlapping populated PTEs, etc) then fallback immediately to
>>>>> order-0. It turned out that this approach caused a performance regression in the
>>>>> Speedometer benchmark.
>>>>
>>>> I suppose it's regression against the v1, not the unpatched kernel.
>>> From the performance data Ryan shared, it's against unpatched kernel:
>>>
>>> Speedometer 2.0:
>>>
>>> | kernel                         |   runs_per_min |
>>> |:-------------------------------|---------------:|
>>> | baseline-4k                    |           0.0% |
>>> | anonfolio-lkml-v1              |           0.7% |
>>> | anonfolio-lkml-v2-simple-order |          -0.9% |
>>> | anonfolio-lkml-v2              |           0.5% |
>>
>> I see. Thanks.
>>
>> A couple of questions:
>> 1. Do we have a stddev?
> 
> | kernel                    |   mean_abs |   std_abs |   mean_rel |   std_rel |
> |:------------------------- |-----------:|----------:|-----------:|----------:|
> | baseline-4k               |      117.4 |       0.8 |       0.0% |      0.7% |
> | anonfolio-v1              |      118.2 |         1 |       0.7% |      0.9% |
> | anonfolio-v2-simple-order |      116.4 |       1.1 |      -0.9% |      0.9% |
> | anonfolio-v2              |        118 |       1.2 |       0.5% |      1.0% |
> 
> This is with 3 runs per reboot across 5 reboots, with first run after reboot
> trimmed (it's always a bit slower, I assume due to cold page cache). So 10 data
> points per kernel in total.
> 
> I've rerun the test multiple times and see similar results each time.
> 
> I've also run anonfolio-v2 with Kconfig FLEXIBLE_THP=disabled and in this case I
> see the same performance as baseline-4k.
> 
> 
>> 2. Do we have a theory why it regressed?
> 
> I have a woolly hypothesis; I think Chromium is doing mmap/munmap in ways that
> mean when we fault, order-4 is often too big to fit in the VMA. So we fallback
> to order-0. I guess this is happening so often for this workload that the cost
> of doing the checks and fallback is outweighing the benefit of the memory that
> does end up with order-4 folios.
> 
> I've sampled the memory in each bucket (once per second) while running and its
> roughly:
> 
> 64K: 25%
> 32K: 15%
> 16K: 15%
> 4K: 45%
> 
> 32K and 16K obviously fold into the 4K bucket with anonfolio-v2-simple-order.
> But potentially, I suspect there is lots of mmap/unmap for the smaller sizes and
> the 64K contents is more static - that's just a guess though.
So this is like out of vma range thing.

> 
>> Assuming no bugs, I don't see how a real regression could happen --
>> falling back to order-0 isn't different from the original behavior.
>> Ryan, could you `perf record` and `cat /proc/vmstat` and share them?
> 
> I can, but it will have to be a bit later in the week. I'll do some more test
> runs overnight so we have a larger number of runs - hopefully that might tell us
> that this is noise to a certain extent.
> 
> I'd still like to hear a clear technical argument for why the bin-packing
> approach is not the correct one!
My understanding to Yu's (Yu, correct me if I am wrong) comments is that we
postpone this part of change and make basic anon large folio support in. Then
discuss which approach we should take. Maybe people will agree retry is the
choice, maybe other approach will be taken...

For example, for this out of VMA range case, per VMA order should be considered.
We don't need make decision that the retry should be taken now.


Regards
Yin, Fengwei

> 
> Thanks,
> Ryan
> 
> 
>
  

On Tue, Jul 4, 2023 at 5:53 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
>
>
>
> On 7/4/23 23:36, Ryan Roberts wrote:
> > On 04/07/2023 08:11, Yu Zhao wrote:
> >> On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
> >>>
> >>> On 7/4/2023 10:18 AM, Yu Zhao wrote:
> >>>> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>>>>
> >>>>> Hi All,
> >>>>>
> >>>>> This is v2 of a series to implement variable order, large folios for anonymous
> >>>>> memory. The objective of this is to improve performance by allocating larger
> >>>>> chunks of memory during anonymous page faults. See [1] for background.
> >>>>
> >>>> Thanks for the quick response!
> >>>>
> >>>>> I've significantly reworked and simplified the patch set based on comments from
> >>>>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
> >>>>> VARIABLE_THP, on Yu's advice.
> >>>>>
> >>>>> The last patch is for arm64 to explicitly override the default
> >>>>> arch_wants_pte_order() and is intended as an example. If this series is accepted
> >>>>> I suggest taking the first 4 patches through the mm tree and the arm64 change
> >>>>> could be handled through the arm64 tree separately. Neither has any build
> >>>>> dependency on the other.
> >>>>>
> >>>>> The one area where I haven't followed Yu's advice is in the determination of the
> >>>>> size of folio to use. It was suggested that I have a single preferred large
> >>>>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
> >>>>> being existing overlapping populated PTEs, etc) then fallback immediately to
> >>>>> order-0. It turned out that this approach caused a performance regression in the
> >>>>> Speedometer benchmark.
> >>>>
> >>>> I suppose it's regression against the v1, not the unpatched kernel.
> >>> From the performance data Ryan shared, it's against unpatched kernel:
> >>>
> >>> Speedometer 2.0:
> >>>
> >>> | kernel                         |   runs_per_min |
> >>> |:-------------------------------|---------------:|
> >>> | baseline-4k                    |           0.0% |
> >>> | anonfolio-lkml-v1              |           0.7% |
> >>> | anonfolio-lkml-v2-simple-order |          -0.9% |
> >>> | anonfolio-lkml-v2              |           0.5% |
> >>
> >> I see. Thanks.
> >>
> >> A couple of questions:
> >> 1. Do we have a stddev?
> >
> > | kernel                    |   mean_abs |   std_abs |   mean_rel |   std_rel |
> > |:------------------------- |-----------:|----------:|-----------:|----------:|
> > | baseline-4k               |      117.4 |       0.8 |       0.0% |      0.7% |
> > | anonfolio-v1              |      118.2 |         1 |       0.7% |      0.9% |
> > | anonfolio-v2-simple-order |      116.4 |       1.1 |      -0.9% |      0.9% |
> > | anonfolio-v2              |        118 |       1.2 |       0.5% |      1.0% |
> >
> > This is with 3 runs per reboot across 5 reboots, with first run after reboot
> > trimmed (it's always a bit slower, I assume due to cold page cache). So 10 data
> > points per kernel in total.
> >
> > I've rerun the test multiple times and see similar results each time.
> >
> > I've also run anonfolio-v2 with Kconfig FLEXIBLE_THP=disabled and in this case I
> > see the same performance as baseline-4k.
> >
> >
> >> 2. Do we have a theory why it regressed?
> >
> > I have a woolly hypothesis; I think Chromium is doing mmap/munmap in ways that
> > mean when we fault, order-4 is often too big to fit in the VMA. So we fallback
> > to order-0. I guess this is happening so often for this workload that the cost
> > of doing the checks and fallback is outweighing the benefit of the memory that
> > does end up with order-4 folios.
> >
> > I've sampled the memory in each bucket (once per second) while running and its
> > roughly:
> >
> > 64K: 25%
> > 32K: 15%
> > 16K: 15%
> > 4K: 45%
> >
> > 32K and 16K obviously fold into the 4K bucket with anonfolio-v2-simple-order.
> > But potentially, I suspect there is lots of mmap/unmap for the smaller sizes and
> > the 64K contents is more static - that's just a guess though.
> So this is like out of vma range thing.
>
> >
> >> Assuming no bugs, I don't see how a real regression could happen --
> >> falling back to order-0 isn't different from the original behavior.
> >> Ryan, could you `perf record` and `cat /proc/vmstat` and share them?
> >
> > I can, but it will have to be a bit later in the week. I'll do some more test
> > runs overnight so we have a larger number of runs - hopefully that might tell us
> > that this is noise to a certain extent.
> >
> > I'd still like to hear a clear technical argument for why the bin-packing
> > approach is not the correct one!
> My understanding to Yu's (Yu, correct me if I am wrong) comments is that we
> postpone this part of change and make basic anon large folio support in. Then
> discuss which approach we should take. Maybe people will agree retry is the
> choice, maybe other approach will be taken...
>
> For example, for this out of VMA range case, per VMA order should be considered.
> We don't need make decision that the retry should be taken now.

I've articulated the reasons in another email. Just summarize the most
important point here:
using more fallback orders makes a system reach equilibrium faster, at
which point it can't allocate the order of arch_wants_pte_order()
anymore. IOW, this best-fit policy can reduce the number of folios of
the h/w prefered order for a system running long enough.
  

On 05/07/2023 01:21, Yu Zhao wrote:
> On Tue, Jul 4, 2023 at 5:53 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
>>
>>
>>
>> On 7/4/23 23:36, Ryan Roberts wrote:
>>> On 04/07/2023 08:11, Yu Zhao wrote:
>>>> On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>>>>>
>>>>> On 7/4/2023 10:18 AM, Yu Zhao wrote:
>>>>>> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>>>>
>>>>>>> Hi All,
>>>>>>>
>>>>>>> This is v2 of a series to implement variable order, large folios for anonymous
>>>>>>> memory. The objective of this is to improve performance by allocating larger
>>>>>>> chunks of memory during anonymous page faults. See [1] for background.
>>>>>>
>>>>>> Thanks for the quick response!
>>>>>>
>>>>>>> I've significantly reworked and simplified the patch set based on comments from
>>>>>>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
>>>>>>> VARIABLE_THP, on Yu's advice.
>>>>>>>
>>>>>>> The last patch is for arm64 to explicitly override the default
>>>>>>> arch_wants_pte_order() and is intended as an example. If this series is accepted
>>>>>>> I suggest taking the first 4 patches through the mm tree and the arm64 change
>>>>>>> could be handled through the arm64 tree separately. Neither has any build
>>>>>>> dependency on the other.
>>>>>>>
>>>>>>> The one area where I haven't followed Yu's advice is in the determination of the
>>>>>>> size of folio to use. It was suggested that I have a single preferred large
>>>>>>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
>>>>>>> being existing overlapping populated PTEs, etc) then fallback immediately to
>>>>>>> order-0. It turned out that this approach caused a performance regression in the
>>>>>>> Speedometer benchmark.
>>>>>>
>>>>>> I suppose it's regression against the v1, not the unpatched kernel.
>>>>> From the performance data Ryan shared, it's against unpatched kernel:
>>>>>
>>>>> Speedometer 2.0:
>>>>>
>>>>> | kernel                         |   runs_per_min |
>>>>> |:-------------------------------|---------------:|
>>>>> | baseline-4k                    |           0.0% |
>>>>> | anonfolio-lkml-v1              |           0.7% |
>>>>> | anonfolio-lkml-v2-simple-order |          -0.9% |
>>>>> | anonfolio-lkml-v2              |           0.5% |
>>>>
>>>> I see. Thanks.
>>>>
>>>> A couple of questions:
>>>> 1. Do we have a stddev?
>>>
>>> | kernel                    |   mean_abs |   std_abs |   mean_rel |   std_rel |
>>> |:------------------------- |-----------:|----------:|-----------:|----------:|
>>> | baseline-4k               |      117.4 |       0.8 |       0.0% |      0.7% |
>>> | anonfolio-v1              |      118.2 |         1 |       0.7% |      0.9% |
>>> | anonfolio-v2-simple-order |      116.4 |       1.1 |      -0.9% |      0.9% |
>>> | anonfolio-v2              |        118 |       1.2 |       0.5% |      1.0% |
>>>
>>> This is with 3 runs per reboot across 5 reboots, with first run after reboot
>>> trimmed (it's always a bit slower, I assume due to cold page cache). So 10 data
>>> points per kernel in total.
>>>
>>> I've rerun the test multiple times and see similar results each time.
>>>
>>> I've also run anonfolio-v2 with Kconfig FLEXIBLE_THP=disabled and in this case I
>>> see the same performance as baseline-4k.
>>>
>>>
>>>> 2. Do we have a theory why it regressed?
>>>
>>> I have a woolly hypothesis; I think Chromium is doing mmap/munmap in ways that
>>> mean when we fault, order-4 is often too big to fit in the VMA. So we fallback
>>> to order-0. I guess this is happening so often for this workload that the cost
>>> of doing the checks and fallback is outweighing the benefit of the memory that
>>> does end up with order-4 folios.
>>>
>>> I've sampled the memory in each bucket (once per second) while running and its
>>> roughly:
>>>
>>> 64K: 25%
>>> 32K: 15%
>>> 16K: 15%
>>> 4K: 45%
>>>
>>> 32K and 16K obviously fold into the 4K bucket with anonfolio-v2-simple-order.
>>> But potentially, I suspect there is lots of mmap/unmap for the smaller sizes and
>>> the 64K contents is more static - that's just a guess though.
>> So this is like out of vma range thing.
>>
>>>
>>>> Assuming no bugs, I don't see how a real regression could happen --
>>>> falling back to order-0 isn't different from the original behavior.
>>>> Ryan, could you `perf record` and `cat /proc/vmstat` and share them?
>>>
>>> I can, but it will have to be a bit later in the week. I'll do some more test
>>> runs overnight so we have a larger number of runs - hopefully that might tell us
>>> that this is noise to a certain extent.
>>>
>>> I'd still like to hear a clear technical argument for why the bin-packing
>>> approach is not the correct one!
>> My understanding to Yu's (Yu, correct me if I am wrong) comments is that we
>> postpone this part of change and make basic anon large folio support in. Then
>> discuss which approach we should take. Maybe people will agree retry is the
>> choice, maybe other approach will be taken...
>>
>> For example, for this out of VMA range case, per VMA order should be considered.
>> We don't need make decision that the retry should be taken now.
> 
> I've articulated the reasons in another email. Just summarize the most
> important point here:
> using more fallback orders makes a system reach equilibrium faster, at
> which point it can't allocate the order of arch_wants_pte_order()
> anymore. IOW, this best-fit policy can reduce the number of folios of
> the h/w prefered order for a system running long enough.

Thanks for taking the time to write all the arguments down. I understand what
you are saying. If we are considering the whole system, then we also need to
think about the page cache though, and that will allocate multiple orders, so
you are still going to suffer fragmentation from that user.

That said, I like the proposal patch posted where we have up to 3 orders that we
try in order of preference; hw-preferred, PAGE_ALLOC_COSTLY_ORDER and 0. That
feels like a good compromise that allows me to fulfil my objectives. I'm going
to pull this together into a v3 patch set and aim to post towards the end of the
week.

Are you ok for me to add a Suggested-by: for you? (submitting-patches.rst says I
need your explicit permission).

On the regression front, I've done a much bigger test run and see the regression
is still present (although the mean has shifted a little bit). I've also built a
kernel based on anonfolio-lkml-v2 but where arch_wants_pte_order() returns
order-3. The aim was to test your hypothesis that 64K allocation is slow. This
kernel is performing even better, so I think that confirms your hypothesis:

| kernel                         |   runs_per_min |   runs |   sessions |
|:-------------------------------|---------------:|-------:|-----------:|
| baseline-4k                    |           0.0% |     75 |         15 |
| anonfolio-lkml-v1              |           1.0% |     75 |         15 |
| anonfolio-lkml-v2-simple-order |          -0.4% |     75 |         15 |
| anonfolio-lkml-v2              |           0.9% |     75 |         15 |
| anonfolio-lkml-v2-32k          |           1.4% |     10 |          5 |

Thanks,
Ryan
  

On Wed, Jul 5, 2023 at 4:16 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 05/07/2023 01:21, Yu Zhao wrote:
> > On Tue, Jul 4, 2023 at 5:53 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
> >>
> >>
> >>
> >> On 7/4/23 23:36, Ryan Roberts wrote:
> >>> On 04/07/2023 08:11, Yu Zhao wrote:
> >>>> On Tue, Jul 4, 2023 at 12:22 AM Yin, Fengwei <fengwei.yin@intel.com> wrote:
> >>>>>
> >>>>> On 7/4/2023 10:18 AM, Yu Zhao wrote:
> >>>>>> On Mon, Jul 3, 2023 at 7:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>>>>>>
> >>>>>>> Hi All,
> >>>>>>>
> >>>>>>> This is v2 of a series to implement variable order, large folios for anonymous
> >>>>>>> memory. The objective of this is to improve performance by allocating larger
> >>>>>>> chunks of memory during anonymous page faults. See [1] for background.
> >>>>>>
> >>>>>> Thanks for the quick response!
> >>>>>>
> >>>>>>> I've significantly reworked and simplified the patch set based on comments from
> >>>>>>> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
> >>>>>>> VARIABLE_THP, on Yu's advice.
> >>>>>>>
> >>>>>>> The last patch is for arm64 to explicitly override the default
> >>>>>>> arch_wants_pte_order() and is intended as an example. If this series is accepted
> >>>>>>> I suggest taking the first 4 patches through the mm tree and the arm64 change
> >>>>>>> could be handled through the arm64 tree separately. Neither has any build
> >>>>>>> dependency on the other.
> >>>>>>>
> >>>>>>> The one area where I haven't followed Yu's advice is in the determination of the
> >>>>>>> size of folio to use. It was suggested that I have a single preferred large
> >>>>>>> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
> >>>>>>> being existing overlapping populated PTEs, etc) then fallback immediately to
> >>>>>>> order-0. It turned out that this approach caused a performance regression in the
> >>>>>>> Speedometer benchmark.
> >>>>>>
> >>>>>> I suppose it's regression against the v1, not the unpatched kernel.
> >>>>> From the performance data Ryan shared, it's against unpatched kernel:
> >>>>>
> >>>>> Speedometer 2.0:
> >>>>>
> >>>>> | kernel                         |   runs_per_min |
> >>>>> |:-------------------------------|---------------:|
> >>>>> | baseline-4k                    |           0.0% |
> >>>>> | anonfolio-lkml-v1              |           0.7% |
> >>>>> | anonfolio-lkml-v2-simple-order |          -0.9% |
> >>>>> | anonfolio-lkml-v2              |           0.5% |
> >>>>
> >>>> I see. Thanks.
> >>>>
> >>>> A couple of questions:
> >>>> 1. Do we have a stddev?
> >>>
> >>> | kernel                    |   mean_abs |   std_abs |   mean_rel |   std_rel |
> >>> |:------------------------- |-----------:|----------:|-----------:|----------:|
> >>> | baseline-4k               |      117.4 |       0.8 |       0.0% |      0.7% |
> >>> | anonfolio-v1              |      118.2 |         1 |       0.7% |      0.9% |
> >>> | anonfolio-v2-simple-order |      116.4 |       1.1 |      -0.9% |      0.9% |
> >>> | anonfolio-v2              |        118 |       1.2 |       0.5% |      1.0% |
> >>>
> >>> This is with 3 runs per reboot across 5 reboots, with first run after reboot
> >>> trimmed (it's always a bit slower, I assume due to cold page cache). So 10 data
> >>> points per kernel in total.
> >>>
> >>> I've rerun the test multiple times and see similar results each time.
> >>>
> >>> I've also run anonfolio-v2 with Kconfig FLEXIBLE_THP=disabled and in this case I
> >>> see the same performance as baseline-4k.
> >>>
> >>>
> >>>> 2. Do we have a theory why it regressed?
> >>>
> >>> I have a woolly hypothesis; I think Chromium is doing mmap/munmap in ways that
> >>> mean when we fault, order-4 is often too big to fit in the VMA. So we fallback
> >>> to order-0. I guess this is happening so often for this workload that the cost
> >>> of doing the checks and fallback is outweighing the benefit of the memory that
> >>> does end up with order-4 folios.
> >>>
> >>> I've sampled the memory in each bucket (once per second) while running and its
> >>> roughly:
> >>>
> >>> 64K: 25%
> >>> 32K: 15%
> >>> 16K: 15%
> >>> 4K: 45%
> >>>
> >>> 32K and 16K obviously fold into the 4K bucket with anonfolio-v2-simple-order.
> >>> But potentially, I suspect there is lots of mmap/unmap for the smaller sizes and
> >>> the 64K contents is more static - that's just a guess though.
> >> So this is like out of vma range thing.
> >>
> >>>
> >>>> Assuming no bugs, I don't see how a real regression could happen --
> >>>> falling back to order-0 isn't different from the original behavior.
> >>>> Ryan, could you `perf record` and `cat /proc/vmstat` and share them?
> >>>
> >>> I can, but it will have to be a bit later in the week. I'll do some more test
> >>> runs overnight so we have a larger number of runs - hopefully that might tell us
> >>> that this is noise to a certain extent.
> >>>
> >>> I'd still like to hear a clear technical argument for why the bin-packing
> >>> approach is not the correct one!
> >> My understanding to Yu's (Yu, correct me if I am wrong) comments is that we
> >> postpone this part of change and make basic anon large folio support in. Then
> >> discuss which approach we should take. Maybe people will agree retry is the
> >> choice, maybe other approach will be taken...
> >>
> >> For example, for this out of VMA range case, per VMA order should be considered.
> >> We don't need make decision that the retry should be taken now.
> >
> > I've articulated the reasons in another email. Just summarize the most
> > important point here:
> > using more fallback orders makes a system reach equilibrium faster, at
> > which point it can't allocate the order of arch_wants_pte_order()
> > anymore. IOW, this best-fit policy can reduce the number of folios of
> > the h/w prefered order for a system running long enough.
>
> Thanks for taking the time to write all the arguments down. I understand what
> you are saying. If we are considering the whole system, then we also need to
> think about the page cache though, and that will allocate multiple orders, so
> you are still going to suffer fragmentation from that user.

1. page cache doesn't use the best-fit policy -- it has the advantage
of having RA hit/miss numbers -- IOW, it doesn't try all orders
without an estimated ROI.
2. page cache causes far less fragmentation in my experience: clean
page cache gets reclaimed first under memory; unmapped page cache is
less costly to migrate. Neither is true for anon, and what makes it
worse is that heavy anon users usually enable zram/zswap: allocating
memory (to store compressed data) under memory pressure makes
reclaim/compaction even harder.

> That said, I like the proposal patch posted where we have up to 3 orders that we
> try in order of preference; hw-preferred, PAGE_ALLOC_COSTLY_ORDER and 0. That
> feels like a good compromise that allows me to fulfil my objectives. I'm going
> to pull this together into a v3 patch set and aim to post towards the end of the
> week.
>
> Are you ok for me to add a Suggested-by: for you? (submitting-patches.rst says I
> need your explicit permission).

Thanks for asking. No need to worry about it -- it's been a great team
work with you, Fengwei, Yang et al.

I'm attaching a single patch containing all pieces I spelled
out/implied/forgot to mention. It doesn't depend on other series -- I
just stress-tested it on top of the latest mm-unstable. Please feel
free to reuse any bits you see fit. Again no need to worry about
Suggested-by.

> On the regression front, I've done a much bigger test run and see the regression
> is still present (although the mean has shifted a little bit). I've also built a
> kernel based on anonfolio-lkml-v2 but where arch_wants_pte_order() returns
> order-3. The aim was to test your hypothesis that 64K allocation is slow. This
> kernel is performing even better, so I think that confirms your hypothesis:

Great, thanks for confirming.

> | kernel                         |   runs_per_min |   runs |   sessions |
> |:-------------------------------|---------------:|-------:|-----------:|
> | baseline-4k                    |           0.0% |     75 |         15 |
> | anonfolio-lkml-v1              |           1.0% |     75 |         15 |
> | anonfolio-lkml-v2-simple-order |          -0.4% |     75 |         15 |
> | anonfolio-lkml-v2              |           0.9% |     75 |         15 |
> | anonfolio-lkml-v2-32k          |           1.4% |     10 |          5 |

Since we are all committed to the effort long term, the last number is
good enough for the initial step to conclude. Hopefully v3 can address
all pending comments and get into mm-unstable.
  

On 03.07.23 15:53, Ryan Roberts wrote:
> Hi All,
> 
> This is v2 of a series to implement variable order, large folios for anonymous
> memory. The objective of this is to improve performance by allocating larger
> chunks of memory during anonymous page faults. See [1] for background.
> 
> I've significantly reworked and simplified the patch set based on comments from
> Yu Zhao (thanks for all your feedback!). I've also renamed the feature to
> VARIABLE_THP, on Yu's advice.
> 
> The last patch is for arm64 to explicitly override the default
> arch_wants_pte_order() and is intended as an example. If this series is accepted
> I suggest taking the first 4 patches through the mm tree and the arm64 change
> could be handled through the arm64 tree separately. Neither has any build
> dependency on the other.
> 
> The one area where I haven't followed Yu's advice is in the determination of the
> size of folio to use. It was suggested that I have a single preferred large
> order, and if it doesn't fit in the VMA (due to exceeding VMA bounds, or there
> being existing overlapping populated PTEs, etc) then fallback immediately to
> order-0. It turned out that this approach caused a performance regression in the
> Speedometer benchmark. With my v1 patch, there were significant quantities of
> memory which could not be placed in the 64K bucket and were instead being
> allocated for the 32K and 16K buckets. With the proposed simplification, that
> memory ended up using the 4K bucket, so page faults increased by 2.75x compared
> to the v1 patch (although due to the 64K bucket, this number is still a bit
> lower than the baseline). So instead, I continue to calculate a folio order that
> is somewhere between the preferred order and 0. (See below for more details).
> 
> The patches are based on top of v6.4 plus Matthew Wilcox's set_ptes() series
> [2], which is a hard dependency. I have a branch at [3].
> 
> 
> Changes since v1 [1]
> --------------------
> 
>    - removed changes to arch-dependent vma_alloc_zeroed_movable_folio()
>    - replaced with arch-independent alloc_anon_folio()
>        - follows THP allocation approach
>    - no longer retry with intermediate orders if allocation fails
>        - fallback directly to order-0
>    - remove folio_add_new_anon_rmap_range() patch
>        - instead add its new functionality to folio_add_new_anon_rmap()
>    - remove batch-zap pte mappings optimization patch
>        - remove enabler folio_remove_rmap_range() patch too
>        - These offer real perf improvement so will submit separately
>    - simplify Kconfig
>        - single FLEXIBLE_THP option, which is independent of arch
>        - depends on TRANSPARENT_HUGEPAGE
>        - when enabled default to max anon folio size of 64K unless arch
>          explicitly overrides
>    - simplify changes to do_anonymous_page():
>        - no more retry loop
> 
> 
> Performance
> -----------
> 
> Below results show 3 benchmarks; kernel compilation with 8 jobs, kernel
> compilation with 80 jobs, and speedometer 2.0 (a javascript benchmark running in
> Chromium). All cases are running on Ampere Altra with 1 NUMA node enabled,
> Ubuntu 22.04 and XFS filesystem. Each benchmark is repeated 15 times over 5
> reboots and averaged.
> 
> 'anonfolio-lkml-v1' is the v1 patchset at [1]. 'anonfolio-lkml-v2' is this v2
> patchset. 'anonfolio-lkml-v2-simple-order' is anonfolio-lkml-v2 but with the
> order selection simplification that Yu Zhao suggested - I'm trying to justify
> here why I did not follow the advice.
> 
> 
> Kernel compilation with 8 jobs:
> 
> | kernel                         |   real-time |   kern-time |   user-time |
> |:-------------------------------|------------:|------------:|------------:|
> | baseline-4k                    |        0.0% |        0.0% |        0.0% |
> | anonfolio-lkml-v1              |       -5.3% |      -42.9% |       -0.6% |
> | anonfolio-lkml-v2-simple-order |       -4.4% |      -36.5% |       -0.4% |
> | anonfolio-lkml-v2              |       -4.8% |      -38.6% |       -0.6% |
> 
> We can see that the simple-order approach is responsible for a regression of
> 0.4%.
> 
> 
> Kernel compilation with 80 jobs:
> 
> | kernel                         |   real-time |   kern-time |   user-time |
> |:-------------------------------|------------:|------------:|------------:|
> | baseline-4k                    |        0.0% |        0.0% |        0.0% |
> | anonfolio-lkml-v1              |       -4.6% |      -45.7% |        1.4% |
> | anonfolio-lkml-v2-simple-order |       -4.7% |      -40.2% |       -0.1% |
> | anonfolio-lkml-v2              |       -5.0% |      -42.6% |       -0.3% |
> 
> simple-order costs 0.3 % here. v2 is actually performing higher than v1 due to
> fixing the v1 regression on user-time.
> 
> 
> Speedometer 2.0:
> 
> | kernel                         |   runs_per_min |
> |:-------------------------------|---------------:|
> | baseline-4k                    |           0.0% |
> | anonfolio-lkml-v1              |           0.7% |
> | anonfolio-lkml-v2-simple-order |          -0.9% |
> | anonfolio-lkml-v2              |           0.5% |
> 
> simple-order regresses performance by 0.9% vs the baseline, for a total negative
> swing of 1.6% vs v1. This is fixed by keeping the more complex order selection
> mechanism from v1.
> 
> 
> The remaining (kernel time) performance gap between v1 and v2 for the above
> benchmarks is due to the removal of the "batch zap" patch in v2. Adding that
> back in gives us the performance back. I intend to submit that as a separate
> series once this series is accepted.
> 
> 
> [1] https://lore.kernel.org/linux-mm/20230626171430.3167004-1-ryan.roberts@arm.com/
> [2] https://lore.kernel.org/linux-mm/20230315051444.3229621-1-willy@infradead.org/
> [3] https://gitlab.arm.com/linux-arm/linux-rr/-/tree/features/granule_perf/anonfolio-lkml_v2
> 
> Thanks,
> Ryan

Hi Ryan,

is page migration already working as expected (what about page 
compaction?), and do we handle migration -ENOMEM when allocating a 
target page: do we split an fallback to 4k page migration?
  

On 05/07/2023 20:38, David Hildenbrand wrote:
> On 03.07.23 15:53, Ryan Roberts wrote:
>> Hi All,
>>
>> This is v2 of a series to implement variable order, large folios for anonymous
>> memory. The objective of this is to improve performance by allocating larger
>> chunks of memory during anonymous page faults. See [1] for background.
>>

[...]

>> Thanks,
>> Ryan
> 
> Hi Ryan,
> 
> is page migration already working as expected (what about page compaction?), and
> do we handle migration -ENOMEM when allocating a target page: do we split an
> fallback to 4k page migration?
> 

Hi David, All,

This series aims to be the bare minimum to demonstrate allocation of large anon
folios. As such, there is a laundry list of things that need to be done for this
feature to play nicely with other features. My preferred route is to merge this
with it's Kconfig defaulted to disabled, and its Kconfig description clearly
shouting that it's EXPERIMENTAL with an explanation of why (similar to
READ_ONLY_THP_FOR_FS).

That said, I've put together a table of the items that I'm aware of that need
attention. It would be great if people can review and add any missing items.
Then we can hopefully parallelize the implementation work. David, I don't think
the items you raised are covered - would you mind providing a bit more detail so
I can add them to the list? (or just add them to the list yourself, if you prefer).

---

- item:
    mlock

  description: >-
    Large, pte-mapped folios are ignored when mlock is requested. Code comment
    for mlock_vma_folio() says "...filter out pte mappings of THPs, which
    cannot be consistently counted: a pte mapping of the THP head cannot be
    distinguished by the page alone."

  location:
    - mlock_pte_range()
    - mlock_vma_folio()

  assignee:
    Yin, Fengwei


- item:
    numa balancing

  description: >-
    Large, pte-mapped folios are ignored by numa-balancing code. Commit
    comment (e81c480): "We're going to have THP mapped with PTEs. It will
    confuse numabalancing. Let's skip them for now."

  location:
    - do_numa_page()

  assignee:
    <none>


- item:
    madvise

  description: >-
    MADV_COLD, MADV_PAGEOUT, MADV_FREE: For large folios, code assumes
    exclusive only if mapcount==1, else skips remainder of operation. For
    large, pte-mapped folios, exclusive folios can have mapcount upto nr_pages
    and still be exclusive. Even better; don't split the folio if it fits
    entirely within the range? Discussion at

https://lore.kernel.org/linux-mm/6cec6f68-248e-63b4-5615-9e0f3f819a0a@redhat.com/
    talks about changing folio mapcounting - may help determine if exclusive
    without pgtable scan?

  location:
    - madvise_cold_or_pageout_pte_range()
    - madvise_free_pte_range()

  assignee:
    <none>


- item:
    shrink_folio_list

  description: >-
    Raised by Yu Zhao; I can't see the problem in the code - need
    clarification

  location:
    - shrink_folio_list()

  assignee:
    <none>


- item:
    compaction

  description: >-
    Raised at LSFMM: Compaction skips non-order-0 pages. Already problem for
    page-cache pages today. Is my understand correct?

  location:
    - <where?>

  assignee:
    <none>
---

Thanks,
Ryan
  

On 06.07.23 10:02, Ryan Roberts wrote:
> On 05/07/2023 20:38, David Hildenbrand wrote:
>> On 03.07.23 15:53, Ryan Roberts wrote:
>>> Hi All,
>>>
>>> This is v2 of a series to implement variable order, large folios for anonymous
>>> memory. The objective of this is to improve performance by allocating larger
>>> chunks of memory during anonymous page faults. See [1] for background.
>>>
> 
> [...]
> 
>>> Thanks,
>>> Ryan
>>
>> Hi Ryan,
>>
>> is page migration already working as expected (what about page compaction?), and
>> do we handle migration -ENOMEM when allocating a target page: do we split an
>> fallback to 4k page migration?
>>
> 
> Hi David, All,

Hi Ryan,

thanks a lot for the list.

But can you comment on the page migration part (IOW did you try it already)?

For example, memory hotunplug, CMA, MCE handling, compaction all rely on 
page migration of something that was allocated using GFP_MOVABLE to 
actually work.

Compaction seems to skip any higher-order folios, but the question is if 
the udnerlying migration itself works.

If it already works: great! If not, this really has to be tackled early, 
because otherwise we'll be breaking the GFP_MOVABLE semantics.

> 
> This series aims to be the bare minimum to demonstrate allocation of large anon
> folios. As such, there is a laundry list of things that need to be done for this
> feature to play nicely with other features. My preferred route is to merge this
> with it's Kconfig defaulted to disabled, and its Kconfig description clearly
> shouting that it's EXPERIMENTAL with an explanation of why (similar to
> READ_ONLY_THP_FOR_FS).
As long as we are not sure about the user space control and as long as 
basic functionality is not working (example, page migration), I would 
tend to not merge this early just for the sake of it.

But yes, something like mlock can eventually be tackled later: as long 
as there is a runtime interface to disable it ;)

> 
> That said, I've put together a table of the items that I'm aware of that need
> attention. It would be great if people can review and add any missing items.
> Then we can hopefully parallelize the implementation work. David, I don't think
> the items you raised are covered - would you mind providing a bit more detail so
> I can add them to the list? (or just add them to the list yourself, if you prefer).
> 
> ---
> 
> - item:
>      mlock
> 
>    description: >-
>      Large, pte-mapped folios are ignored when mlock is requested. Code comment
>      for mlock_vma_folio() says "...filter out pte mappings of THPs, which
>      cannot be consistently counted: a pte mapping of the THP head cannot be
>      distinguished by the page alone."
> 
>    location:
>      - mlock_pte_range()
>      - mlock_vma_folio()
> 
>    assignee:
>      Yin, Fengwei
> 
> 
> - item:
>      numa balancing
> 
>    description: >-
>      Large, pte-mapped folios are ignored by numa-balancing code. Commit
>      comment (e81c480): "We're going to have THP mapped with PTEs. It will
>      confuse numabalancing. Let's skip them for now."
> 
>    location:
>      - do_numa_page()
> 
>    assignee:
>      <none>
> 
> 
> - item:
>      madvise
> 
>    description: >-
>      MADV_COLD, MADV_PAGEOUT, MADV_FREE: For large folios, code assumes
>      exclusive only if mapcount==1, else skips remainder of operation. For
>      large, pte-mapped folios, exclusive folios can have mapcount upto nr_pages
>      and still be exclusive. Even better; don't split the folio if it fits
>      entirely within the range? Discussion at
> 
> https://lore.kernel.org/linux-mm/6cec6f68-248e-63b4-5615-9e0f3f819a0a@redhat.com/
>      talks about changing folio mapcounting - may help determine if exclusive
>      without pgtable scan?
> 
>    location:
>      - madvise_cold_or_pageout_pte_range()
>      - madvise_free_pte_range()
> 
>    assignee:
>      <none>
> 
> 
> - item:
>      shrink_folio_list
> 
>    description: >-
>      Raised by Yu Zhao; I can't see the problem in the code - need
>      clarification
> 
>    location:
>      - shrink_folio_list()
> 
>    assignee:
>      <none>
> 
> 
> - item:
>      compaction
> 
>    description: >-
>      Raised at LSFMM: Compaction skips non-order-0 pages. Already problem for
>      page-cache pages today. Is my understand correct?
> 
>    location:
>      - <where?>
> 
>    assignee:
>      <none>

I'm still thinking about the whole mapcount thingy (and I burned way too 
much time on that yesterday), which is a big item for such a list and 
affects some of these items.

A pagetable scan is pretty much irrelevant for order-2 pages. But once 
we're talking about higher orders we really don't want to do that.

I'm preparing a writeup with users and challenges.


Is swapping working as expected? zswap?
  

On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
> On 06.07.23 10:02, Ryan Roberts wrote:
> But can you comment on the page migration part (IOW did you try it already)?
> 
> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
> page migration of something that was allocated using GFP_MOVABLE to actually
> work.
> 
> Compaction seems to skip any higher-order folios, but the question is if the
> udnerlying migration itself works.
> 
> If it already works: great! If not, this really has to be tackled early,
> because otherwise we'll be breaking the GFP_MOVABLE semantics.

I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
is not.

If you look at a function like folio_migrate_mapping(), it all seems
appropriately folio-ised.  There might be something in there that is
slightly wrong, but that would just be a bug to fix, not a huge
architectural problem.

The problem comes in the callers of migrate_pages().  They pass a
new_folio_t callback.  alloc_migration_target() is the usual one passed
and as far as I can tell is fine.  I've seen no problems reported with it.

compaction_alloc() is a disaster, and I don't know how to fix it.
The compaction code has its own allocator which is populated with order-0
folios.  How it populates that freelist is awful ... see split_map_pages()

> Is swapping working as expected? zswap?

Suboptimally.  Swap will split folios in order to swap them.  Somebody
needs to fix that, but it should work.
  

On 07.07.23 15:12, Matthew Wilcox wrote:
> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>> On 06.07.23 10:02, Ryan Roberts wrote:
>> But can you comment on the page migration part (IOW did you try it already)?
>>
>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>> page migration of something that was allocated using GFP_MOVABLE to actually
>> work.
>>
>> Compaction seems to skip any higher-order folios, but the question is if the
>> udnerlying migration itself works.
>>
>> If it already works: great! If not, this really has to be tackled early,
>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
> 
> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
> is not.

Thanks! Very nice if at least ordinary migration works.

> 
> If you look at a function like folio_migrate_mapping(), it all seems
> appropriately folio-ised.  There might be something in there that is
> slightly wrong, but that would just be a bug to fix, not a huge
> architectural problem.
> 
> The problem comes in the callers of migrate_pages().  They pass a
> new_folio_t callback.  alloc_migration_target() is the usual one passed
> and as far as I can tell is fine.  I've seen no problems reported with it.
> 
> compaction_alloc() is a disaster, and I don't know how to fix it.
> The compaction code has its own allocator which is populated with order-0
> folios.  How it populates that freelist is awful ... see split_map_pages()

Yeah, all that code was written under the assumption that we're moving 
order-0 pages (which is what the anon+pagecache pages part).

 From what I recall, we're allocating order-0 pages from the high memory 
addresses, so we can migrate from low memory addresses, effectively 
freeing up low memory addresses and filling high memory addresses.

Adjusting that will be ... interesting. Instead of allocating order-0 
pages from high addresses, we might want to allocate "as large as 
possible" ("grab what we can") from high addresses and then have our own 
kind of buddy for allocating from that pool a compaction destination 
page, depending on our source page. Nasty.

What should always work is the split->migrate. But that's definitely not 
what we want in many cases.

> 
>> Is swapping working as expected? zswap?
> 
> Suboptimally.  Swap will split folios in order to swap them.  Somebody
> needs to fix that, but it should work.

Good!

It would be great to have some kind of a feature matrix that tells us 
what works perfectly, sub-optimally, barely, not at all (and what has 
not been tested). Maybe (likely!) we'll also find things that are 
sub-optimal for ordinary THP (like swapping, not even sure about).

I suspect that KSM should work mostly fine with flexible-thp. When 
deduplciating, we'll simply split the compound page and proceed as 
expected. But might be worth testing as well.
  

On 07/07/2023 14:24, David Hildenbrand wrote:
> On 07.07.23 15:12, Matthew Wilcox wrote:
>> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>>> On 06.07.23 10:02, Ryan Roberts wrote:
>>> But can you comment on the page migration part (IOW did you try it already)?
>>>
>>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>>> page migration of something that was allocated using GFP_MOVABLE to actually
>>> work.
>>>
>>> Compaction seems to skip any higher-order folios, but the question is if the
>>> udnerlying migration itself works.
>>>
>>> If it already works: great! If not, this really has to be tackled early,
>>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
>>
>> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
>> is not.
> 
> Thanks! Very nice if at least ordinary migration works.

That's good to hear - I hadn't personally investigated.

> 
>>
>> If you look at a function like folio_migrate_mapping(), it all seems
>> appropriately folio-ised.  There might be something in there that is
>> slightly wrong, but that would just be a bug to fix, not a huge
>> architectural problem.
>>
>> The problem comes in the callers of migrate_pages().  They pass a
>> new_folio_t callback.  alloc_migration_target() is the usual one passed
>> and as far as I can tell is fine.  I've seen no problems reported with it.
>>
>> compaction_alloc() is a disaster, and I don't know how to fix it.
>> The compaction code has its own allocator which is populated with order-0
>> folios.  How it populates that freelist is awful ... see split_map_pages()

I think this compaction issue also affects large folios in the page cache? So
really it is a pre-existing bug in the code base that needs to be fixed
independently of large anon folios? Should I assume you are tackling this, Matthew?

> 
> Yeah, all that code was written under the assumption that we're moving order-0
> pages (which is what the anon+pagecache pages part).
> 
> From what I recall, we're allocating order-0 pages from the high memory
> addresses, so we can migrate from low memory addresses, effectively freeing up
> low memory addresses and filling high memory addresses.
> 
> Adjusting that will be ... interesting. Instead of allocating order-0 pages from
> high addresses, we might want to allocate "as large as possible" ("grab what we
> can") from high addresses and then have our own kind of buddy for allocating
> from that pool a compaction destination page, depending on our source page. Nasty.
> 
> What should always work is the split->migrate. But that's definitely not what we
> want in many cases.
> 
>>
>>> Is swapping working as expected? zswap?
>>
>> Suboptimally.  Swap will split folios in order to swap them.  Somebody
>> needs to fix that, but it should work.
> 
> Good!
> 
> It would be great to have some kind of a feature matrix that tells us what works
> perfectly, sub-optimally, barely, not at all (and what has not been tested).
> Maybe (likely!) we'll also find things that are sub-optimal for ordinary THP
> (like swapping, not even sure about).

I'm building a list of known issues, but so far it has been based on code I've
found during review and things raised by people in these threads. Are there test
suites that explicitly test these features? If so I'll happily run them against
large anon folios, but at the moment I'm ignorant I'm afraid. I have been trying
to get mm selftests up and running, but I currently have a bunch of failures on
arm64, even without any of my patches - somthing I'm working through.

> 
> I suspect that KSM should work mostly fine with flexible-thp. When
> deduplciating, we'll simply split the compound page and proceed as expected. But
> might be worth testing as well.
>
  

On 7 Jul 2023, at 9:24, David Hildenbrand wrote:

> On 07.07.23 15:12, Matthew Wilcox wrote:
>> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>>> On 06.07.23 10:02, Ryan Roberts wrote:
>>> But can you comment on the page migration part (IOW did you try it already)?
>>>
>>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>>> page migration of something that was allocated using GFP_MOVABLE to actually
>>> work.
>>>
>>> Compaction seems to skip any higher-order folios, but the question is if the
>>> udnerlying migration itself works.
>>>
>>> If it already works: great! If not, this really has to be tackled early,
>>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
>>
>> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
>> is not.
>
> Thanks! Very nice if at least ordinary migration works.
>
>>
>> If you look at a function like folio_migrate_mapping(), it all seems
>> appropriately folio-ised.  There might be something in there that is
>> slightly wrong, but that would just be a bug to fix, not a huge
>> architectural problem.
>>
>> The problem comes in the callers of migrate_pages().  They pass a
>> new_folio_t callback.  alloc_migration_target() is the usual one passed
>> and as far as I can tell is fine.  I've seen no problems reported with it.
>>
>> compaction_alloc() is a disaster, and I don't know how to fix it.
>> The compaction code has its own allocator which is populated with order-0
>> folios.  How it populates that freelist is awful ... see split_map_pages()
>
> Yeah, all that code was written under the assumption that we're moving order-0 pages (which is what the anon+pagecache pages part).
>
> From what I recall, we're allocating order-0 pages from the high memory addresses, so we can migrate from low memory addresses, effectively freeing up low memory addresses and filling high memory addresses.
>
> Adjusting that will be ... interesting. Instead of allocating order-0 pages from high addresses, we might want to allocate "as large as possible" ("grab what we can") from high addresses and then have our own kind of buddy for allocating from that pool a compaction destination page, depending on our source page. Nasty.

We probably do not need a pool, since before migration, we have isolated folios to
be migrated and can come up with a stats on how many folios there are at each order.
Then, we can isolate free pages based on the stats and do not split free pages
all the way down to order-0. We can sort the source folios based on their orders
and isolate free pages from largest order to smallest order. That could avoid
a free page pool.

--
Best Regards,
Yan, Zi
  

On Mon, Jul 10, 2023 at 11:07:47AM +0100, Ryan Roberts wrote:
> I think this compaction issue also affects large folios in the page cache? So
> really it is a pre-existing bug in the code base that needs to be fixed
> independently of large anon folios? Should I assume you are tackling this, Matthew?

It does need to be fixed independently of large anon folios.  Said fix
should probably be backported to 6.1 once it's suitably stable.  However,
I'm not working on it.  I have a lot of projects and this one's a
missed-opportunity, not a show-stopper.  Sounds like Zi Yan might be
interested in tackling it though!
  

On Fri, Jul 07, 2023 at 02:12:01PM +0100, Matthew Wilcox wrote:
> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
> 
> > Is swapping working as expected? zswap?
> 
> Suboptimally.  Swap will split folios in order to swap them.

Wouldn't that mean if high order folios are used a lot but swap is also
used, until this is fixed you wouldn't get the expected reclaim gains
for high order folios and we'd need compaction more then?

> Somebody needs to fix that, but it should work.

As we look at shmem stuff it was on the path so something we have
considered doing. Ie, it's on our team's list of items to help with
but currently on a backburner.

  Luis
  

On Tue, Jul 11, 2023 at 02:11:19PM -0700, Luis Chamberlain wrote:
> On Fri, Jul 07, 2023 at 02:12:01PM +0100, Matthew Wilcox wrote:
> > On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
> > 
> > > Is swapping working as expected? zswap?
> > 
> > Suboptimally.  Swap will split folios in order to swap them.
> 
> Wouldn't that mean if high order folios are used a lot but swap is also
> used, until this is fixed you wouldn't get the expected reclaim gains
> for high order folios and we'd need compaction more then?

They're split in shrink_folio_list(), so they stay intact until
that point?

> > Somebody needs to fix that, but it should work.
> 
> As we look at shmem stuff it was on the path so something we have
> considered doing. Ie, it's on our team's list of items to help with
> but currently on a backburner.

Something I was thinking about is that you'll need to prohibit swap
devices or swap files being created on large block devices.  Until
we rewrite the entire swap subsystem ...
  

On 10/07/2023 17:53, Zi Yan wrote:
> On 7 Jul 2023, at 9:24, David Hildenbrand wrote:
> 
>> On 07.07.23 15:12, Matthew Wilcox wrote:
>>> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>>>> On 06.07.23 10:02, Ryan Roberts wrote:
>>>> But can you comment on the page migration part (IOW did you try it already)?
>>>>
>>>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>>>> page migration of something that was allocated using GFP_MOVABLE to actually
>>>> work.
>>>>
>>>> Compaction seems to skip any higher-order folios, but the question is if the
>>>> udnerlying migration itself works.
>>>>
>>>> If it already works: great! If not, this really has to be tackled early,
>>>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
>>>
>>> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
>>> is not.
>>
>> Thanks! Very nice if at least ordinary migration works.
>>
>>>
>>> If you look at a function like folio_migrate_mapping(), it all seems
>>> appropriately folio-ised.  There might be something in there that is
>>> slightly wrong, but that would just be a bug to fix, not a huge
>>> architectural problem.
>>>
>>> The problem comes in the callers of migrate_pages().  They pass a
>>> new_folio_t callback.  alloc_migration_target() is the usual one passed
>>> and as far as I can tell is fine.  I've seen no problems reported with it.
>>>
>>> compaction_alloc() is a disaster, and I don't know how to fix it.
>>> The compaction code has its own allocator which is populated with order-0
>>> folios.  How it populates that freelist is awful ... see split_map_pages()
>>
>> Yeah, all that code was written under the assumption that we're moving order-0 pages (which is what the anon+pagecache pages part).
>>
>> From what I recall, we're allocating order-0 pages from the high memory addresses, so we can migrate from low memory addresses, effectively freeing up low memory addresses and filling high memory addresses.
>>
>> Adjusting that will be ... interesting. Instead of allocating order-0 pages from high addresses, we might want to allocate "as large as possible" ("grab what we can") from high addresses and then have our own kind of buddy for allocating from that pool a compaction destination page, depending on our source page. Nasty.
> 
> We probably do not need a pool, since before migration, we have isolated folios to
> be migrated and can come up with a stats on how many folios there are at each order.
> Then, we can isolate free pages based on the stats and do not split free pages
> all the way down to order-0. We can sort the source folios based on their orders
> and isolate free pages from largest order to smallest order. That could avoid
> a free page pool.

Hi Zi, I just wanted to check; is this something you are working on or planning
to work on? I'm trying to maintain a list of all the items that need to get
sorted for large anon folios. It would be great to put your name against it! ;-)

> 
> --
> Best Regards,
> Yan, Zi
  

On 19 Jul 2023, at 11:49, Ryan Roberts wrote:

> On 10/07/2023 17:53, Zi Yan wrote:
>> On 7 Jul 2023, at 9:24, David Hildenbrand wrote:
>>
>>> On 07.07.23 15:12, Matthew Wilcox wrote:
>>>> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>>>>> On 06.07.23 10:02, Ryan Roberts wrote:
>>>>> But can you comment on the page migration part (IOW did you try it already)?
>>>>>
>>>>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>>>>> page migration of something that was allocated using GFP_MOVABLE to actually
>>>>> work.
>>>>>
>>>>> Compaction seems to skip any higher-order folios, but the question is if the
>>>>> udnerlying migration itself works.
>>>>>
>>>>> If it already works: great! If not, this really has to be tackled early,
>>>>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
>>>>
>>>> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
>>>> is not.
>>>
>>> Thanks! Very nice if at least ordinary migration works.
>>>
>>>>
>>>> If you look at a function like folio_migrate_mapping(), it all seems
>>>> appropriately folio-ised.  There might be something in there that is
>>>> slightly wrong, but that would just be a bug to fix, not a huge
>>>> architectural problem.
>>>>
>>>> The problem comes in the callers of migrate_pages().  They pass a
>>>> new_folio_t callback.  alloc_migration_target() is the usual one passed
>>>> and as far as I can tell is fine.  I've seen no problems reported with it.
>>>>
>>>> compaction_alloc() is a disaster, and I don't know how to fix it.
>>>> The compaction code has its own allocator which is populated with order-0
>>>> folios.  How it populates that freelist is awful ... see split_map_pages()
>>>
>>> Yeah, all that code was written under the assumption that we're moving order-0 pages (which is what the anon+pagecache pages part).
>>>
>>> From what I recall, we're allocating order-0 pages from the high memory addresses, so we can migrate from low memory addresses, effectively freeing up low memory addresses and filling high memory addresses.
>>>
>>> Adjusting that will be ... interesting. Instead of allocating order-0 pages from high addresses, we might want to allocate "as large as possible" ("grab what we can") from high addresses and then have our own kind of buddy for allocating from that pool a compaction destination page, depending on our source page. Nasty.
>>
>> We probably do not need a pool, since before migration, we have isolated folios to
>> be migrated and can come up with a stats on how many folios there are at each order.
>> Then, we can isolate free pages based on the stats and do not split free pages
>> all the way down to order-0. We can sort the source folios based on their orders
>> and isolate free pages from largest order to smallest order. That could avoid
>> a free page pool.
>
> Hi Zi, I just wanted to check; is this something you are working on or planning
> to work on? I'm trying to maintain a list of all the items that need to get
> sorted for large anon folios. It would be great to put your name against it! ;-)

Sure. I can work on this one.

--
Best Regards,
Yan, Zi
  

On 19/07/2023 17:05, Zi Yan wrote:
> On 19 Jul 2023, at 11:49, Ryan Roberts wrote:
> 
>> On 10/07/2023 17:53, Zi Yan wrote:
>>> On 7 Jul 2023, at 9:24, David Hildenbrand wrote:
>>>
>>>> On 07.07.23 15:12, Matthew Wilcox wrote:
>>>>> On Fri, Jul 07, 2023 at 01:40:53PM +0200, David Hildenbrand wrote:
>>>>>> On 06.07.23 10:02, Ryan Roberts wrote:
>>>>>> But can you comment on the page migration part (IOW did you try it already)?
>>>>>>
>>>>>> For example, memory hotunplug, CMA, MCE handling, compaction all rely on
>>>>>> page migration of something that was allocated using GFP_MOVABLE to actually
>>>>>> work.
>>>>>>
>>>>>> Compaction seems to skip any higher-order folios, but the question is if the
>>>>>> udnerlying migration itself works.
>>>>>>
>>>>>> If it already works: great! If not, this really has to be tackled early,
>>>>>> because otherwise we'll be breaking the GFP_MOVABLE semantics.
>>>>>
>>>>> I have looked at this a bit.  _Migration_ should be fine.  _Compaction_
>>>>> is not.
>>>>
>>>> Thanks! Very nice if at least ordinary migration works.
>>>>
>>>>>
>>>>> If you look at a function like folio_migrate_mapping(), it all seems
>>>>> appropriately folio-ised.  There might be something in there that is
>>>>> slightly wrong, but that would just be a bug to fix, not a huge
>>>>> architectural problem.
>>>>>
>>>>> The problem comes in the callers of migrate_pages().  They pass a
>>>>> new_folio_t callback.  alloc_migration_target() is the usual one passed
>>>>> and as far as I can tell is fine.  I've seen no problems reported with it.
>>>>>
>>>>> compaction_alloc() is a disaster, and I don't know how to fix it.
>>>>> The compaction code has its own allocator which is populated with order-0
>>>>> folios.  How it populates that freelist is awful ... see split_map_pages()
>>>>
>>>> Yeah, all that code was written under the assumption that we're moving order-0 pages (which is what the anon+pagecache pages part).
>>>>
>>>> From what I recall, we're allocating order-0 pages from the high memory addresses, so we can migrate from low memory addresses, effectively freeing up low memory addresses and filling high memory addresses.
>>>>
>>>> Adjusting that will be ... interesting. Instead of allocating order-0 pages from high addresses, we might want to allocate "as large as possible" ("grab what we can") from high addresses and then have our own kind of buddy for allocating from that pool a compaction destination page, depending on our source page. Nasty.
>>>
>>> We probably do not need a pool, since before migration, we have isolated folios to
>>> be migrated and can come up with a stats on how many folios there are at each order.
>>> Then, we can isolate free pages based on the stats and do not split free pages
>>> all the way down to order-0. We can sort the source folios based on their orders
>>> and isolate free pages from largest order to smallest order. That could avoid
>>> a free page pool.
>>
>> Hi Zi, I just wanted to check; is this something you are working on or planning
>> to work on? I'm trying to maintain a list of all the items that need to get
>> sorted for large anon folios. It would be great to put your name against it! ;-)
> 
> Sure. I can work on this one.

Awesome - thanks!

> 
> --
> Best Regards,
> Yan, Zi