linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: zhong jiang <zhongjiang@huawei.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH] mm: walk the zone in pageblock_nr_pages steps
Date: Tue, 26 Jul 2016 17:30:59 +0800	[thread overview]
Message-ID: <57972DD3.3050909@huawei.com> (raw)
In-Reply-To: <473964c8-23cd-cee7-b25c-6ef020547b9a@suse.cz>

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@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.
>
> 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@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>

  reply	other threads:[~2016-07-26  9:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-26  3:08 [PATCH] mm: walk the zone in pageblock_nr_pages steps zhongjiang
2016-07-26  3:08 ` zhongjiang
2016-07-26  6:24 ` Vlastimil Babka
2016-07-26  8:31   ` zhong jiang
2016-07-26  8:53     ` Vlastimil Babka
2016-07-26  9:30       ` zhong jiang [this message]
2016-07-28  6:57         ` Joonsoo Kim
2016-07-28  7:23           ` zhong jiang
  -- strict thread matches above, loose matches on Subject: below --
2016-07-23 12:26 zhongjiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57972DD3.3050909@huawei.com \
    --to=zhongjiang@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-mm@kvack.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).