On 2016/7/26 14:24, Vlastimil Babka wrote:
> On 07/26/2016 05:08 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@xxxxxxxxxx>
>>
>> when walking the zone, we can happens to the holes. we should not
>> align MAX_ORDER_NR_PAGES, so it can skip the normal memory.
>>
>> In addition, pagetypeinfo_showmixedcount_print reflect fragmentization.
>> we hope to get more accurate data. therefore, I decide to fix it.
>
> Can't say I'm happy with another random half-fix. What's the real granularity of holes for CONFIG_HOLES_IN_ZONE systems? I suspect it can be below pageblock_nr_pages. The pfn_valid_within() mechanism seems rather insufficient... it does prevent running unexpectedly into holes in the middle of pageblock/MAX_ORDER block, but together with the large skipping it doesn't guarantee that we cover all non-holes.
>
I am sorry for that. I did not review the whole code before sending above patch. In arch of x86, The real granularity of holes is in 256, that is a section. while in arm64, we can see that the hole is identify by located in
SYSTEM_RAM. I admit that that is not a best way. but at present, it's a better way to amend.
> I think in a robust solution, functions such as these should use something like PAGE_HOLE_GRANULARITY which equals MAX_ORDER_NR_PAGES for !CONFIG_HOLES_IN_ZONE and some arch/config/system specific value for CONFIG_HOLES_IN_ZONE. This would then be used in the ALIGN() part.
> It could be also used together with pfn_valid_within() in the inner loop to skip over holes more quickly (if it's worth).
>
Maybe reimplement the code about hole punch is a better way.
> Also I just learned there's also CONFIG_ARCH_HAS_HOLES_MEMORYMODEL that affects a function called memmap_valid_within(). But that one has only one caller - pagetypeinfo_showblockcount_print(). Why is it needed there and not in pagetypeinfo_showmixedcount_print() (or anywhere else?)
>
yes, but in other place, for example, the caller apagetypeinfo_showmixedcount_print you can see the commit.(91c43c7313a995a8908f8f6b911a85d00fdbffd)
>> Signed-off-by: zhong jiang <zhongjiang@xxxxxxxxxx>
>> ---
>> mm/vmstat.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/vmstat.c b/mm/vmstat.c
>> index cb2a67b..3508f74 100644
>> --- a/mm/vmstat.c
>> +++ b/mm/vmstat.c
>> @@ -1033,7 +1033,7 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
>> */
>> for (; pfn < end_pfn; ) {
>> if (!pfn_valid(pfn)) {
>> - pfn = ALIGN(pfn + 1, MAX_ORDER_NR_PAGES);
>> + pfn = ALIGN(pfn + 1, pageblock_nr_pages);
>> continue;
>> }
>>
>>
>
> --
> 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>
>
>
--
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>