linux-kernel - Re: [PATCH 07/14] mm: consider zone which is not fully populated to have holes

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <3d5d4d0b-bc5e-dbb5-de68-f4ea31abb38c@suse.cz>
Date:   Fri, 19 May 2017 09:21:20 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        Mel Gorman <mgorman@...e.de>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Jerome Glisse <jglisse@...hat.com>,
        Reza Arbab <arbab@...ux.vnet.ibm.com>,
        Yasuaki Ishimatsu <yasu.isimatu@...il.com>,
        qiuxishi@...wei.com, Kani Toshimitsu <toshi.kani@....com>,
        slaoub@...il.com, Joonsoo Kim <js1304@...il.com>,
        Andi Kleen <ak@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Daniel Kiper <daniel.kiper@...cle.com>,
        Igor Mammedov <imammedo@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 07/14] mm: consider zone which is not fully populated to
 have holes

On 05/18/2017 06:42 PM, Michal Hocko wrote:
> On Thu 18-05-17 18:14:39, Vlastimil Babka wrote:
>> On 05/15/2017 10:58 AM, Michal Hocko wrote:
> [...]
>>>  #ifdef CONFIG_MEMORY_HOTPLUG
>>> +/*
>>> + * Return page for the valid pfn only if the page is online. All pfn
>>> + * walkers which rely on the fully initialized page->flags and others
>>> + * should use this rather than pfn_valid && pfn_to_page
>>> + */
>>> +#define pfn_to_online_page(pfn)				\
>>> +({							\
>>> +	struct page *___page = NULL;			\
>>> +							\
>>> +	if (online_section_nr(pfn_to_section_nr(pfn)))	\
>>> +		___page = pfn_to_page(pfn);		\
>>> +	___page;					\
>>> +})
>>
>> This seems to be already assuming pfn_valid() to be true. There's no
>> "pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS" check and the comment
>> suggests as such, but...
> 
> Yes, we should check the validity of the section number. We do not have
> to check whether the section is valid because online sections are a
> subset of those that are valid.
> 
>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index 05796ee974f7..c3a146028ba6 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -929,6 +929,9 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
>>>  	unsigned long i;
>>>  	unsigned long onlined_pages = *(unsigned long *)arg;
>>>  	struct page *page;
>>> +
>>> +	online_mem_sections(start_pfn, start_pfn + nr_pages);
>>
>> Shouldn't this be moved *below* the loop that initializes struct pages?
>> In the offline case you do mark sections offline before "tearing" struct
>> pages, so that should be symmetric.
> 
> You are right! Andrew, could you fold the following intot the patch?
> ---
> From 0550b61203d6970b47fd79f5e6372dccd143cbec Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@...e.com>
> Date: Thu, 18 May 2017 18:38:24 +0200
> Subject: [PATCH] fold me "mm: consider zone which is not fully populated to
>  have holes"
> 
> - check valid section number in pfn_to_online_page - Vlastimil
> - mark sections online after all struct pages are initialized in
>   online_pages_range - Vlastimil
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>

Both the patch and fix:

Acked-by: Vlastimil Babka <vbabka@...e.cz>