| Message ID | 20230823131350.114942-21-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,01/37] mm: page_alloc: Rename gfp_to_alloc_flags_cma -> gfp_to_alloc_flags_fast | expand |
Hi Alexandru, On Wed, Aug 23, 2023 at 6:16 AM Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > If the source page being migrated has metadata associated with it, make > sure to reserve the metadata storage when choosing a suitable destination > page from the free list. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > mm/compaction.c | 9 +++++++++ > mm/internal.h | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/mm/compaction.c b/mm/compaction.c > index cc0139fa0cb0..af2ee3085623 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -570,6 +570,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > bool locked = false; > unsigned long blockpfn = *start_pfn; > unsigned int order; > + int ret; > > /* Strict mode is for isolation, speed is secondary */ > if (strict) > @@ -626,6 +627,11 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > > /* Found a free page, will break it into order-0 pages */ > order = buddy_order(page); > + if (metadata_storage_enabled() && cc->reserve_metadata) { > + ret = reserve_metadata_storage(page, order, cc->gfp_mask); At this point the zone lock is held and preemption is disabled, which makes it invalid to call reserve_metadata_storage. Peter > + if (ret) > + goto isolate_fail; > + } > isolated = __isolate_free_page(page, order); > if (!isolated) > break; > @@ -1757,6 +1763,9 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data) > struct compact_control *cc = (struct compact_control *)data; > struct folio *dst; > > + if (metadata_storage_enabled()) > + cc->reserve_metadata = folio_has_metadata(src); > + > if (list_empty(&cc->freepages)) { > isolate_freepages(cc); > > diff --git a/mm/internal.h b/mm/internal.h > index d28ac0085f61..046cc264bfbe 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -492,6 +492,7 @@ struct compact_control { > */ > bool alloc_contig; /* alloc_contig_range allocation */ > bool source_has_metadata; /* source pages have associated metadata */ > + bool reserve_metadata; > }; > > /* > -- > 2.41.0 >
Hi Peter, On Mon, Nov 20, 2023 at 08:49:32PM -0800, Peter Collingbourne wrote: > Hi Alexandru, > > On Wed, Aug 23, 2023 at 6:16 AM Alexandru Elisei > <alexandru.elisei@arm.com> wrote: > > > > If the source page being migrated has metadata associated with it, make > > sure to reserve the metadata storage when choosing a suitable destination > > page from the free list. > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > --- > > mm/compaction.c | 9 +++++++++ > > mm/internal.h | 1 + > > 2 files changed, 10 insertions(+) > > > > diff --git a/mm/compaction.c b/mm/compaction.c > > index cc0139fa0cb0..af2ee3085623 100644 > > --- a/mm/compaction.c > > +++ b/mm/compaction.c > > @@ -570,6 +570,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > > bool locked = false; > > unsigned long blockpfn = *start_pfn; > > unsigned int order; > > + int ret; > > > > /* Strict mode is for isolation, speed is secondary */ > > if (strict) > > @@ -626,6 +627,11 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > > > > /* Found a free page, will break it into order-0 pages */ > > order = buddy_order(page); > > + if (metadata_storage_enabled() && cc->reserve_metadata) { > > + ret = reserve_metadata_storage(page, order, cc->gfp_mask); > > At this point the zone lock is held and preemption is disabled, which > makes it invalid to call reserve_metadata_storage. You are correct, I missed that. I dropped reserving tag storage during compaction in the next iteration, so fortunately I unintentionally fixed it. Thanks, Alex > > Peter > > > + if (ret) > > + goto isolate_fail; > > + } > > isolated = __isolate_free_page(page, order); > > if (!isolated) > > break; > > @@ -1757,6 +1763,9 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data) > > struct compact_control *cc = (struct compact_control *)data; > > struct folio *dst; > > > > + if (metadata_storage_enabled()) > > + cc->reserve_metadata = folio_has_metadata(src); > > + > > if (list_empty(&cc->freepages)) { > > isolate_freepages(cc); > > > > diff --git a/mm/internal.h b/mm/internal.h > > index d28ac0085f61..046cc264bfbe 100644 > > --- a/mm/internal.h > > +++ b/mm/internal.h > > @@ -492,6 +492,7 @@ struct compact_control { > > */ > > bool alloc_contig; /* alloc_contig_range allocation */ > > bool source_has_metadata; /* source pages have associated metadata */ > > + bool reserve_metadata; > > }; > > > > /* > > -- > > 2.41.0 > >
diff --git a/mm/compaction.c b/mm/compaction.c index cc0139fa0cb0..af2ee3085623 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -570,6 +570,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, bool locked = false; unsigned long blockpfn = *start_pfn; unsigned int order; + int ret; /* Strict mode is for isolation, speed is secondary */ if (strict) @@ -626,6 +627,11 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, /* Found a free page, will break it into order-0 pages */ order = buddy_order(page); + if (metadata_storage_enabled() && cc->reserve_metadata) { + ret = reserve_metadata_storage(page, order, cc->gfp_mask); + if (ret) + goto isolate_fail; + } isolated = __isolate_free_page(page, order); if (!isolated) break; @@ -1757,6 +1763,9 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data) struct compact_control *cc = (struct compact_control *)data; struct folio *dst; + if (metadata_storage_enabled()) + cc->reserve_metadata = folio_has_metadata(src); + if (list_empty(&cc->freepages)) { isolate_freepages(cc); diff --git a/mm/internal.h b/mm/internal.h index d28ac0085f61..046cc264bfbe 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -492,6 +492,7 @@ struct compact_control { */ bool alloc_contig; /* alloc_contig_range allocation */ bool source_has_metadata; /* source pages have associated metadata */ + bool reserve_metadata; }; /*
If the source page being migrated has metadata associated with it, make sure to reserve the metadata storage when choosing a suitable destination page from the free list. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- mm/compaction.c | 9 +++++++++ mm/internal.h | 1 + 2 files changed, 10 insertions(+)