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.
>

Your guess is right. :-)

I took a look in the code

    zone->pageset = alloc_percpu(struct per_cpu_pageset);

each zone has its pageset.

This means just a portion of the pageset is used on a multi-node
system, since a node just belongs to one node. Could we allocate just
this part or initialize just this part? Maybe it is too small to polish.

Well, I am lost on when we will allocate a page from remote node. Let me
try to understand :-)

>> +	/*
>> +	 * 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.
>
>Otherwise I second the recommendation from Mel.
>

Sure, I have to say you are right.

BTW, is there other channel not as formal as mail list to raise some
question or discussion? Reading the code alone is not that exciting and
sometimes when I get some idea or confusion, I really willing to chat
with someone or to understand why it is so.

Mail list seems not the proper channel, maybe the irc is a proper way?

>Cheers,
>Vlastimil

-- 
Wei Yang
Help you, Help me