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
