Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230703135330.1865927-1-ryan.roberts@arm.com> <20230703135330.1865927-6-ryan.roberts@arm.com>
In-Reply-To: <20230703135330.1865927-6-ryan.roberts@arm.com>
From:   Yu Zhao <yuzhao@google.com>
Date:   Mon, 3 Jul 2023 14:02:13 -0600
Message-ID: <CAOUHufa_FkK0BNhpfMhexFetwhbugjVcKOBKpu4aT-0n8NeAxg@mail.gmail.com>
Subject: Re: [PATCH v2 5/5] arm64: mm: Override arch_wants_pte_order()
To:     Ryan Roberts <ryan.roberts@arm.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Yin Fengwei <fengwei.yin@intel.com>,
        David Hildenbrand <david@redhat.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Yang Shi <shy828301@gmail.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jul 3, 2023 at 7:53=E2=80=AFAM Ryan Roberts <ryan.roberts@arm.com> =
wrote:
>
> Define an arch-specific override of arch_wants_pte_order() so that when
> FLEXIBLE_THP is enabled, large folios will be allocated for anonymous
> memory with an order that is compatible with arm64's contpte mappings.
>
> arch_wants_pte_order() returns an order according to the following
> policy: For the unhinted case, when THP is not requested for the vma,
> don't allow anything bigger than 64K. This means we don't waste too much
> memory. Additionally, for 4K pages this is the contpte size, and for
> 16K, this is (usually) the HPA size when the uarch feature is
> implemented. For the hinted case, when THP is requested for the vma,
> allow the contpte size for all page size configurations; 64K for 4K, 2M
> for 16K and 2M for 64K.
>
> Additionally, the THP and NOTHP order constants are defined using
> Kconfig so it is possible to override them at build time.
>
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>  arch/arm64/Kconfig               | 12 ++++++++++++
>  arch/arm64/include/asm/pgtable.h |  4 ++++
>  arch/arm64/mm/mmu.c              |  8 ++++++++
>  3 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 343e1e1cae10..689c5bf13dc1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -281,6 +281,18 @@ config ARM64_CONT_PMD_SHIFT
>         default 5 if ARM64_16K_PAGES
>         default 4
>
> +config ARM64_PTE_ORDER_NOTHP
> +       int
> +       default 0 if ARM64_64K_PAGES    # 64K (1 page)
> +       default 2 if ARM64_16K_PAGES    # 64K (4 pages; benefits from HPA=
 where HW supports it)
> +       default 4 if ARM64_4K_PAGES     # 64K (16 pages; eligible for con=
tpte-mapping)
> +
> +config ARM64_PTE_ORDER_THP
> +       int
> +       default 5 if ARM64_64K_PAGES    # 2M  (32 pages; eligible for con=
tpte-mapping)
> +       default 7 if ARM64_16K_PAGES    # 2M  (128 pages; eligible for co=
ntpte-mapping)
> +       default 4 if ARM64_4K_PAGES     # 64K (16 pages; eligible for con=
tpte-mapping)
> +
>  config ARCH_MMAP_RND_BITS_MIN
>         default 14 if ARM64_64K_PAGES
>         default 16 if ARM64_16K_PAGES
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pg=
table.h
> index 6fd012663a01..8463d5f9f307 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -1117,6 +1117,10 @@ extern pte_t ptep_modify_prot_start(struct vm_area=
_struct *vma,
>  extern void ptep_modify_prot_commit(struct vm_area_struct *vma,
>                                     unsigned long addr, pte_t *ptep,
>                                     pte_t old_pte, pte_t new_pte);
> +
> +#define arch_wants_pte_order arch_wants_pte_order
> +extern int arch_wants_pte_order(struct vm_area_struct *vma);
> +
>  #endif /* !__ASSEMBLY__ */
>
>  #endif /* __ASM_PGTABLE_H */
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index af6bc8403ee4..8556c4a9b507 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1481,3 +1481,11 @@ void ptep_modify_prot_commit(struct vm_area_struct=
 *vma, unsigned long addr, pte
>  {
>         set_pte_at(vma->vm_mm, addr, ptep, pte);
>  }
> +
> +int arch_wants_pte_order(struct vm_area_struct *vma)
> +{
> +       if (hugepage_vma_check(vma, vma->vm_flags, false, true, true))
> +               return CONFIG_ARM64_PTE_ORDER_THP;
> +       else
> +               return CONFIG_ARM64_PTE_ORDER_NOTHP;
> +}

I don't really like this because it's a mix of h/w preference and s/w
policy -- from my POV, it's supposed to be the former only. The policy
part should be left to core MM (arch-independent).

That being said, no objection if ARM MM people think this is really
what they want.