Re: [PATCH RFC v3 01/35] mm: page_alloc: Add gfp_flags parameter to arch_alloc_page()
Hi,
On Tue, Jan 30, 2024 at 09:56:10AM +0530, Anshuman Khandual wrote:
>
>
> On 1/29/24 17:11, Alexandru Elisei wrote:
> > Hi,
> >
> > On Mon, Jan 29, 2024 at 11:18:59AM +0530, Anshuman Khandual wrote:
> >> On 1/25/24 22:12, Alexandru Elisei wrote:
> >>> Extend the usefulness of arch_alloc_page() by adding the gfp_flags
> >>> parameter.
> >> Although the change here is harmless in itself, it will definitely benefit
> >> from some additional context explaining the rationale, taking into account
> >> why-how arch_alloc_page() got added particularly for s390 platform and how
> >> it's going to be used in the present proposal.
> > arm64 will use it to reserve tag storage if the caller requested a tagged
> > page. Right now that means that __GFP_ZEROTAGS is set in the gfp mask, but
> > I'll rename it to __GFP_TAGGED in patch #18 ("arm64: mte: Rename
> > __GFP_ZEROTAGS to __GFP_TAGGED") [1].
> >
> > [1]
> > https://lore.kernel.org/lkml/20240125164256.4147-19-alexandru.eli...@arm.com/
>
> Makes sense, but please do update the commit message explaining how
> new gfp mask argument will be used to detect tagged page allocation
> requests, further requiring tag storage allocation.
Will do, thanks!
Alex
Re: [PATCH RFC v3 01/35] mm: page_alloc: Add gfp_flags parameter to arch_alloc_page()
On 1/29/24 17:11, Alexandru Elisei wrote:
> Hi,
>
> On Mon, Jan 29, 2024 at 11:18:59AM +0530, Anshuman Khandual wrote:
>> On 1/25/24 22:12, Alexandru Elisei wrote:
>>> Extend the usefulness of arch_alloc_page() by adding the gfp_flags
>>> parameter.
>> Although the change here is harmless in itself, it will definitely benefit
>> from some additional context explaining the rationale, taking into account
>> why-how arch_alloc_page() got added particularly for s390 platform and how
>> it's going to be used in the present proposal.
> arm64 will use it to reserve tag storage if the caller requested a tagged
> page. Right now that means that __GFP_ZEROTAGS is set in the gfp mask, but
> I'll rename it to __GFP_TAGGED in patch #18 ("arm64: mte: Rename
> __GFP_ZEROTAGS to __GFP_TAGGED") [1].
>
> [1]
> https://lore.kernel.org/lkml/20240125164256.4147-19-alexandru.eli...@arm.com/
Makes sense, but please do update the commit message explaining how
new gfp mask argument will be used to detect tagged page allocation
requests, further requiring tag storage allocation.
Re: [PATCH RFC v3 01/35] mm: page_alloc: Add gfp_flags parameter to arch_alloc_page()
Hi,
On Mon, Jan 29, 2024 at 11:18:59AM +0530, Anshuman Khandual wrote:
>
> On 1/25/24 22:12, Alexandru Elisei wrote:
> > Extend the usefulness of arch_alloc_page() by adding the gfp_flags
> > parameter.
>
> Although the change here is harmless in itself, it will definitely benefit
> from some additional context explaining the rationale, taking into account
> why-how arch_alloc_page() got added particularly for s390 platform and how
> it's going to be used in the present proposal.
arm64 will use it to reserve tag storage if the caller requested a tagged
page. Right now that means that __GFP_ZEROTAGS is set in the gfp mask, but
I'll rename it to __GFP_TAGGED in patch #18 ("arm64: mte: Rename
__GFP_ZEROTAGS to __GFP_TAGGED") [1].
[1]
https://lore.kernel.org/lkml/20240125164256.4147-19-alexandru.eli...@arm.com/
Thanks,
Alex
>
> >
> > Signed-off-by: Alexandru Elisei
> > ---
> >
> > Changes since rfc v2:
> >
> > * New patch.
> >
> > arch/s390/include/asm/page.h | 2 +-
> > arch/s390/mm/page-states.c | 2 +-
> > include/linux/gfp.h | 2 +-
> > mm/page_alloc.c | 2 +-
> > 4 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
> > index 73b9c3bf377f..859f0958c574 100644
> > --- a/arch/s390/include/asm/page.h
> > +++ b/arch/s390/include/asm/page.h
> > @@ -163,7 +163,7 @@ static inline int page_reset_referenced(unsigned long
> > addr)
> >
> > struct page;
> > void arch_free_page(struct page *page, int order);
> > -void arch_alloc_page(struct page *page, int order);
> > +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags);
> >
> > static inline int devmem_is_allowed(unsigned long pfn)
> > {
> > diff --git a/arch/s390/mm/page-states.c b/arch/s390/mm/page-states.c
> > index 01f9b39e65f5..b986c8b158e3 100644
> > --- a/arch/s390/mm/page-states.c
> > +++ b/arch/s390/mm/page-states.c
> > @@ -21,7 +21,7 @@ void arch_free_page(struct page *page, int order)
> > __set_page_unused(page_to_virt(page), 1UL << order);
> > }
> >
> > -void arch_alloc_page(struct page *page, int order)
> > +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags)
> > {
> > if (!cmma_flag)
> > return;
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index de292a007138..9e8aa3d144db 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -172,7 +172,7 @@ static inline struct zonelist *node_zonelist(int nid,
> > gfp_t flags)
> > static inline void arch_free_page(struct page *page, int order) { }
> > #endif
> > #ifndef HAVE_ARCH_ALLOC_PAGE
> > -static inline void arch_alloc_page(struct page *page, int order) { }
> > +static inline void arch_alloc_page(struct page *page, int order, gfp_t
> > gfp_flags) { }
> > #endif
> >
> > struct page *__alloc_pages(gfp_t gfp, unsigned int order, int
> > preferred_nid,
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 150d4f23b010..2c140abe5ee6 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1485,7 +1485,7 @@ inline void post_alloc_hook(struct page *page,
> > unsigned int order,
> > set_page_private(page, 0);
> > set_page_refcounted(page);
> >
> > - arch_alloc_page(page, order);
> > + arch_alloc_page(page, order, gfp_flags);
> > debug_pagealloc_map_pages(page, 1 << order);
> >
> > /*
>
> Otherwise LGTM.
Re: [PATCH RFC v3 01/35] mm: page_alloc: Add gfp_flags parameter to arch_alloc_page()
On 1/25/24 22:12, Alexandru Elisei wrote:
> Extend the usefulness of arch_alloc_page() by adding the gfp_flags
> parameter.
Although the change here is harmless in itself, it will definitely benefit
from some additional context explaining the rationale, taking into account
why-how arch_alloc_page() got added particularly for s390 platform and how
it's going to be used in the present proposal.
>
> Signed-off-by: Alexandru Elisei
> ---
>
> Changes since rfc v2:
>
> * New patch.
>
> arch/s390/include/asm/page.h | 2 +-
> arch/s390/mm/page-states.c | 2 +-
> include/linux/gfp.h | 2 +-
> mm/page_alloc.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
> index 73b9c3bf377f..859f0958c574 100644
> --- a/arch/s390/include/asm/page.h
> +++ b/arch/s390/include/asm/page.h
> @@ -163,7 +163,7 @@ static inline int page_reset_referenced(unsigned long
> addr)
>
> struct page;
> void arch_free_page(struct page *page, int order);
> -void arch_alloc_page(struct page *page, int order);
> +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags);
>
> static inline int devmem_is_allowed(unsigned long pfn)
> {
> diff --git a/arch/s390/mm/page-states.c b/arch/s390/mm/page-states.c
> index 01f9b39e65f5..b986c8b158e3 100644
> --- a/arch/s390/mm/page-states.c
> +++ b/arch/s390/mm/page-states.c
> @@ -21,7 +21,7 @@ void arch_free_page(struct page *page, int order)
> __set_page_unused(page_to_virt(page), 1UL << order);
> }
>
> -void arch_alloc_page(struct page *page, int order)
> +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags)
> {
> if (!cmma_flag)
> return;
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index de292a007138..9e8aa3d144db 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -172,7 +172,7 @@ static inline struct zonelist *node_zonelist(int nid,
> gfp_t flags)
> static inline void arch_free_page(struct page *page, int order) { }
> #endif
> #ifndef HAVE_ARCH_ALLOC_PAGE
> -static inline void arch_alloc_page(struct page *page, int order) { }
> +static inline void arch_alloc_page(struct page *page, int order, gfp_t
> gfp_flags) { }
> #endif
>
> struct page *__alloc_pages(gfp_t gfp, unsigned int order, int preferred_nid,
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 150d4f23b010..2c140abe5ee6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1485,7 +1485,7 @@ inline void post_alloc_hook(struct page *page, unsigned
> int order,
> set_page_private(page, 0);
> set_page_refcounted(page);
>
> - arch_alloc_page(page, order);
> + arch_alloc_page(page, order, gfp_flags);
> debug_pagealloc_map_pages(page, 1 << order);
>
> /*
Otherwise LGTM.
