[RFC PATCH v3 03/16] mm: Provide mm_struct and address to huge_ptep_get()
On powerpc 8xx huge_ptep_get() will need to know whether the given
ptep is a PTE entry or a PMD entry. This cannot be known with the
PMD entry itself because there is no easy way to know it from the
content of the entry.
So huge_ptep_get() will need to know either the size of the page
or get the pmd.
In order to be consistent with huge_ptep_get_and_clear(), give
mm and address to huge_ptep_get().
Signed-off-by: Christophe Leroy
---
v2: Add missing changes in arch implementations
v3: Fixed a comment in ARM and missing changes in S390
---
arch/arm/include/asm/hugetlb-3level.h | 4 +--
arch/arm64/include/asm/hugetlb.h | 2 +-
arch/arm64/mm/hugetlbpage.c | 2 +-
arch/riscv/include/asm/hugetlb.h | 2 +-
arch/riscv/mm/hugetlbpage.c | 2 +-
arch/s390/include/asm/hugetlb.h | 4 +--
arch/s390/mm/hugetlbpage.c| 4 +--
fs/hugetlbfs/inode.c | 2 +-
fs/proc/task_mmu.c| 8 ++---
fs/userfaultfd.c | 2 +-
include/asm-generic/hugetlb.h | 2 +-
include/linux/swapops.h | 2 +-
mm/damon/vaddr.c | 6 ++--
mm/gup.c | 2 +-
mm/hmm.c | 2 +-
mm/hugetlb.c | 46 +--
mm/memory-failure.c | 2 +-
mm/mempolicy.c| 2 +-
mm/migrate.c | 4 +--
mm/mincore.c | 2 +-
mm/userfaultfd.c | 2 +-
21 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/arch/arm/include/asm/hugetlb-3level.h
b/arch/arm/include/asm/hugetlb-3level.h
index a30be5505793..87d48e2d90ad 100644
--- a/arch/arm/include/asm/hugetlb-3level.h
+++ b/arch/arm/include/asm/hugetlb-3level.h
@@ -13,12 +13,12 @@
/*
* If our huge pte is non-zero then mark the valid bit.
- * This allows pte_present(huge_ptep_get(ptep)) to return true for non-zero
+ * This allows pte_present(huge_ptep_get(mm,addr,ptep)) to return true for
non-zero
* ptes.
* (The valid bit is automatically cleared by set_pte_at for PROT_NONE ptes).
*/
#define __HAVE_ARCH_HUGE_PTEP_GET
-static inline pte_t huge_ptep_get(pte_t *ptep)
+static inline pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr,
pte_t *ptep)
{
pte_t retval = *ptep;
if (pte_val(retval))
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index 2ddc33d93b13..1af39a74e791 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -46,7 +46,7 @@ extern pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
extern void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, unsigned long sz);
#define __HAVE_ARCH_HUGE_PTEP_GET
-extern pte_t huge_ptep_get(pte_t *ptep);
+extern pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t
*ptep);
void __init arm64_hugetlb_cma_reserve(void);
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index b872b003a55f..19c4abde13a3 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -141,7 +141,7 @@ static inline int num_contig_ptes(unsigned long size,
size_t *pgsize)
return contig_ptes;
}
-pte_t huge_ptep_get(pte_t *ptep)
+pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
{
int ncontig, i;
size_t pgsize;
diff --git a/arch/riscv/include/asm/hugetlb.h b/arch/riscv/include/asm/hugetlb.h
index 22deb7a2a6ec..6321bca08740 100644
--- a/arch/riscv/include/asm/hugetlb.h
+++ b/arch/riscv/include/asm/hugetlb.h
@@ -44,7 +44,7 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
pte_t pte, int dirty);
#define __HAVE_ARCH_HUGE_PTEP_GET
-pte_t huge_ptep_get(pte_t *ptep);
+pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep);
pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags);
#define arch_make_huge_pte arch_make_huge_pte
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index 5ef2a6891158..20bf499044b7 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -3,7 +3,7 @@
#include
#ifdef CONFIG_RISCV_ISA_SVNAPOT
-pte_t huge_ptep_get(pte_t *ptep)
+pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
{
unsigned long pte_num;
int i;
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
index deb198a61039..3b4835094fd5 100644
--- a/arch/s390/include/asm/hugetlb.h
+++ b/arch/s390/include/asm/hugetlb.h
@@ -19,7 +19,7 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned long sz);
void __set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte);
-pte_t huge_ptep_get(pte_t *ptep);
+pte
Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to huge_ptep_get()
On Sun, May 26, 2024 at 11:22:23AM +0200, Christophe Leroy wrote: > On powerpc 8xx huge_ptep_get() will need to know whether the given > ptep is a PTE entry or a PMD entry. This cannot be known with the > PMD entry itself because there is no easy way to know it from the > content of the entry. > > So huge_ptep_get() will need to know either the size of the page > or get the pmd. > > In order to be consistent with huge_ptep_get_and_clear(), give > mm and address to huge_ptep_get(). > > Signed-off-by: Christophe Leroy > --- > v2: Add missing changes in arch implementations > v3: Fixed a comment in ARM and missing changes in S390 > --- > arch/arm/include/asm/hugetlb-3level.h | 4 +-- > arch/arm64/include/asm/hugetlb.h | 2 +- > arch/arm64/mm/hugetlbpage.c | 2 +- > arch/riscv/include/asm/hugetlb.h | 2 +- > arch/riscv/mm/hugetlbpage.c | 2 +- > arch/s390/include/asm/hugetlb.h | 4 +-- > arch/s390/mm/hugetlbpage.c| 4 +-- I was wondering whether we could do something similar for what we did in patch#1, so we do not touch architectures code. > diff --git a/mm/gup.c b/mm/gup.c > index 1611e73b1121..86b5105b82a1 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2812,7 +2812,7 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, > unsigned long addr, > if (pte_end < end) > end = pte_end; > > - pte = huge_ptep_get(ptep); > + pte = huge_ptep_get(NULL, addr, ptep); I know that after this series all this code is gone, but I was not sure about the behaviour between this patch and the last one. It made me nervous, until I realized that this code is only used on CONFIG_ARCH_HAS_HUGEPD, which should not be the case anymore for 8xx after patch#8, and since 8xx is the only one that will use the mm parameter from huge_ptep_get, we are all good. -- Oscar Salvador SUSE Labs
Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to huge_ptep_get()
Le 27/05/2024 à 13:19, Oscar Salvador a écrit :
> On Sun, May 26, 2024 at 11:22:23AM +0200, Christophe Leroy wrote:
>> On powerpc 8xx huge_ptep_get() will need to know whether the given
>> ptep is a PTE entry or a PMD entry. This cannot be known with the
>> PMD entry itself because there is no easy way to know it from the
>> content of the entry.
>>
>> So huge_ptep_get() will need to know either the size of the page
>> or get the pmd.
>>
>> In order to be consistent with huge_ptep_get_and_clear(), give
>> mm and address to huge_ptep_get().
>>
>> Signed-off-by: Christophe Leroy
>> ---
>> v2: Add missing changes in arch implementations
>> v3: Fixed a comment in ARM and missing changes in S390
>> ---
>> arch/arm/include/asm/hugetlb-3level.h | 4 +--
>> arch/arm64/include/asm/hugetlb.h | 2 +-
>> arch/arm64/mm/hugetlbpage.c | 2 +-
>> arch/riscv/include/asm/hugetlb.h | 2 +-
>> arch/riscv/mm/hugetlbpage.c | 2 +-
>> arch/s390/include/asm/hugetlb.h | 4 +--
>> arch/s390/mm/hugetlbpage.c| 4 +--
>
> I was wondering whether we could do something similar for what we did in
> patch#1, so we do not touch architectures code.
We could be is that worth the churn ?
With patch 1 there was only one callsite.
Here we have many callsites, and we also have huge_ptep_get_and_clear()
which already takes three arguments. So for me it make more sense to
adapt huge_ptep_get() here.
Today several of the huge-related functions already have parameters that
are used only by a few architectures and everytime one architecture
needs a new parameter it is added for all of them, and there are
exemples in the past of new functions added to get new parameters for
only a few architectures that ended up with a mess and a need to
re-factor at the end.
See for instance the story around arch_make_huge_pte() and pte_mkhuge(),
both do the same but arch_make_huge_pte() was added to take additional
parameters by commit d9ed9faac283 ("mm: add new arch_make_huge_pte()
method for tile support") then they were merged by commit 16785bd77431
("mm: merge pte_mkhuge() call into arch_make_huge_pte()")
So I'm open to any suggestion but we need to try not make it a bigger
mess at the end.
By the way, I think most if not all huge related helpers should all take
the same parameters even if not all of them are used, then it would make
things easier. And maybe the cleanest would be to give the page size to
all those functions instead of having them guess it.
So let's have your ideas here on the most straight forward way to handle
that.
>
>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index 1611e73b1121..86b5105b82a1 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2812,7 +2812,7 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz,
>> unsigned long addr,
>> if (pte_end < end)
>> end = pte_end;
>>
>> -pte = huge_ptep_get(ptep);
>> +pte = huge_ptep_get(NULL, addr, ptep);
>
> I know that after this series all this code is gone, but I was not sure
> about the behaviour between this patch and the last one.
>
> It made me nervous, until I realized that this code is only used
> on CONFIG_ARCH_HAS_HUGEPD, which should not be the case anymore for 8xx after
> patch#8, and since 8xx is the only one that will use the mm parameter from
> huge_ptep_get, we are all good.
>
By the way, after commit 01d89b93e176 ("mm/gup: fix hugepd handling in
hugetlb rework") we now have the vma in gup_hugepte() so we now pass
vma->vm_mm
Thanks for the review
Christophe
Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to huge_ptep_get()
On Mon, May 27, 2024 at 03:51:41PM +, Christophe Leroy wrote:
> We could be is that worth the churn ?
Probably not.
> With patch 1 there was only one callsite.
Yes, you are right here.
> Here we have many callsites, and we also have huge_ptep_get_and_clear()
> which already takes three arguments. So for me it make more sense to
> adapt huge_ptep_get() here.
>
> Today several of the huge-related functions already have parameters that
> are used only by a few architectures and everytime one architecture
> needs a new parameter it is added for all of them, and there are
> exemples in the past of new functions added to get new parameters for
> only a few architectures that ended up with a mess and a need to
> re-factor at the end.
>
> See for instance the story around arch_make_huge_pte() and pte_mkhuge(),
> both do the same but arch_make_huge_pte() was added to take additional
> parameters by commit d9ed9faac283 ("mm: add new arch_make_huge_pte()
> method for tile support") then they were merged by commit 16785bd77431
> ("mm: merge pte_mkhuge() call into arch_make_huge_pte()")
>
> So I'm open to any suggestion but we need to try not make it a bigger
> mess at the end.
>
> By the way, I think most if not all huge related helpers should all take
> the same parameters even if not all of them are used, then it would make
> things easier. And maybe the cleanest would be to give the page size to
> all those functions instead of having them guess it.
>
> So let's have your ideas here on the most straight forward way to handle
> that.
It is probably not worth pursuing this then.
As you said, there are many callers and we would have to create some kind of
hook
for only those interested places, which I guess would end up looking just too
ugly
in order to save little code in arch code.
So please disregard my comment here, and stick with what we have.
> By the way, after commit 01d89b93e176 ("mm/gup: fix hugepd handling in
> hugetlb rework") we now have the vma in gup_hugepte() so we now pass
> vma->vm_mm
I did not notice, thanks.
--
Oscar Salvador
SUSE Labs
