Re: [PATCH v1 6/8] mm: hugetlb: Convert set_huge_pte_at() to take vma
Hi Ryan,
On Thu, 21 Sep 2023 17:20:05 +0100 Ryan Roberts wrote:
> In order to fix a bug, arm64 needs access to the vma inside it's
> implementation of set_huge_pte_at(). Provide for this by converting the
> mm parameter to be a vma. Any implementations that require the mm can
> access it via vma->vm_mm.
>
> This commit makes the required modifications to the core mm. Separate
> commits update the arches, before the actual bug is fixed in arm64.
>
> No behavioral changes intended.
>
> Signed-off-by: Ryan Roberts
For mm/damon/ part change,
Reviewed-by: SeongJae Park
Thanks,
SJ
> ---
> include/asm-generic/hugetlb.h | 6 +++---
> include/linux/hugetlb.h | 6 +++---
> mm/damon/vaddr.c | 2 +-
> mm/hugetlb.c | 30 +++---
> mm/migrate.c | 2 +-
> mm/rmap.c | 10 +-
> mm/vmalloc.c | 5 -
> 7 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
> index 4da02798a00b..515e4777fb65 100644
> --- a/include/asm-generic/hugetlb.h
> +++ b/include/asm-generic/hugetlb.h
> @@ -75,10 +75,10 @@ static inline void hugetlb_free_pgd_range(struct
> mmu_gather *tlb,
> #endif
>
> #ifndef __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT
> -static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
> - pte_t *ptep, pte_t pte)
> +static inline void set_huge_pte_at(struct vm_area_struct *vma,
> + unsigned long addr, pte_t *ptep, pte_t pte)
> {
> - set_pte_at(mm, addr, ptep, pte);
> + set_pte_at(vma->vm_mm, addr, ptep, pte);
> }
> #endif
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 5b2626063f4f..08184f32430c 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -984,7 +984,7 @@ static inline void huge_ptep_modify_prot_commit(struct
> vm_area_struct *vma,
> unsigned long addr, pte_t *ptep,
> pte_t old_pte, pte_t pte)
> {
> - set_huge_pte_at(vma->vm_mm, addr, ptep, pte);
> + set_huge_pte_at(vma, addr, ptep, pte);
> }
> #endif
>
> @@ -1172,8 +1172,8 @@ static inline pte_t huge_ptep_clear_flush(struct
> vm_area_struct *vma,
> #endif
> }
>
> -static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
> -pte_t *ptep, pte_t pte)
> +static inline void set_huge_pte_at(struct vm_area_struct *vma,
> +unsigned long addr, pte_t *ptep, pte_t pte)
> {
> }
>
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 4c81a9dbd044..55da8cee8fbc 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -347,7 +347,7 @@ static void damon_hugetlb_mkold(pte_t *pte, struct
> mm_struct *mm,
> if (pte_young(entry)) {
> referenced = true;
> entry = pte_mkold(entry);
> - set_huge_pte_at(mm, addr, pte, entry);
> + set_huge_pte_at(vma, addr, pte, entry);
> }
>
> #ifdef CONFIG_MMU_NOTIFIER
[...]
Thanks,
SJ
Re: [PATCH v2 3/5] mmu_notifiers: Call invalidate_range() when invalidating TLBs
On Thu, 20 Jul 2023 10:52:59 +1000 Alistair Popple wrote:
>
> SeongJae Park writes:
>
> > Hi Alistair,
> >
> > On Wed, 19 Jul 2023 22:18:44 +1000 Alistair Popple
> > wrote:
> >
> >> The invalidate_range() is going to become an architecture specific mmu
> >> notifier used to keep the TLB of secondary MMUs such as an IOMMU in
> >> sync with the CPU page tables. Currently it is called from separate
> >> code paths to the main CPU TLB invalidations. This can lead to a
> >> secondary TLB not getting invalidated when required and makes it hard
> >> to reason about when exactly the secondary TLB is invalidated.
> >>
> >> To fix this move the notifier call to the architecture specific TLB
> >> maintenance functions for architectures that have secondary MMUs
> >> requiring explicit software invalidations.
> >>
> >> This fixes a SMMU bug on ARM64. On ARM64 PTE permission upgrades
> >> require a TLB invalidation. This invalidation is done by the
> >> architecutre specific ptep_set_access_flags() which calls
> >> flush_tlb_page() if required. However this doesn't call the notifier
> >> resulting in infinite faults being generated by devices using the SMMU
> >> if it has previously cached a read-only PTE in it's TLB.
> >>
> >> Moving the invalidations into the TLB invalidation functions ensures
> >> all invalidations happen at the same time as the CPU invalidation. The
> >> architecture specific flush_tlb_all() routines do not call the
> >> notifier as none of the IOMMUs require this.
> >>
> >> Signed-off-by: Alistair Popple
> >> Suggested-by: Jason Gunthorpe
> >
> > I found below kernel NULL-dereference issue on latest mm-unstable tree, and
> > bisect points me to the commit of this patch, namely
> > 75c400f82d347af1307010a3e06f3aa5d831d995.
> >
> > To reproduce, I use 'stress-ng --bigheap $(nproc)'. The issue happens as
> > soon
> > as it starts reclaiming memory. I didn't dive deep into this yet, but
> > reporting this issue first, since you might have an idea already.
>
> Thanks for the report SJ!
>
> I see the problem - current->mm can (obviously!) be NULL which is what's
> leading to the NULL dereference. Instead I think on x86 I need to call
> the notifier when adding the invalidate to the tlbbatch in
> arch_tlbbatch_add_pending() which is equivalent to what ARM64 does.
>
> The below should fix it. Will do a respin with this.
Thank you for this quick reply! I confirm this fixes my issue.
Tested-by: SeongJae Park
>
> ---
>
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 837e4a50281a..79c46da919b9 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -4,6 +4,7 @@
>
> #include
> #include
> +#include
Nit. How about putting it between mm_types.h and sched.h, so that it looks
alphabetically sorted?
>
> #include
> #include
> @@ -282,6 +283,7 @@ static inline void arch_tlbbatch_add_pending(struct
> arch_tlbflush_unmap_batch *b
> {
> inc_mm_tlb_gen(mm);
> cpumask_or(&batch->cpumask, &batch->cpumask, mm_cpumask(mm));
> + mmu_notifier_arch_invalidate_secondary_tlbs(mm, 0, -1UL);
> }
>
> static inline void arch_flush_tlb_batched_pending(struct mm_struct *mm)
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 0b990fb56b66..2d253919b3e8 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -1265,7 +1265,6 @@ void arch_tlbbatch_flush(struct
> arch_tlbflush_unmap_batch *batch)
>
> put_flush_tlb_info();
> put_cpu();
> - mmu_notifier_arch_invalidate_secondary_tlbs(current->mm, 0, -1UL);
> }
>
> /*
>
>
Thanks,
SJ
Re: [PATCH v1 1/5] treewide: use get_random_u32_below() instead of deprecated function
Hi Jason, Cc-ing da...@lists.linux.dev and linux...@kvack.org. On Fri, 21 Oct 2022 21:43:59 -0400 "Jason A. Donenfeld" wrote: > This is a simple mechanical transformation done by: > > @@ > expression E; > @@ > - prandom_u32_max(E) > + get_random_u32_below(E) > > Signed-off-by: Jason A. Donenfeld > --- [...] > include/linux/damon.h | 2 +- For the damon.h part, Reviewed-by: SeongJae Park Thanks, SJ
Re: [PATCH v3 03/12] mm: Make HPAGE_PXD_* macros even if !THP
Hi Peter, On Thu, 21 Mar 2024 18:07:53 -0400 pet...@redhat.com wrote: > From: Peter Xu > > These macros can be helpful when we plan to merge hugetlb code into generic > code. Move them out and define them even if !THP. > > We actually already defined HPAGE_PMD_NR for other reasons even if !THP. > Reorganize these macros. > > Reviewed-by: Christoph Hellwig > Reviewed-by: Jason Gunthorpe > Signed-off-by: Peter Xu > --- > include/linux/huge_mm.h | 17 ++--- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index de0c89105076..3bcdfc7e5d57 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -64,9 +64,6 @@ ssize_t single_hugepage_flag_show(struct kobject *kobj, > enum transparent_hugepage_flag flag); > extern struct kobj_attribute shmem_enabled_attr; > > -#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT) > -#define HPAGE_PMD_NR (1< - > /* > * Mask of all large folio orders supported for anonymous THP; all orders up > to > * and including PMD_ORDER, except order-0 (which is not "huge") and order-1 > @@ -87,14 +84,19 @@ extern struct kobj_attribute shmem_enabled_attr; > #define thp_vma_allowable_order(vma, vm_flags, smaps, in_pf, enforce_sysfs, > order) \ > (!!thp_vma_allowable_orders(vma, vm_flags, smaps, in_pf, enforce_sysfs, > BIT(order))) > > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE > #define HPAGE_PMD_SHIFT PMD_SHIFT > #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT) > #define HPAGE_PMD_MASK (~(HPAGE_PMD_SIZE - 1)) > +#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT) > +#define HPAGE_PMD_NR (1< > #define HPAGE_PUD_SHIFT PUD_SHIFT > #define HPAGE_PUD_SIZE ((1UL) << HPAGE_PUD_SHIFT) > #define HPAGE_PUD_MASK (~(HPAGE_PUD_SIZE - 1)) > +#define HPAGE_PUD_ORDER (HPAGE_PUD_SHIFT-PAGE_SHIFT) > +#define HPAGE_PUD_NR (1
Re: [PATCH v3 03/12] mm: Make HPAGE_PXD_* macros even if !THP
Hi Peter, On Fri, 22 Mar 2024 20:30:24 -0400 Peter Xu wrote: > On Fri, Mar 22, 2024 at 10:14:56AM -0700, SeongJae Park wrote: > > Hi Peter, > > Hi, SeongJae, > > > > > On Thu, 21 Mar 2024 18:07:53 -0400 pet...@redhat.com wrote: > > > > > From: Peter Xu > > > > > > These macros can be helpful when we plan to merge hugetlb code into > > > generic > > > code. Move them out and define them even if !THP. > > > > > > We actually already defined HPAGE_PMD_NR for other reasons even if !THP. > > > Reorganize these macros. > > > > > > Reviewed-by: Christoph Hellwig > > > Reviewed-by: Jason Gunthorpe > > > Signed-off-by: Peter Xu > > > --- > > > include/linux/huge_mm.h | 17 ++--- > > > 1 file changed, 6 insertions(+), 11 deletions(-) > > > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > > index de0c89105076..3bcdfc7e5d57 100644 > > > --- a/include/linux/huge_mm.h > > > +++ b/include/linux/huge_mm.h > > > @@ -64,9 +64,6 @@ ssize_t single_hugepage_flag_show(struct kobject *kobj, > > > enum transparent_hugepage_flag flag); > > > extern struct kobj_attribute shmem_enabled_attr; > > > > > > -#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT) > > > -#define HPAGE_PMD_NR (1< > > - > > > /* > > > * Mask of all large folio orders supported for anonymous THP; all > > > orders up to > > > * and including PMD_ORDER, except order-0 (which is not "huge") and > > > order-1 > > > @@ -87,14 +84,19 @@ extern struct kobj_attribute shmem_enabled_attr; > > > #define thp_vma_allowable_order(vma, vm_flags, smaps, in_pf, > > > enforce_sysfs, order) \ > > > (!!thp_vma_allowable_orders(vma, vm_flags, smaps, in_pf, enforce_sysfs, > > > BIT(order))) > > > > > > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE > > > #define HPAGE_PMD_SHIFT PMD_SHIFT > > > #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT) > > > #define HPAGE_PMD_MASK (~(HPAGE_PMD_SIZE - 1)) > > > +#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT) > > > +#define HPAGE_PMD_NR (1< > > > > > #define HPAGE_PUD_SHIFT PUD_SHIFT > > > #define HPAGE_PUD_SIZE ((1UL) << HPAGE_PUD_SHIFT) > > > #define HPAGE_PUD_MASK (~(HPAGE_PUD_SIZE - 1)) > > > +#define HPAGE_PUD_ORDER (HPAGE_PUD_SHIFT-PAGE_SHIFT) > > > +#define HPAGE_PUD_NR (1< > > > I just found latest mm-unstable fails one of my build configurations[1] with > > below error. 'git bisect' says this is the first patch set started the > > failure. I haven't looked in deep, but just reporting first. > > > > In file included from .../include/linux/mm.h:1115, > > from .../mm/vmstat.c:14: > > .../mm/vmstat.c: In function 'zoneinfo_show_print': > > .../include/linux/huge_mm.h:87:25: error: 'PMD_SHIFT' undeclared (first > > use in this function); did you mean 'PUD_SHIFT'? > >87 | #define HPAGE_PMD_SHIFT PMD_SHIFT > > | ^ > > > > [1] > > https://github.com/awslabs/damon-tests/blob/next/corr/tests/build_m68k.sh > > Apologies for the issue. No problem at all, this blocks nothing in real :) > This is caused by !CONFIG_MMU, I think. > > I thought this would be fairly easy to fix by putting these macros under > CONFIG_PGTABLE_HAS_HUGE_LEAVES, however when doing this I could have found > some other issue that violates this rule.. mm/vmstat.c has referenced > HPAGE_PMD_NR even if vmstat_item_print_in_thp() wanted to guard it only > with CONFIG_THP. > > /home/peterx/git/linux/mm/vmstat.c: In function 'zoneinfo_show_print': > /home/peterx/git/linux/mm/vmstat.c:1689:42: error: 'HPAGE_PMD_NR' undeclared > (first use in this function) > 1689 | pages /= HPAGE_PMD_NR; > | ^~~~ > /home/peterx/git/linux/mm/vmstat.c:1689:42: note: each undeclared identifier > is reported only once for each function it appears in > CC drivers/tty/tty_port.o > /home/peterx/git/linux/mm/vmstat.c: In function 'vmstat_start': > /home/peterx/git/linux/mm/vmstat.c:1822:33: error: 'HPAGE_PMD_NR' undeclared > (first use in this function) > 1822 | v[i] /= HPAGE_PMD_NR; > | ^~~~ > ma
