On 2017年08月15日 17:58, Mel Gorman wrote:
> On Tue, Aug 15, 2017 at 04:45:36PM +0800, Kemi Wang wrote:
>> Threshold CPU cycles Throughput(88 threads)
>> 32 799 241760478
>> 64 640 301628829
>> 125 537 358906028 <==> system by default (base)
>> 256 468 412397590
>> 512 428 450550704
>> 4096 399 482520943
>> 20000 394 489009617
>> 30000 395 488017817
>> 32765 394(-26.6%) 488932078(+36.2%) <==> with this patchset
>> N/A 342(-36.3%) 562900157(+56.8%) <==> disable zone_statistics
>>
>> Signed-off-by: Kemi Wang <kemi.wang@xxxxxxxxx>
>> Suggested-by: Dave Hansen <dave.hansen@xxxxxxxxx>
>> Suggested-by: Ying Huang <ying.huang@xxxxxxxxx>
>> ---
>> include/linux/mmzone.h | 4 ++--
>> include/linux/vmstat.h | 6 +++++-
>> mm/vmstat.c | 23 ++++++++++-------------
>> 3 files changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 0b11ba7..7eaf0e8 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -282,8 +282,8 @@ struct per_cpu_pageset {
>> struct per_cpu_pages pcp;
>> #ifdef CONFIG_NUMA
>> s8 expire;
>> - s8 numa_stat_threshold;
>> - s8 vm_numa_stat_diff[NR_VM_ZONE_NUMA_STAT_ITEMS];
>> + s16 numa_stat_threshold;
>> + s16 vm_numa_stat_diff[NR_VM_ZONE_NUMA_STAT_ITEMS];
>
> I'm fairly sure this pushes the size of that structure into the next
> cache line which is not welcome.
>
Hi Mel
I am refreshing this patch. Would you pls be more explicit of what "that
structure" indicates.
If you mean "struct per_cpu_pageset", for 64 bits machine, this structure
still occupies two caches line after extending s8 to s16/u16, that should
not be a problem. For 32 bits machine, we probably does not need to extend
the size of vm_numa_stat_diff[] since 32 bits OS nearly not be used in large
numa system, and s8/u8 is large enough for it, in this case, we can keep the
same size of "struct per_cpu_pageset".
If you mean "s16 vm_numa_stat_diff[]", and want to keep it in a single cache
line, we probably can add some padding after "s8 expire" to achieve it.
Again, thanks for your comments to make this patch more graceful.
> vm_numa_stat_diff is an always incrementing field. How much do you gain
> if this becomes a u8 code and remove any code that deals with negative
> values? That would double the threshold without consuming another cache line.
>
> Furthermore, the stats in question are only ever incremented by one.
> That means that any calcluation related to overlap can be removed and
> special cased that it'll never overlap by more than 1. That potentially
> removes code that is required for other stats but not locality stats.
> This may give enough savings to avoid moving to s16.
>
> Very broadly speaking, I like what you're doing but I would like to see
> more work on reducing any unnecessary code in that path (such as dealing
> with overlaps for single increments) and treat incrasing the cache footprint
> only as a very last resort.
>
>>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
