All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: "alex@ghiti.fr" <alex@ghiti.fr>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Paul Mackerras <paulus@samba.org>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 07/10] powerpc/mm: Use generic_hugetlb_get_unmapped_area()
Date: Thu, 09 Dec 2021 20:02:03 +1000	[thread overview]
Message-ID: <1639043741.e2zqhea1ix.astroid@bobo.none> (raw)
In-Reply-To: <f292a83707b64c73fcb02a8708f18f09422f7eea.1638976229.git.christophe.leroy@csgroup.eu>

Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
> Use the generic version of arch_hugetlb_get_unmapped_area()
> which is now available at all time.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/book3s/64/hugetlb.h |  4 --
>  arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
>  arch/powerpc/mm/hugetlbpage.c                |  4 +-
>  3 files changed, 1 insertion(+), 62 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index 12e150e615b7..b37a28f62cf6 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -8,10 +8,6 @@
>   */
>  void radix__flush_hugetlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
>  void radix__local_flush_hugetlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
> -extern unsigned long
> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> -				unsigned long len, unsigned long pgoff,
> -				unsigned long flags);
>  
>  extern void radix__huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
>  						unsigned long addr, pte_t *ptep,
> diff --git a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> index 23d3e08911d3..d2fb776febb4 100644
> --- a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> +++ b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> @@ -41,61 +41,6 @@ void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma, unsigned long st
>  		radix__flush_tlb_range_psize(vma->vm_mm, start, end, psize);
>  }
>  
> -/*
> - * A vairant of hugetlb_get_unmapped_area doing topdown search
> - * FIXME!! should we do as x86 does or non hugetlb area does ?
> - * ie, use topdown or not based on mmap_is_legacy check ?
> - */
> -unsigned long
> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> -				unsigned long len, unsigned long pgoff,
> -				unsigned long flags)
> -{
> -	struct mm_struct *mm = current->mm;
> -	struct vm_area_struct *vma;
> -	struct hstate *h = hstate_file(file);
> -	int fixed = (flags & MAP_FIXED);
> -	unsigned long high_limit;
> -	struct vm_unmapped_area_info info;
> -
> -	high_limit = DEFAULT_MAP_WINDOW;
> -	if (addr >= high_limit || (fixed && (addr + len > high_limit)))
> -		high_limit = TASK_SIZE;

I wonder if generic hugetlb_get_unmapped_area needs to have the
arch_get_mmap_end() added.

arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so
it looks like it has broken large address hint logic for hugetlbfs
mappings? x86-64 defines their own and does the same hinting for
normal and hugetlbfs mmap.

If we had that and defied arch_get_mmap_end(), then this patch should
work.

Thanks,
Nick

WARNING: multiple messages have this Message-ID (diff)
From: Nicholas Piggin <npiggin@gmail.com>
To: "alex@ghiti.fr" <alex@ghiti.fr>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Paul Mackerras <paulus@samba.org>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v4 07/10] powerpc/mm: Use generic_hugetlb_get_unmapped_area()
Date: Thu, 09 Dec 2021 20:02:03 +1000	[thread overview]
Message-ID: <1639043741.e2zqhea1ix.astroid@bobo.none> (raw)
In-Reply-To: <f292a83707b64c73fcb02a8708f18f09422f7eea.1638976229.git.christophe.leroy@csgroup.eu>

Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
> Use the generic version of arch_hugetlb_get_unmapped_area()
> which is now available at all time.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/book3s/64/hugetlb.h |  4 --
>  arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
>  arch/powerpc/mm/hugetlbpage.c                |  4 +-
>  3 files changed, 1 insertion(+), 62 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index 12e150e615b7..b37a28f62cf6 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -8,10 +8,6 @@
>   */
>  void radix__flush_hugetlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
>  void radix__local_flush_hugetlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
> -extern unsigned long
> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> -				unsigned long len, unsigned long pgoff,
> -				unsigned long flags);
>  
>  extern void radix__huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
>  						unsigned long addr, pte_t *ptep,
> diff --git a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> index 23d3e08911d3..d2fb776febb4 100644
> --- a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> +++ b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c
> @@ -41,61 +41,6 @@ void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma, unsigned long st
>  		radix__flush_tlb_range_psize(vma->vm_mm, start, end, psize);
>  }
>  
> -/*
> - * A vairant of hugetlb_get_unmapped_area doing topdown search
> - * FIXME!! should we do as x86 does or non hugetlb area does ?
> - * ie, use topdown or not based on mmap_is_legacy check ?
> - */
> -unsigned long
> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> -				unsigned long len, unsigned long pgoff,
> -				unsigned long flags)
> -{
> -	struct mm_struct *mm = current->mm;
> -	struct vm_area_struct *vma;
> -	struct hstate *h = hstate_file(file);
> -	int fixed = (flags & MAP_FIXED);
> -	unsigned long high_limit;
> -	struct vm_unmapped_area_info info;
> -
> -	high_limit = DEFAULT_MAP_WINDOW;
> -	if (addr >= high_limit || (fixed && (addr + len > high_limit)))
> -		high_limit = TASK_SIZE;

I wonder if generic hugetlb_get_unmapped_area needs to have the
arch_get_mmap_end() added.

arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so
it looks like it has broken large address hint logic for hugetlbfs
mappings? x86-64 defines their own and does the same hinting for
normal and hugetlbfs mmap.

If we had that and defied arch_get_mmap_end(), then this patch should
work.

Thanks,
Nick


  reply	other threads:[~2021-12-09 10:02 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-08 17:18 [PATCH v4 00/10] Convert powerpc to default topdown mmap layout Christophe Leroy
2021-12-08 17:18 ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 01/10] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 02/10] mm, hugetlbfs: Allow an arch to always use generic versions of get_unmapped_area functions Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-09  9:40   ` Nicholas Piggin
2021-12-09  9:40     ` Nicholas Piggin
2021-12-08 17:18 ` [PATCH v4 03/10] powerpc/mm: Move vma_mmu_pagesize() Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-09  9:36   ` Nicholas Piggin
2021-12-09  9:36     ` Nicholas Piggin
2021-12-08 17:18 ` [PATCH v4 05/10] powerpc/mm: Remove CONFIG_PPC_MM_SLICES Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 04/10] powerpc/mm: Make slice specific to book3s/64 Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 06/10] powerpc/mm: Use generic_get_unmapped_area() and call it from arch_get_unmapped_area() Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-09  9:50   ` Nicholas Piggin
2021-12-09  9:50     ` Nicholas Piggin
2021-12-10 17:47     ` Christophe Leroy
2021-12-10 17:47       ` Christophe Leroy
2021-12-16 14:25     ` Christophe Leroy
2021-12-16 14:25       ` Christophe Leroy
2021-12-13 13:02   ` Michael Ellerman
2021-12-13 13:02     ` Michael Ellerman
2021-12-08 17:18 ` [PATCH v4 07/10] powerpc/mm: Use generic_hugetlb_get_unmapped_area() Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-09 10:02   ` Nicholas Piggin [this message]
2021-12-09 10:02     ` Nicholas Piggin
2021-12-16 17:13     ` Christophe Leroy
2021-12-16 17:13       ` Christophe Leroy
2021-12-16 17:13       ` Christophe Leroy
2021-12-16 18:55       ` Catalin Marinas
2021-12-16 18:55         ` Catalin Marinas
2021-12-16 18:55         ` Catalin Marinas
2021-12-08 17:18 ` [PATCH v4 08/10] powerpc/mm: Move get_unmapped_area functions to slice.c Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 09/10] powerpc/mm: Convert to default topdown mmap layout Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy
2021-12-09 10:15   ` Nicholas Piggin
2021-12-09 10:15     ` Nicholas Piggin
2021-12-09 10:22     ` Christophe Leroy
2021-12-09 10:22       ` Christophe Leroy
2021-12-09 10:43       ` Nicholas Piggin
2021-12-09 10:43         ` Nicholas Piggin
2021-12-09 11:22         ` Michael Ellerman
2021-12-09 11:22           ` Michael Ellerman
2021-12-09 11:32           ` Christophe Leroy
2021-12-09 11:32             ` Christophe Leroy
2021-12-09 23:56             ` Michael Ellerman
2021-12-09 23:56               ` Michael Ellerman
2021-12-16 14:27     ` Christophe Leroy
2021-12-16 14:27       ` Christophe Leroy
2021-12-08 17:18 ` [PATCH v4 10/10] powerpc/mm: Properly randomise mmap with slices Christophe Leroy
2021-12-08 17:18   ` Christophe Leroy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1639043741.e2zqhea1ix.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex@ghiti.fr \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.