On 2016/7/26 16:53, Vlastimil Babka wrote:
> On 07/26/2016 10:31 AM, zhong jiang wrote:
>> 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.
>
> Huh, x86 doesn't even have CONFIG_HOLES_IN_ZONE? So any pfn valid within MAX_ORDER_NR_PAGES (and within zone boundaries?) should mean the whole range is valid? AFAICS only ia64, mips and s390 has CONFIG_HOLES_IN_ZONE.
>
> Maybe I misunderstand... can you help by demonstrating on which arch and configuration your patch makes a difference?
>
a x86 arch ,for example, when CONFIG_HOLES_IN_ZONE disable, hole punch is not existence. we scan the zone in the way of pageblock ,compared with the MAX_ORDER_NR_PAGES, it should be more resonable.
when CONFIG_HOLES_IN_ZONE enable, hole punch is existence. it will prevent the rest 2M to be skipped. you can get from code that we prefer to align with pageblock.
>> 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)
>
> Hmm I don't see such commit in linus.git, mmotm or linux-next trees.
>
>>>> 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>