diff mbox series

[RFC,v3,03/35] mm: page_alloc: Add an arch hook to filter MIGRATE_CMA allocations

Message ID 20240125164256.4147-4-alexandru.elisei@arm.com (mailing list archive)
State New
Headers show
Series Add support for arm64 MTE dynamic tag storage reuse | expand

Commit Message

Alexandru Elisei Jan. 25, 2024, 4:42 p.m. UTC
As an architecture might have specific requirements around the allocation
of CMA pages, add an arch hook that can disable allocations from
MIGRATE_CMA, if the allocation was otherwise allowed.

This will be used by arm64, which will put tag storage pages on the
MIGRATE_CMA list, and tag storage pages cannot be tagged. The filter will
be used to deny using MIGRATE_CMA for __GFP_TAGGED allocations.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 include/linux/pgtable.h | 7 +++++++
 mm/page_alloc.c         | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Anshuman Khandual Jan. 29, 2024, 8:44 a.m. UTC | #1
On 1/25/24 22:12, Alexandru Elisei wrote:
> As an architecture might have specific requirements around the allocation
> of CMA pages, add an arch hook that can disable allocations from
> MIGRATE_CMA, if the allocation was otherwise allowed.
> 
> This will be used by arm64, which will put tag storage pages on the
> MIGRATE_CMA list, and tag storage pages cannot be tagged. The filter will
> be used to deny using MIGRATE_CMA for __GFP_TAGGED allocations.

Just wondering how allocation requests would be blocked for direct
alloc_contig_range() requests ?

> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  include/linux/pgtable.h | 7 +++++++
>  mm/page_alloc.c         | 3 ++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 6d98d5fdd697..c5ddec6b5305 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -905,6 +905,13 @@ static inline void arch_do_swap_page(struct mm_struct *mm,
>  static inline void arch_free_pages_prepare(struct page *page, int order) { }
>  #endif
>  
> +#ifndef __HAVE_ARCH_ALLOC_CMA

Same as last patch i.e __HAVE_ARCH_ALLOC_CMA could be avoided via
a direct check on #ifndef arch_alloc_cma instead.

> +static inline bool arch_alloc_cma(gfp_t gfp)
> +{
> +	return true;
> +}
> +#endif
> +
>  #ifndef __HAVE_ARCH_UNMAP_ONE
>  /*
>   * Some architectures support metadata associated with a page. When a
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 27282a1c82fe..a96d47a6393e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3157,7 +3157,8 @@ static inline unsigned int gfp_to_alloc_flags_cma(gfp_t gfp_mask,
>  						  unsigned int alloc_flags)
>  {
>  #ifdef CONFIG_CMA
> -	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> +	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE &&
> +	    arch_alloc_cma(gfp_mask))
>  		alloc_flags |= ALLOC_CMA;
>  #endif
>  	return alloc_flags;
Alexandru Elisei Jan. 29, 2024, 11:45 a.m. UTC | #2
Hi,

On Mon, Jan 29, 2024 at 02:14:16PM +0530, Anshuman Khandual wrote:
> 
> 
> On 1/25/24 22:12, Alexandru Elisei wrote:
> > As an architecture might have specific requirements around the allocation
> > of CMA pages, add an arch hook that can disable allocations from
> > MIGRATE_CMA, if the allocation was otherwise allowed.
> > 
> > This will be used by arm64, which will put tag storage pages on the
> > MIGRATE_CMA list, and tag storage pages cannot be tagged. The filter will
> > be used to deny using MIGRATE_CMA for __GFP_TAGGED allocations.
> 
> Just wondering how allocation requests would be blocked for direct
> alloc_contig_range() requests ?

alloc_contig_range() does page allocation in __alloc_contig_migrate_range()
-> alloc_migration_target(); __alloc_contig_migrate_range() ignores the
gfp_mask parameter passed to alloc_contig_range() when building struct
migration_target_control, even though it's available in the struct
compact_control argument. That looks like a bug to me, as the decription
for the gfp_mask parameter says: "GFP mask to use during compaction".

Regardless, when tag storage page T1 is migrated to it can be used to
storage tags, it doesn't matter if it is replaced by another tag storage
page T2 or a regular page, as long as the replacement isn't also tagged. If
the replacement is also tagged, the code to reserve tag storage would
recurse and deadlock. See patch #16 ("KVM: arm64: Don't deny VM_PFNMAP VMAs
when kvm_has_mte()") [1] for the code.

Does that make sense?

[1] https://lore.kernel.org/linux-mm/20240125164256.4147-24-alexandru.elisei@arm.com/

> 
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  include/linux/pgtable.h | 7 +++++++
> >  mm/page_alloc.c         | 3 ++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> > index 6d98d5fdd697..c5ddec6b5305 100644
> > --- a/include/linux/pgtable.h
> > +++ b/include/linux/pgtable.h
> > @@ -905,6 +905,13 @@ static inline void arch_do_swap_page(struct mm_struct *mm,
> >  static inline void arch_free_pages_prepare(struct page *page, int order) { }
> >  #endif
> >  
> > +#ifndef __HAVE_ARCH_ALLOC_CMA
> 
> Same as last patch i.e __HAVE_ARCH_ALLOC_CMA could be avoided via
> a direct check on #ifndef arch_alloc_cma instead.

include/linux/pgtable.h uses __HAVE_ARCH_*, and I would rather keep it
consistent.

Thanks,
Alex

> 
> > +static inline bool arch_alloc_cma(gfp_t gfp)
> > +{
> > +	return true;
> > +}
> > +#endif
> > +
> >  #ifndef __HAVE_ARCH_UNMAP_ONE
> >  /*
> >   * Some architectures support metadata associated with a page. When a
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 27282a1c82fe..a96d47a6393e 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3157,7 +3157,8 @@ static inline unsigned int gfp_to_alloc_flags_cma(gfp_t gfp_mask,
> >  						  unsigned int alloc_flags)
> >  {
> >  #ifdef CONFIG_CMA
> > -	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> > +	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE &&
> > +	    arch_alloc_cma(gfp_mask))
> >  		alloc_flags |= ALLOC_CMA;
> >  #endif
> >  	return alloc_flags;
diff mbox series

Patch

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 6d98d5fdd697..c5ddec6b5305 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -905,6 +905,13 @@  static inline void arch_do_swap_page(struct mm_struct *mm,
 static inline void arch_free_pages_prepare(struct page *page, int order) { }
 #endif
 
+#ifndef __HAVE_ARCH_ALLOC_CMA
+static inline bool arch_alloc_cma(gfp_t gfp)
+{
+	return true;
+}
+#endif
+
 #ifndef __HAVE_ARCH_UNMAP_ONE
 /*
  * Some architectures support metadata associated with a page. When a
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 27282a1c82fe..a96d47a6393e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3157,7 +3157,8 @@  static inline unsigned int gfp_to_alloc_flags_cma(gfp_t gfp_mask,
 						  unsigned int alloc_flags)
 {
 #ifdef CONFIG_CMA
-	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE)
+	if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE &&
+	    arch_alloc_cma(gfp_mask))
 		alloc_flags |= ALLOC_CMA;
 #endif
 	return alloc_flags;