Re: [PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
On 1/30/24 17:27, Alexandru Elisei wrote:
> Hi,
>
> On Tue, Jan 30, 2024 at 10:04:02AM +0530, Anshuman Khandual wrote:
>>
>>
>> On 1/29/24 17:16, Alexandru Elisei wrote:
>>> Hi,
>>>
>>> On Mon, Jan 29, 2024 at 02:31:23PM +0530, Anshuman Khandual wrote:
On 1/25/24 22:12, Alexandru Elisei wrote:
> The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
> removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
> because CMA is always allowed when __GFP_MOVABLE is set.
>
> With the introduction of the arch_alloc_cma() function, the above is not
> true anymore, so bring back the filter.
This makes sense as arch_alloc_cma() now might prevent ALLOC_CMA being
assigned to alloc_flags in gfp_to_alloc_flags_cma().
>>>
>>> Can I add your Reviewed-by tag then?
>>
>> I think all these changes need to be reviewed in their entirety
>> even though some patches do look good on their own. For example
>> this patch depends on whether [PATCH 03/35] is acceptable or not.
>>
>> I would suggest separating out CMA patches which could be debated
>> and merged regardless of this series.
>
> Ah, I see, makes sense. Since basically all the core mm changes are there
> to enable dynamic tag storage for arm64, I'll hold on until the series
> stabilises before separating the core mm from the arm64 patches.
Fair enough but at least could you please separate out this particular
patch right away and send across.
mm: cma: Don't append newline when generating CMA area name
Re: [PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
Hi,
On Tue, Jan 30, 2024 at 10:04:02AM +0530, Anshuman Khandual wrote:
>
>
> On 1/29/24 17:16, Alexandru Elisei wrote:
> > Hi,
> >
> > On Mon, Jan 29, 2024 at 02:31:23PM +0530, Anshuman Khandual wrote:
> >>
> >>
> >> On 1/25/24 22:12, Alexandru Elisei wrote:
> >>> The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
> >>> removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
> >>> because CMA is always allowed when __GFP_MOVABLE is set.
> >>>
> >>> With the introduction of the arch_alloc_cma() function, the above is not
> >>> true anymore, so bring back the filter.
> >>
> >> This makes sense as arch_alloc_cma() now might prevent ALLOC_CMA being
> >> assigned to alloc_flags in gfp_to_alloc_flags_cma().
> >
> > Can I add your Reviewed-by tag then?
>
> I think all these changes need to be reviewed in their entirety
> even though some patches do look good on their own. For example
> this patch depends on whether [PATCH 03/35] is acceptable or not.
>
> I would suggest separating out CMA patches which could be debated
> and merged regardless of this series.
Ah, I see, makes sense. Since basically all the core mm changes are there
to enable dynamic tag storage for arm64, I'll hold on until the series
stabilises before separating the core mm from the arm64 patches.
Thanks,
Alex
Re: [PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
On 1/29/24 17:16, Alexandru Elisei wrote:
> Hi,
>
> On Mon, Jan 29, 2024 at 02:31:23PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 1/25/24 22:12, Alexandru Elisei wrote:
>>> The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
>>> removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
>>> because CMA is always allowed when __GFP_MOVABLE is set.
>>>
>>> With the introduction of the arch_alloc_cma() function, the above is not
>>> true anymore, so bring back the filter.
>>
>> This makes sense as arch_alloc_cma() now might prevent ALLOC_CMA being
>> assigned to alloc_flags in gfp_to_alloc_flags_cma().
>
> Can I add your Reviewed-by tag then?
I think all these changes need to be reviewed in their entirety
even though some patches do look good on their own. For example
this patch depends on whether [PATCH 03/35] is acceptable or not.
I would suggest separating out CMA patches which could be debated
and merged regardless of this series.
Re: [PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
Hi,
On Mon, Jan 29, 2024 at 02:31:23PM +0530, Anshuman Khandual wrote:
>
>
> On 1/25/24 22:12, Alexandru Elisei wrote:
> > The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
> > removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
> > because CMA is always allowed when __GFP_MOVABLE is set.
> >
> > With the introduction of the arch_alloc_cma() function, the above is not
> > true anymore, so bring back the filter.
>
> This makes sense as arch_alloc_cma() now might prevent ALLOC_CMA being
> assigned to alloc_flags in gfp_to_alloc_flags_cma().
Can I add your Reviewed-by tag then?
Thanks,
Alex
>
> >
> > This is a partially revert because the stale comment remains removed.
> >
> > Signed-off-by: Alexandru Elisei
> > ---
> > mm/page_alloc.c | 15 +++
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index a96d47a6393e..0fa34bcfb1af 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -2897,10 +2897,17 @@ struct page *rmqueue(struct zone *preferred_zone,
> > WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));
> >
> > if (likely(pcp_allowed_order(order))) {
> > - page = rmqueue_pcplist(preferred_zone, zone, order,
> > - migratetype, alloc_flags);
> > - if (likely(page))
> > - goto out;
> > + /*
> > +* MIGRATE_MOVABLE pcplist could have the pages on CMA area and
> > +* we need to skip it when CMA area isn't allowed.
> > +*/
> > + if (!IS_ENABLED(CONFIG_CMA) || alloc_flags & ALLOC_CMA ||
> > + migratetype != MIGRATE_MOVABLE) {
> > + page = rmqueue_pcplist(preferred_zone, zone, order,
> > + migratetype, alloc_flags);
> > + if (likely(page))
> > + goto out;
> > + }
> > }
> >
> > page = rmqueue_buddy(preferred_zone, zone, order, alloc_flags,
Re: [PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
On 1/25/24 22:12, Alexandru Elisei wrote:
> The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
> removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
> because CMA is always allowed when __GFP_MOVABLE is set.
>
> With the introduction of the arch_alloc_cma() function, the above is not
> true anymore, so bring back the filter.
This makes sense as arch_alloc_cma() now might prevent ALLOC_CMA being
assigned to alloc_flags in gfp_to_alloc_flags_cma().
>
> This is a partially revert because the stale comment remains removed.
>
> Signed-off-by: Alexandru Elisei
> ---
> mm/page_alloc.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a96d47a6393e..0fa34bcfb1af 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2897,10 +2897,17 @@ struct page *rmqueue(struct zone *preferred_zone,
> WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));
>
> if (likely(pcp_allowed_order(order))) {
> - page = rmqueue_pcplist(preferred_zone, zone, order,
> -migratetype, alloc_flags);
> - if (likely(page))
> - goto out;
> + /*
> + * MIGRATE_MOVABLE pcplist could have the pages on CMA area and
> + * we need to skip it when CMA area isn't allowed.
> + */
> + if (!IS_ENABLED(CONFIG_CMA) || alloc_flags & ALLOC_CMA ||
> + migratetype != MIGRATE_MOVABLE) {
> + page = rmqueue_pcplist(preferred_zone, zone, order,
> + migratetype, alloc_flags);
> + if (likely(page))
> + goto out;
> + }
> }
>
> page = rmqueue_buddy(preferred_zone, zone, order, alloc_flags,
[PATCH RFC v3 04/35] mm: page_alloc: Partially revert "mm: page_alloc: remove stale CMA guard code"
The patch f945116e4e19 ("mm: page_alloc: remove stale CMA guard code")
removed the CMA filter when allocating from the MIGRATE_MOVABLE pcp list
because CMA is always allowed when __GFP_MOVABLE is set.
With the introduction of the arch_alloc_cma() function, the above is not
true anymore, so bring back the filter.
This is a partially revert because the stale comment remains removed.
Signed-off-by: Alexandru Elisei
---
mm/page_alloc.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a96d47a6393e..0fa34bcfb1af 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2897,10 +2897,17 @@ struct page *rmqueue(struct zone *preferred_zone,
WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));
if (likely(pcp_allowed_order(order))) {
- page = rmqueue_pcplist(preferred_zone, zone, order,
- migratetype, alloc_flags);
- if (likely(page))
- goto out;
+ /*
+* MIGRATE_MOVABLE pcplist could have the pages on CMA area and
+* we need to skip it when CMA area isn't allowed.
+*/
+ if (!IS_ENABLED(CONFIG_CMA) || alloc_flags & ALLOC_CMA ||
+ migratetype != MIGRATE_MOVABLE) {
+ page = rmqueue_pcplist(preferred_zone, zone, order,
+ migratetype, alloc_flags);
+ if (likely(page))
+ goto out;
+ }
}
page = rmqueue_buddy(preferred_zone, zone, order, alloc_flags,
--
2.43.0
