| Message ID | 20230109151631.24923-2-mgorman@techsingularity.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Discard __GFP_ATOMIC | expand |
On Mon 09-01-23 15:16:25, Mel Gorman wrote: > __GFP_HIGH aliases to ALLOC_HIGH but the name does not really hint > what it means. As ALLOC_HIGH is internal to the allocator, rename > it to ALLOC_MIN_RESERVE to document that the min reserves can > be depleted. > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > Acked-by: Vlastimil Babka <vbabka@suse.cz> Naming is hard but ALLOC_HIGH is definitely much more confusing as it can collide with high watermark. ALLOC_MIN_RESERVE says that some reserves are involved. ALl the reserves are below min watermark by defition but I cannot really come up with a better name. I do not think we want to encode the amount of reserves into the name. Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/internal.h | 4 +++- > mm/page_alloc.c | 8 ++++---- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/mm/internal.h b/mm/internal.h > index bcf75a8b032d..403e4386626d 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -736,7 +736,9 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, > #endif > > #define ALLOC_HARDER 0x10 /* try to alloc harder */ > -#define ALLOC_HIGH 0x20 /* __GFP_HIGH set */ > +#define ALLOC_MIN_RESERVE 0x20 /* __GFP_HIGH set. Allow access to 50% > + * of the min watermark. > + */ > #define ALLOC_CPUSET 0x40 /* check for correct cpuset */ > #define ALLOC_CMA 0x80 /* allow allocations from CMA areas */ > #ifdef CONFIG_ZONE_DMA32 > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 0745aedebb37..244c1e675dc8 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -3976,7 +3976,7 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark, > /* free_pages may go negative - that's OK */ > free_pages -= __zone_watermark_unusable_free(z, order, alloc_flags); > > - if (alloc_flags & ALLOC_HIGH) > + if (alloc_flags & ALLOC_MIN_RESERVE) > min -= min / 2; > > if (unlikely(alloc_harder)) { > @@ -4818,18 +4818,18 @@ gfp_to_alloc_flags(gfp_t gfp_mask) > unsigned int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET; > > /* > - * __GFP_HIGH is assumed to be the same as ALLOC_HIGH > + * __GFP_HIGH is assumed to be the same as ALLOC_MIN_RESERVE > * and __GFP_KSWAPD_RECLAIM is assumed to be the same as ALLOC_KSWAPD > * to save two branches. > */ > - BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_HIGH); > + BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_MIN_RESERVE); > BUILD_BUG_ON(__GFP_KSWAPD_RECLAIM != (__force gfp_t) ALLOC_KSWAPD); > > /* > * The caller may dip into page reserves a bit more if the caller > * cannot run direct reclaim, or if the caller has realtime scheduling > * policy or is asking for __GFP_HIGH memory. GFP_ATOMIC requests will > - * set both ALLOC_HARDER (__GFP_ATOMIC) and ALLOC_HIGH (__GFP_HIGH). > + * set both ALLOC_HARDER (__GFP_ATOMIC) and ALLOC_MIN_RESERVE(__GFP_HIGH). > */ > alloc_flags |= (__force int) > (gfp_mask & (__GFP_HIGH | __GFP_KSWAPD_RECLAIM)); > -- > 2.35.3
On Wed, Jan 11, 2023 at 04:18:50PM +0100, Michal Hocko wrote: > On Mon 09-01-23 15:16:25, Mel Gorman wrote: > > __GFP_HIGH aliases to ALLOC_HIGH but the name does not really hint > > what it means. As ALLOC_HIGH is internal to the allocator, rename > > it to ALLOC_MIN_RESERVE to document that the min reserves can > > be depleted. > > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > > Naming is hard but ALLOC_HIGH is definitely much more confusing as it > can collide with high watermark. ALLOC_MIN_RESERVE says that some > reserves are involved. ALl the reserves are below min watermark by > defition but I cannot really come up with a better name. I do not think > we want to encode the amount of reserves into the name. > It's internal to the page allocator so I didn't sweat about it too much. Access to the reserves currently means "allow pages to be allocated below the min reserve". Even if that changes in the future, the name can change with it. > Acked-by: Michal Hocko <mhocko@suse.com> > Thanks!
diff --git a/mm/internal.h b/mm/internal.h index bcf75a8b032d..403e4386626d 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -736,7 +736,9 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, #endif #define ALLOC_HARDER 0x10 /* try to alloc harder */ -#define ALLOC_HIGH 0x20 /* __GFP_HIGH set */ +#define ALLOC_MIN_RESERVE 0x20 /* __GFP_HIGH set. Allow access to 50% + * of the min watermark. + */ #define ALLOC_CPUSET 0x40 /* check for correct cpuset */ #define ALLOC_CMA 0x80 /* allow allocations from CMA areas */ #ifdef CONFIG_ZONE_DMA32 diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 0745aedebb37..244c1e675dc8 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3976,7 +3976,7 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark, /* free_pages may go negative - that's OK */ free_pages -= __zone_watermark_unusable_free(z, order, alloc_flags); - if (alloc_flags & ALLOC_HIGH) + if (alloc_flags & ALLOC_MIN_RESERVE) min -= min / 2; if (unlikely(alloc_harder)) { @@ -4818,18 +4818,18 @@ gfp_to_alloc_flags(gfp_t gfp_mask) unsigned int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET; /* - * __GFP_HIGH is assumed to be the same as ALLOC_HIGH + * __GFP_HIGH is assumed to be the same as ALLOC_MIN_RESERVE * and __GFP_KSWAPD_RECLAIM is assumed to be the same as ALLOC_KSWAPD * to save two branches. */ - BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_HIGH); + BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_MIN_RESERVE); BUILD_BUG_ON(__GFP_KSWAPD_RECLAIM != (__force gfp_t) ALLOC_KSWAPD); /* * The caller may dip into page reserves a bit more if the caller * cannot run direct reclaim, or if the caller has realtime scheduling * policy or is asking for __GFP_HIGH memory. GFP_ATOMIC requests will - * set both ALLOC_HARDER (__GFP_ATOMIC) and ALLOC_HIGH (__GFP_HIGH). + * set both ALLOC_HARDER (__GFP_ATOMIC) and ALLOC_MIN_RESERVE(__GFP_HIGH). */ alloc_flags |= (__force int) (gfp_mask & (__GFP_HIGH | __GFP_KSWAPD_RECLAIM));