Re: [RFC] put page to pcp->lists[] tail if it is not on the same node

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Oct 19, 2018 at 03:43:29PM +0200, Vlastimil Babka wrote:
>On 10/19/18 6:33 AM, Wei Yang wrote:
>> @@ -2763,7 +2764,14 @@ static void free_unref_page_commit(struct page *page, unsigned long pfn)
>>  	}
>>  
>>  	pcp = &this_cpu_ptr(zone->pageset)->pcp;
>> -	list_add(&page->lru, &pcp->lists[migratetype]);
>
>My impression is that you think there's only one pcp per cpu. But the
>"pcp" here is already specific to the zone (and thus node) of the page
>being freed. So it doesn't matter if we put the page to the list or
>tail. For allocation we already typically prefer local nodes, thus local
>zones, thus pcp's containing only local pages.
>
>> +	/*
>> +	 * If the page has the same node_id as this cpu, put the page at head.
>> +	 * Otherwise, put at the end.
>> +	 */
>> +	if (page_node == pcp->node)
>
>So this should in fact be always true due to what I explained above.

Vlastimil,

After looking at the code, I got some new understanding of the pcp
pages, which maybe a little different from yours.

Every zone has a per_cpu_pageset for each cpu, and the pages allocated
to per_cpu_pageset is either of the same node with this *cpu* or
different node.

So this comparison (page_node == pcp->node) would always be true or
false for a particular per_cpu_pageset.

Well, one thing for sure is putting a page to tail will not improve the
locality.

>
>Otherwise I second the recommendation from Mel.
>
>Cheers,
>Vlastimil

-- 
Wei Yang
Help you, Help me