Messages in this thread | | | From | "Aneesh Kumar K.V" <> | Subject | Re: [PATCH v7 09/12] mm/demotion: Demote pages according to allocation fallback order | Date | Mon, 27 Jun 2022 09:24:31 +0530 |
| |
Alistair Popple <apopple@nvidia.com> writes:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > >> From: Jagdish Gediya <jvgediya@linux.ibm.com> > > [...] > >> -static struct page *alloc_demote_page(struct page *page, unsigned long node) >> +static struct page *alloc_demote_page(struct page *page, unsigned long private) >> { >> - struct migration_target_control mtc = { >> - /* >> - * Allocate from 'node', or fail quickly and quietly. >> - * When this happens, 'page' will likely just be discarded >> - * instead of migrated. >> - */ >> - .gfp_mask = (GFP_HIGHUSER_MOVABLE & ~__GFP_RECLAIM) | >> - __GFP_THISNODE | __GFP_NOWARN | >> - __GFP_NOMEMALLOC | GFP_NOWAIT, >> - .nid = node >> - }; >> + struct page *target_page; >> + nodemask_t *allowed_mask; >> + struct migration_target_control *mtc; >> + >> + mtc = (struct migration_target_control *)private; >> + >> + allowed_mask = mtc->nmask; >> + /* >> + * make sure we allocate from the target node first also trying to >> + * reclaim pages from the target node via kswapd if we are low on >> + * free memory on target node. If we don't do this and if we have low >> + * free memory on the target memtier, we would start allocating pages >> + * from higher memory tiers without even forcing a demotion of cold >> + * pages from the target memtier. This can result in the kernel placing >> + * hotpages in higher memory tiers. >> + */ >> + mtc->nmask = NULL; >> + mtc->gfp_mask |= __GFP_THISNODE; >> + target_page = alloc_migration_target(page, (unsigned long)&mtc); > > I finally managed to get a system setup to start testing some of this > out. However it quickly crashed due to the bad pointer in the above call > - you need mtc not &mtc here.
I remember fixing that during earlier testing. I guess I missed to copy the change from test to my development. Thanks for testing this. I have now also tested the complete series with the above-suggested changes and did make sure we are indeed doing demotion by looking at /proc/vmstat:pgdemote_kswapd/pgdemote_direct
> >> + if (target_page) >> + return target_page; >> + >> + mtc->gfp_mask &= ~__GFP_THISNODE; >> + mtc->nmask = allowed_mask; >> >> return alloc_migration_target(page, (unsigned long)&mtc); > > And here. >
I will fold this changes in and send a v8 after waiting for review feedback from others.
-aneesh
| |