Re: [PATCH RFC v3 23/35] arm64: mte: Try to reserve tag storage in arch_alloc_page()

Alexandru Elisei
2024-01-30T11:38:38Z

Hi Peter,

On Mon, Jan 29, 2024 at 04:04:18PM -0800, Peter Collingbourne wrote:
> On Thu, Jan 25, 2024 at 8:45 AM Alexandru Elisei
> <alexandru.elisei@arm.com> wrote:
> >
> > Reserve tag storage for a page that is being allocated as tagged. This
> > is a best effort approach, and failing to reserve tag storage is
> > allowed.
> >
> > When all the associated tagged pages have been freed, return the tag
> > storage pages back to the page allocator, where they can be used again for
> > data allocations.
> >
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >
> > Changes since rfc v2:
> >
> > * Based on rfc v2 patch #16 ("arm64: mte: Manage tag storage on page
> > allocation").
> > * Fixed calculation of the number of associated tag storage blocks (Hyesoo
> > Yu).
> > * Tag storage is reserved in arch_alloc_page() instead of
> > arch_prep_new_page().
> >
> > arch/arm64/include/asm/mte.h | 16 +-
> > arch/arm64/include/asm/mte_tag_storage.h | 31 +++
> > arch/arm64/include/asm/page.h | 5 +
> > arch/arm64/include/asm/pgtable.h | 19 ++
> > arch/arm64/kernel/mte_tag_storage.c | 234 +++++++++++++++++++++++
> > arch/arm64/mm/fault.c | 7 +
> > fs/proc/page.c | 1 +
> > include/linux/kernel-page-flags.h | 1 +
> > include/linux/page-flags.h | 1 +
> > include/trace/events/mmflags.h | 3 +-
> > mm/huge_memory.c | 1 +
> > 11 files changed, 316 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> > index 8034695b3dd7..6457b7899207 100644
> > --- a/arch/arm64/include/asm/mte.h
> > +++ b/arch/arm64/include/asm/mte.h
> > @@ -40,12 +40,24 @@ void mte_free_tag_buf(void *buf);
> > #ifdef CONFIG_ARM64_MTE
> >
> > /* track which pages have valid allocation tags */
> > -#define PG_mte_tagged PG_arch_2
> > +#define PG_mte_tagged PG_arch_2
> > /* simple lock to avoid multiple threads tagging the same page */
> > -#define PG_mte_lock PG_arch_3
> > +#define PG_mte_lock PG_arch_3
> > +/* Track if a tagged page has tag storage reserved */
> > +#define PG_tag_storage_reserved PG_arch_4
> > +
> > +#ifdef CONFIG_ARM64_MTE_TAG_STORAGE
> > +DECLARE_STATIC_KEY_FALSE(tag_storage_enabled_key);
> > +extern bool page_tag_storage_reserved(struct page *page);
> > +#endif
> >
> > static inline void set_page_mte_tagged(struct page *page)
> > {
> > +#ifdef CONFIG_ARM64_MTE_TAG_STORAGE
> > + /* Open code mte_tag_storage_enabled() */
> > + WARN_ON_ONCE(static_branch_likely(&tag_storage_enabled_key) &&
> > + !page_tag_storage_reserved(page));
> > +#endif
> > /*
> > * Ensure that the tags written prior to this function are visible
> > * before the page flags update.
> > diff --git a/arch/arm64/include/asm/mte_tag_storage.h b/arch/arm64/include/asm/mte_tag_storage.h
> > index 7b3f6bff8e6f..09f1318d924e 100644
> > --- a/arch/arm64/include/asm/mte_tag_storage.h
> > +++ b/arch/arm64/include/asm/mte_tag_storage.h
> > @@ -5,6 +5,12 @@
> > #ifndef __ASM_MTE_TAG_STORAGE_H
> > #define __ASM_MTE_TAG_STORAGE_H
> >
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <linux/mm_types.h>
> > +
> > +#include <asm/mte.h>
> > +
> > #ifdef CONFIG_ARM64_MTE_TAG_STORAGE
> >
> > DECLARE_STATIC_KEY_FALSE(tag_storage_enabled_key);
> > @@ -15,6 +21,15 @@ static inline bool tag_storage_enabled(void)
> > }
> >
> > void mte_init_tag_storage(void);
> > +
> > +static inline bool alloc_requires_tag_storage(gfp_t gfp)
> > +{
> > + return gfp & __GFP_TAGGED;
> > +}
> > +int reserve_tag_storage(struct page *page, int order, gfp_t gfp);
> > +void free_tag_storage(struct page *page, int order);
> > +
> > +bool page_tag_storage_reserved(struct page *page);
> > #else
> > static inline bool tag_storage_enabled(void)
> > {
> > @@ -23,6 +38,22 @@ static inline bool tag_storage_enabled(void)
> > static inline void mte_init_tag_storage(void)
> > {
> > }
> > +static inline bool alloc_requires_tag_storage(struct page *page)
>
> This function should take a gfp_t to match the
> CONFIG_ARM64_MTE_TAG_STORAGE case.

Ah, yes, it should, nice catch, the compiler didn't throw an error. Will
fix, thanks!

Alex