linux-kernel - Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added memory range

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:   Thu, 25 Mar 2021 15:40:04 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     Oscar Salvador <osalvador@...e.de>
Cc:     David Hildenbrand <david@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Pavel Tatashin <pasha.tatashin@...een.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added
 memory range

On Thu 25-03-21 15:02:26, Oscar Salvador wrote:
> On Thu, Mar 25, 2021 at 01:26:34PM +0100, Michal Hocko wrote:
> > Yeah, David has raised the contiguous flag for zone already. And to be
> > completely honest I fail to see why we should shape a design based on an
> > optimization. If anything we can teach set_zone_contiguous to simply
> > ignore zone affiliation of vmemmap pages. I would be really curious if
> > that would pose any harm to the compaction code as they are reserved and
> > compaction should simply skip them.
> 
> No, compaction code is clever enough to skip over those pages as it
> already does for any Reserved page.
> My comment was more towards having the zone contiguous.
> 
> I know it is an optimization, but
> 
>  commit 7cf91a98e607c2f935dbcc177d70011e95b8faff
>  Author: Joonsoo Kim <iamjoonsoo.kim@....com>
>  Date:   Tue Mar 15 14:57:51 2016 -0700
>  
>  mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous
> 
> talks about 30% of improvment. I am not sure if those numbers would
> still hold nowawadys, but it feels wrong to drop it to the ground when
> we can do better there, and IMHO, it does not overly complicate things.

Again, do not shape design around an optimization. If this turns out a
real problem then it can be handled on top.

> > THere is nothing like a proper zone.
> 
> I guess not, but for me it makes sense that vmemmap pages stay within
> the same zone as the pages they describe.

This is not the case for normal hotplug so why this should be any
different.

> Of course, this is a matter of opinions/taste.
> 
> > Not sure what you are referring to but if you have prior to f1dd2cd13c4b
> > ("mm, memory_hotplug: do not associate hotadded memory to zones until
> > online") then this was entirely a different story. Users do care where
> > they memory goes because that depends on the usecase but do they care
> > about vmemmap?
> 
> As I said, that is not what I am worried about.
> Users do not really care where those pages end up, that is transparent
> to them (wrt. vmemmap pages), but we (internally) kind of do.
> 
> So, as I said, I see advantatges of using your way, but I see downsides
> as:
> 
> - I would like to consider zone, and for that we would have to pull
>   some of the functions that check for the zone at an aearly stage, and
>   the mere thought sounds ugly.

This is impossible and whatever kind of heuristic you come up with might
be wrong.

> - Section containing vmemmap can remain offline and would have to come
>   up to sort that out

Yes, this is a problem indeed and as I've said in other email this would
be a problem for your initial implementation as well if the memory block
is still offline. I suspect we need to treat these Vmemmap pages as
online (via pfn_to_online_page).
-- 
Michal Hocko
SUSE Labs