| Message ID | 814e20c19b110209ee12ecae7cb05f8a78d021c8.1653625820.git.baolin.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/hugetlb: Fix building errors in huge_ptep_clear_flush() | expand |
Hi Baolin, On 5/27/22 12:51 PM, Baolin Wang wrote: > Fix below building errors which was caused by commit ae07562909f3 > ("mm: change huge_ptep_clear_flush() to return the original pte") > interacting with commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush > from get_clear_flush()"). > > Due to the new get_clear_contig() has dropped TLB flush, we should > add an explicit TLB flush in huge_ptep_clear_flush() to keep original > semantics when changing to use new get_clear_contig(). > > " > arch/arm64/mm/hugetlbpage.c: In function ‘huge_ptep_clear_flush’: > arch/arm64/mm/hugetlbpage.c:515:9: error: implicit declaration of > function ‘get_clear_flush’; did you mean ‘ptep_clear_flush’? > [-Werror=implicit-function-declaration] > 515 | return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); > | ^~~~~~~~~~~~~~~ > | ptep_clear_flush > " > > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > Suggested-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > --- > arch/arm64/mm/hugetlbpage.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > I ran to the compiling failure either and it would be caused by commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from get_clear_flush()"). It's worthy to have a "Fixes" tag. With those fixed: Reviewed-by: Gavin Shan <gshan@redhat.com> > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c > index 0f0c17dfeb9c..e2a5ec9fdc0d 100644 > --- a/arch/arm64/mm/hugetlbpage.c > +++ b/arch/arm64/mm/hugetlbpage.c > @@ -507,12 +507,15 @@ pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, > { > size_t pgsize; > int ncontig; > + pte_t orig_pte; > > if (!pte_cont(READ_ONCE(*ptep))) > return ptep_clear_flush(vma, addr, ptep); > > ncontig = find_num_contig(vma->vm_mm, addr, ptep, &pgsize); > - return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); > + orig_pte = get_clear_contig(vma->vm_mm, addr, ptep, pgsize, ncontig); > + flush_tlb_range(vma, addr, addr + pgsize * ncontig); > + return orig_pte; > } > > static int __init hugetlbpage_init(void) > Thanks, Gavin
On 5/27/2022 3:12 PM, Gavin Shan wrote: > Hi Baolin, > > On 5/27/22 12:51 PM, Baolin Wang wrote: >> Fix below building errors which was caused by commit ae07562909f3 >> ("mm: change huge_ptep_clear_flush() to return the original pte") >> interacting with commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush >> from get_clear_flush()"). >> >> Due to the new get_clear_contig() has dropped TLB flush, we should >> add an explicit TLB flush in huge_ptep_clear_flush() to keep original >> semantics when changing to use new get_clear_contig(). >> >> " >> arch/arm64/mm/hugetlbpage.c: In function ‘huge_ptep_clear_flush’: >> arch/arm64/mm/hugetlbpage.c:515:9: error: implicit declaration of >> function ‘get_clear_flush’; did you mean ‘ptep_clear_flush’? >> [-Werror=implicit-function-declaration] >> 515 | return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, >> ncontig); >> | ^~~~~~~~~~~~~~~ >> | ptep_clear_flush >> " >> >> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> >> Suggested-by: Catalin Marinas <catalin.marinas@arm.com> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Anshuman Khandual <anshuman.khandual@arm.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> --- >> arch/arm64/mm/hugetlbpage.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> > > I ran to the compiling failure either and it would be caused by > commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from > get_clear_flush()"). > It's worthy to have a "Fixes" tag. With those fixed: Thanks for reminding. IMHO, better to add 2 related commits' fix tag. Linus, could you help to add them when applying this patch, or need a resend? Thanks. Fixes: ae07562909f3 ("mm: change huge_ptep_clear_flush() to return the original pte") Fixes: fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from get_clear_flush()"). > > Reviewed-by: Gavin Shan <gshan@redhat.com> > > >> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c >> index 0f0c17dfeb9c..e2a5ec9fdc0d 100644 >> --- a/arch/arm64/mm/hugetlbpage.c >> +++ b/arch/arm64/mm/hugetlbpage.c >> @@ -507,12 +507,15 @@ pte_t huge_ptep_clear_flush(struct >> vm_area_struct *vma, >> { >> size_t pgsize; >> int ncontig; >> + pte_t orig_pte; >> if (!pte_cont(READ_ONCE(*ptep))) >> return ptep_clear_flush(vma, addr, ptep); >> ncontig = find_num_contig(vma->vm_mm, addr, ptep, &pgsize); >> - return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); >> + orig_pte = get_clear_contig(vma->vm_mm, addr, ptep, pgsize, >> ncontig); >> + flush_tlb_range(vma, addr, addr + pgsize * ncontig); >> + return orig_pte; >> } >> static int __init hugetlbpage_init(void) >> > > Thanks, > Gavin
On 5/27/22 14:12, Baolin Wang wrote: > > > On 5/27/2022 3:12 PM, Gavin Shan wrote: >> Hi Baolin, >> >> On 5/27/22 12:51 PM, Baolin Wang wrote: >>> Fix below building errors which was caused by commit ae07562909f3 >>> ("mm: change huge_ptep_clear_flush() to return the original pte") >>> interacting with commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush >>> from get_clear_flush()"). >>> >>> Due to the new get_clear_contig() has dropped TLB flush, we should >>> add an explicit TLB flush in huge_ptep_clear_flush() to keep original >>> semantics when changing to use new get_clear_contig(). >>> >>> " >>> arch/arm64/mm/hugetlbpage.c: In function ‘huge_ptep_clear_flush’: >>> arch/arm64/mm/hugetlbpage.c:515:9: error: implicit declaration of >>> function ‘get_clear_flush’; did you mean ‘ptep_clear_flush’? >>> [-Werror=implicit-function-declaration] >>> 515 | return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); >>> | ^~~~~~~~~~~~~~~ >>> | ptep_clear_flush >>> " >>> >>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> >>> Suggested-by: Catalin Marinas <catalin.marinas@arm.com> >>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>> Cc: Anshuman Khandual <anshuman.khandual@arm.com> >>> Cc: Andrew Morton <akpm@linux-foundation.org> >>> --- >>> arch/arm64/mm/hugetlbpage.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >> >> I ran to the compiling failure either and it would be caused by >> commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from get_clear_flush()"). >> It's worthy to have a "Fixes" tag. With those fixed: > > Thanks for reminding. IMHO, better to add 2 related commits' fix tag. Linus, could you help to add them when applying this patch, or need a resend? Thanks. > > Fixes: ae07562909f3 ("mm: change huge_ptep_clear_flush() to return the original pte") > Fixes: fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from get_clear_flush()"). Although this merge conflict happened in flight, the earlier commit ae07562909f3 came afterwards by which time get_clear_contig() has been added without required TLB flush. Hence seems like 'Fixes:' tag applies to the earlier commit itself. Nonetheless LGTM. Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > >> >> Reviewed-by: Gavin Shan <gshan@redhat.com> >> >> >>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c >>> index 0f0c17dfeb9c..e2a5ec9fdc0d 100644 >>> --- a/arch/arm64/mm/hugetlbpage.c >>> +++ b/arch/arm64/mm/hugetlbpage.c >>> @@ -507,12 +507,15 @@ pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, >>> { >>> size_t pgsize; >>> int ncontig; >>> + pte_t orig_pte; >>> if (!pte_cont(READ_ONCE(*ptep))) >>> return ptep_clear_flush(vma, addr, ptep); >>> ncontig = find_num_contig(vma->vm_mm, addr, ptep, &pgsize); >>> - return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); >>> + orig_pte = get_clear_contig(vma->vm_mm, addr, ptep, pgsize, ncontig); >>> + flush_tlb_range(vma, addr, addr + pgsize * ncontig); >>> + return orig_pte; >>> } >>> static int __init hugetlbpage_init(void) >>> >> >> Thanks, >> Gavin
On Fri, 27 May 2022 at 10:21, Baolin Wang <baolin.wang@linux.alibaba.com> wrote: > > Fix below building errors which was caused by commit ae07562909f3 > ("mm: change huge_ptep_clear_flush() to return the original pte") > interacting with commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush > from get_clear_flush()"). > > Due to the new get_clear_contig() has dropped TLB flush, we should > add an explicit TLB flush in huge_ptep_clear_flush() to keep original > semantics when changing to use new get_clear_contig(). > > " > arch/arm64/mm/hugetlbpage.c: In function ‘huge_ptep_clear_flush’: > arch/arm64/mm/hugetlbpage.c:515:9: error: implicit declaration of > function ‘get_clear_flush’; did you mean ‘ptep_clear_flush’? > [-Werror=implicit-function-declaration] > 515 | return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); > | ^~~~~~~~~~~~~~~ > | ptep_clear_flush > " > This patch tested on arm, arm64, i386 and x86 with gcc-11 and clang-14. > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > Suggested-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Andrew Morton <akpm@linux-foundation.org> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> > --- > arch/arm64/mm/hugetlbpage.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c > index 0f0c17dfeb9c..e2a5ec9fdc0d 100644 > --- a/arch/arm64/mm/hugetlbpage.c > +++ b/arch/arm64/mm/hugetlbpage.c > @@ -507,12 +507,15 @@ pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, > { > size_t pgsize; > int ncontig; > + pte_t orig_pte; > > if (!pte_cont(READ_ONCE(*ptep))) > return ptep_clear_flush(vma, addr, ptep); > > ncontig = find_num_contig(vma->vm_mm, addr, ptep, &pgsize); > - return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); > + orig_pte = get_clear_contig(vma->vm_mm, addr, ptep, pgsize, ncontig); > + flush_tlb_range(vma, addr, addr + pgsize * ncontig); > + return orig_pte; > } > > static int __init hugetlbpage_init(void) > -- > 2.27.0 > -- Linaro LKFT https://lkft.linaro.org
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c index 0f0c17dfeb9c..e2a5ec9fdc0d 100644 --- a/arch/arm64/mm/hugetlbpage.c +++ b/arch/arm64/mm/hugetlbpage.c @@ -507,12 +507,15 @@ pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, { size_t pgsize; int ncontig; + pte_t orig_pte; if (!pte_cont(READ_ONCE(*ptep))) return ptep_clear_flush(vma, addr, ptep); ncontig = find_num_contig(vma->vm_mm, addr, ptep, &pgsize); - return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); + orig_pte = get_clear_contig(vma->vm_mm, addr, ptep, pgsize, ncontig); + flush_tlb_range(vma, addr, addr + pgsize * ncontig); + return orig_pte; } static int __init hugetlbpage_init(void)
Fix below building errors which was caused by commit ae07562909f3 ("mm: change huge_ptep_clear_flush() to return the original pte") interacting with commit fb396bb459c1 ("arm64/hugetlb: Drop TLB flush from get_clear_flush()"). Due to the new get_clear_contig() has dropped TLB flush, we should add an explicit TLB flush in huge_ptep_clear_flush() to keep original semantics when changing to use new get_clear_contig(). " arch/arm64/mm/hugetlbpage.c: In function ‘huge_ptep_clear_flush’: arch/arm64/mm/hugetlbpage.c:515:9: error: implicit declaration of function ‘get_clear_flush’; did you mean ‘ptep_clear_flush’? [-Werror=implicit-function-declaration] 515 | return get_clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig); | ^~~~~~~~~~~~~~~ | ptep_clear_flush " Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> Suggested-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Anshuman Khandual <anshuman.khandual@arm.com> Cc: Andrew Morton <akpm@linux-foundation.org> --- arch/arm64/mm/hugetlbpage.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)