linux-kernel - Re: [PATCH 2/3] mm: Introduce subsection_dev

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]

Date:   Wed, 13 Nov 2019 19:51:27 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Dan Williams <dan.j.williams@...el.com>,
        Toshiki Fukasawa <t-fukasawa@...jp.nec.com>
Cc:     "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "mhocko@...nel.org" <mhocko@...nel.org>,
        "adobriyan@...il.com" <adobriyan@...il.com>,
        "hch@....de" <hch@....de>,
        "longman@...hat.com" <longman@...hat.com>,
        "sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
        "mst@...hat.com" <mst@...hat.com>, "cai@....pw" <cai@....pw>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Junichi Nomura <j-nomura@...jp.nec.com>
Subject: Re: [PATCH 2/3] mm: Introduce subsection_dev_map

On 08.11.19 20:13, Dan Williams wrote:
> On Thu, Nov 7, 2019 at 4:15 PM Toshiki Fukasawa
> <t-fukasawa@...jp.nec.com> wrote:
>>
>> Currently, there is no way to identify pfn on ZONE_DEVICE.
>> Identifying pfn on system memory can be done by using a
>> section-level flag. On the other hand, identifying pfn on
>> ZONE_DEVICE requires a subsection-level flag since ZONE_DEVICE
>> can be created in units of subsections.
>>
>> This patch introduces a new bitmap subsection_dev_map so that
>> we can identify pfn on ZONE_DEVICE.
>>
>> Also, subsection_dev_map is used to prove that struct pages
>> included in the subsection have been initialized since it is
>> set after memmap_init_zone_device(). We can avoid accessing
>> pages currently being initialized by checking subsection_dev_map.
>>
>> Signed-off-by: Toshiki Fukasawa <t-fukasawa@...jp.nec.com>
>> ---
>>   include/linux/mmzone.h | 19 +++++++++++++++++++
>>   mm/memremap.c          |  2 ++
>>   mm/sparse.c            | 32 ++++++++++++++++++++++++++++++++
>>   3 files changed, 53 insertions(+)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index bda2028..11376c4 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -1174,11 +1174,17 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
>>
>>   struct mem_section_usage {
>>          DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
>> +#ifdef CONFIG_ZONE_DEVICE
>> +       DECLARE_BITMAP(subsection_dev_map, SUBSECTIONS_PER_SECTION);
>> +#endif
> 
> Hi Toshiki,
> 
> There is currently an effort to remove the PageReserved() flag as some
> code is using that to detect ZONE_DEVICE. In reviewing those patches
> we realized that what many code paths want is to detect online memory.
> So instead of a subsection_dev_map add a subsection_online_map. That
> way pfn_to_online_page() can reliably avoid ZONE_DEVICE ranges. I
> otherwise question the use case for pfn_walkers to return pages for
> ZONE_DEVICE pages, I think the skip behavior when pfn_to_online_page()
> == false is the right behavior.

To be more precise, I recommended an subsection_active_map, to indicate 
which memmaps were fully initialized and can safely be touched (e.g., to 
read the zone/nid). This map would also be set when the devmem memmaps 
were initialized (race between adding memory/growing the section and 
initializing the memmap).

See

https://lkml.org/lkml/2019/10/10/87

and

https://www.spinics.net/lists/linux-driver-devel/msg130012.html

I dislike a map that is specific to ZONE_DEVICE or (currently) 
!ZONE_DEVICE. I rather want an indication "this memmap is safe to 
touch". As discussed along the mentioned threads, we can combine this 
later with RCU to handle some races that are currently possible.

-- 

Thanks,

David / dhildenb