linux-kernel - Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160526003718.GA9302@js1304-P5Q-DELUXE>
Date:	Thu, 26 May 2016 09:37:19 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	"Shi, Yang" <yang.shi@...aro.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	linaro-kernel@...ts.linaro.org,
	Mel Gorman <mgorman@...hsingularity.net>
Subject: Re: [PATCH] mm: use early_pfn_to_nid in
 register_page_bootmem_info_node

Ccing Mel.

On Wed, May 25, 2016 at 03:36:48PM -0700, Shi, Yang wrote:
> On 5/25/2016 3:23 PM, Andrew Morton wrote:
> >On Wed, 25 May 2016 14:00:07 -0700 Yang Shi <yang.shi@...aro.org> wrote:
> >
> >>register_page_bootmem_info_node() is invoked in mem_init(), so it will be
> >>called before page_alloc_init_late() if CONFIG_DEFERRED_STRUCT_PAGE_INIT
> >>is enabled. But, pfn_to_nid() depends on memmap which won't be fully setup
> >>until page_alloc_init_late() is done, so replace pfn_to_nid() by
> >>early_pfn_to_nid().
> >
> >What are the runtime effects of this fix?
> 
> I didn't experience any problem without the fix. During working on
> the page_ext_init() fix (replace to early_pfn_to_nid()), I added
> printk before each pfn_to_nid() calls to check which one might be
> called before page_alloc_init_late(), then this one is caught.
> 
> From the code perspective, it sounds not right since
> register_page_bootmem_info_section() may miss some pfns when
> CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, just like the problem
> happened in page_ext_init().

Hello, Mel.

There was an issue in page_ext [1] due to your deferred struct page init
feature. Before your change, we assumed that we can use pfn_to_nid()
after memmap init is called. But, after your change, we can use
pfn_to_nid() after page_alloc_init_late(). Yang found two call sites
that uses pfn_to_nid() before page_alloc_init_late() and they could be
fixed by using early_pfn_to_nid(). I guess that there are more
problems due to this change so it's better to check it by patch author.

One thing I have noticed is that dirty_limit could be set wrongly. It
is intialized by using freepage count. Since it is intialized before
page_alloc_init_late(), freepages are not initialized yet and it could
be wrong. If my analysis is correct, please fix it.

And, could you check again that there is no more problem?

Thanks.

[1]
http://lkml.kernel.org/r/CAAmzW4OUmyPwQjvd7QUfc6W1Aic__TyAuH80MLRZNMxKy0-wPQ@mail.gmail.com