linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: zhong jiang <zhongjiang@huawei.com>
Cc: Michal Hocko <mhocko@suse.com>,
	akpm@linux-foundation.org, linux-mm@kvack.org,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [patch 13/15] mm/page_owner: align with pageblock_nr pages
Date: Wed, 6 Dec 2017 09:18:14 +0100	[thread overview]
Message-ID: <6e599005-392a-38e1-481e-72ff4195e749@suse.cz> (raw)
In-Reply-To: <5A269613.5090405@huawei.com>

On 12/05/2017 01:50 PM, zhong jiang wrote:
>>>  yes,   limited by my knowledge and english.  Maybe Vlastimil  can  address it  in detail.  
>> Hi, on a fresh look, I believe this patch doesn't improve anything in
>> practice. It potentially makes init_pages_in_zone() catch more early
>> allocations, if a hole happens to be placed in the beginning of
>> MAX_ORDER block, and the following pageblock within the block was early
>> allocated.
>  Hi, Vlastimil
> 
>   I have a stupid question about holes
> 
>   because a hole is possible to have within a MAX_ORDER_NR_PAGES, it indeed
>   exist in first pfn. it that is true, why we must skip the whole MAX_ORDER block?
>   Any limit ?  I can not find the answer.

It's not that we "must skip". If I understand it correctly, on kernels
without CONFIG_HOLES_IN_ZONE, we can skip a MAX_ORDER block if *any* pfn
(including the first pfn) is invalid, because we know that the whole
block is invalid. On CONFIG_HOLES_IN_ZONE, there is no such guarantee.

So if we see that the first pfn is valid, we continue with the block,
but use pfn_valid_within() (which is defined as pfn_valid() on
CONFIG_HOLES_IN_ZONE and hardcoded "true" elsewhere) to validate each
pfn. This is slow, but the arches pay the price for CONFIG_HOLES_IN_ZONE.

If we see that first pfn is invalid, we are safe to skip the MAX_ORDER
block when CONFIG_HOLES_IN_ZONE=n and we know we won't miss anything. On
CONFIG_HOLES_IN_ZONE we might miss something, so to be sure we don't
miss something, we should validate each pfn. The potential price there
is probably worse, because we might be validating arbitrary large holes
not limited by physical amount of RAM. So e.g. compaction doesn't pay
this price, and MAX_ORDER blocks that would have hole at the beginning
and end (with valid pages in the middle) are skipped.

page_owner on the other hand is a debugging feature not normally
enabled, with significant overhead, so paying the price there might not
be an issue. But it means rewriting both init_pages_in_zone() and
read_page_owner() to not skip MAX_ORDER block (nor pageblock_order) when
CONFIG_HOLES_IN_ZONE=y. I don't think there's a simple wrapper similar
to pfn_valid_within() for that, but it could be created (input: current
pfn, output: start pfn of next MAX_ORDER block if
CONFIG_HOLES_IN_ZONE=n, pfn+1 when CONFIG_HOLES_IN_ZONE=y).

>   Thanks
>   zhongjiang
>> However, read_page_owner() skips whole MAX_ORDER block as well in this
>> situation, so we won't be able to read the info anyway...
>>
>> Also the problem is not as simple as documenting MAX_ORDER_NR_PAGES vs
>> pabeblock_nr_pages. We discussed it year ago when this patch was first
>> posted, how skipping over holes would have to be made more robust, and
>> how architectures should define hole granularity to avoid checking each
>> individual pfn in what appears to be a hole, to see if the hole has ended.
>>
>>> Thanks
>>> zhongjiang
>>>
>>
>> .
>>
> 
> 

--
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:[~2017-12-06  8:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30 22:15 akpm
2017-12-01 16:58 ` Vlastimil Babka
2017-12-01 17:15   ` Michal Hocko
2017-12-04 11:51     ` zhong jiang
2017-12-04 12:01       ` Michal Hocko
2017-12-04 12:23         ` zhong jiang
2017-12-04 12:35           ` Michal Hocko
2017-12-04 12:56             ` zhong jiang
2017-12-05 11:22               ` Vlastimil Babka
2017-12-05 11:47                 ` Vlastimil Babka
2017-12-05 12:50                 ` zhong jiang
2017-12-06  8:18                   ` Vlastimil Babka [this message]

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=6e599005-392a-38e1-481e-72ff4195e749@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=zhongjiang@huawei.com \
    /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