Alistair Popple <apopple@xxxxxxxxxx> writes:
> "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxx> writes:
>
>> From: Jagdish Gediya <jvgediya@xxxxxxxxxxxxx>
>
> [...]
>
>> -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
