Re: Re: [PATCH] mm: walk the zone in pageblock_nr_pages steps

[zhong jiang <zhongjiang@huawei.com>]

On 2016/7/26 14:24, Vlastimil Babka wrote:
> On 07/26/2016 05:08 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> 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@huawei.com>
>> ---
>>  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@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
>


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>