| Message ID | 20241203094732.200195-6-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/page_alloc: gfp flags cleanups for alloc_contig_*() | expand |
On 12/3/24 10:47, David Hildenbrand wrote: > In the __GFP_COMP case, we already pass the gfp_flags to > prep_new_page()->post_alloc_hook(). However, in the !__GFP_COMP case, we > essentially pass only hardcoded __GFP_MOVABLE to post_alloc_hook(), > preventing some action modifiers from being effective.. > > Let's pass our now properly adjusted gfp flags there as well. > > This way, we can now support __GFP_ZERO for alloc_contig_*(). > > As a side effect, we now also support __GFP_SKIP_ZERO and__GFP_ZEROTAGS; > but we'll keep the more special stuff (KASAN, NOLOCKDEP) disabled for > now. > > It's worth noting that with __GFP_ZERO, we might unnecessarily zero pages > when we have to release part of our range using free_contig_range() again. > This can be optimized in the future, if ever required; the caller we'll > be converting (powernv/memtrace) next won't trigger this. > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/page_alloc.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 54594cc4f650..71d70bc0ad79 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -6364,7 +6364,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc, > return (ret < 0) ? ret : 0; > } > > -static void split_free_pages(struct list_head *list) > +static void split_free_pages(struct list_head *list, gfp_t gfp_mask) > { > int order; > > @@ -6375,7 +6375,7 @@ static void split_free_pages(struct list_head *list) > list_for_each_entry_safe(page, next, &list[order], lru) { > int i; > > - post_alloc_hook(page, order, __GFP_MOVABLE); > + post_alloc_hook(page, order, gfp_mask); > set_page_refcounted(page); > if (!order) > continue; > @@ -6393,7 +6393,8 @@ static void split_free_pages(struct list_head *list) > static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask) > { > const gfp_t reclaim_mask = __GFP_IO | __GFP_FS | __GFP_RECLAIM; > - const gfp_t action_mask = __GFP_COMP | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > + const gfp_t action_mask = __GFP_COMP | __GFP_RETRY_MAYFAIL | __GFP_NOWARN | > + __GFP_ZERO | __GFP_ZEROTAGS | __GFP_SKIP_ZERO; > const gfp_t cc_action_mask = __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > /* > @@ -6541,7 +6542,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end, > } > > if (!(gfp_mask & __GFP_COMP)) { > - split_free_pages(cc.freepages); > + split_free_pages(cc.freepages, gfp_mask); > > /* Free head and tail (if any) */ > if (start != outer_start)
On Tue, Dec 03, 2024 at 10:47:31AM +0100, David Hildenbrand wrote: > In the __GFP_COMP case, we already pass the gfp_flags to > prep_new_page()->post_alloc_hook(). However, in the !__GFP_COMP case, we > essentially pass only hardcoded __GFP_MOVABLE to post_alloc_hook(), > preventing some action modifiers from being effective.. > > Let's pass our now properly adjusted gfp flags there as well. > > This way, we can now support __GFP_ZERO for alloc_contig_*(). > > As a side effect, we now also support __GFP_SKIP_ZERO and__GFP_ZEROTAGS; > but we'll keep the more special stuff (KASAN, NOLOCKDEP) disabled for > now. > > It's worth noting that with __GFP_ZERO, we might unnecessarily zero pages > when we have to release part of our range using free_contig_range() again. > This can be optimized in the future, if ever required; the caller we'll > be converting (powernv/memtrace) next won't trigger this. > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Oscar Salvador <osalvador@suse.de>
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 54594cc4f650..71d70bc0ad79 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -6364,7 +6364,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc, return (ret < 0) ? ret : 0; } -static void split_free_pages(struct list_head *list) +static void split_free_pages(struct list_head *list, gfp_t gfp_mask) { int order; @@ -6375,7 +6375,7 @@ static void split_free_pages(struct list_head *list) list_for_each_entry_safe(page, next, &list[order], lru) { int i; - post_alloc_hook(page, order, __GFP_MOVABLE); + post_alloc_hook(page, order, gfp_mask); set_page_refcounted(page); if (!order) continue; @@ -6393,7 +6393,8 @@ static void split_free_pages(struct list_head *list) static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask) { const gfp_t reclaim_mask = __GFP_IO | __GFP_FS | __GFP_RECLAIM; - const gfp_t action_mask = __GFP_COMP | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; + const gfp_t action_mask = __GFP_COMP | __GFP_RETRY_MAYFAIL | __GFP_NOWARN | + __GFP_ZERO | __GFP_ZEROTAGS | __GFP_SKIP_ZERO; const gfp_t cc_action_mask = __GFP_RETRY_MAYFAIL | __GFP_NOWARN; /* @@ -6541,7 +6542,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end, } if (!(gfp_mask & __GFP_COMP)) { - split_free_pages(cc.freepages); + split_free_pages(cc.freepages, gfp_mask); /* Free head and tail (if any) */ if (start != outer_start)
In the __GFP_COMP case, we already pass the gfp_flags to prep_new_page()->post_alloc_hook(). However, in the !__GFP_COMP case, we essentially pass only hardcoded __GFP_MOVABLE to post_alloc_hook(), preventing some action modifiers from being effective.. Let's pass our now properly adjusted gfp flags there as well. This way, we can now support __GFP_ZERO for alloc_contig_*(). As a side effect, we now also support __GFP_SKIP_ZERO and__GFP_ZEROTAGS; but we'll keep the more special stuff (KASAN, NOLOCKDEP) disabled for now. It's worth noting that with __GFP_ZERO, we might unnecessarily zero pages when we have to release part of our range using free_contig_range() again. This can be optimized in the future, if ever required; the caller we'll be converting (powernv/memtrace) next won't trigger this. Signed-off-by: David Hildenbrand <david@redhat.com> --- mm/page_alloc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)