Date:   Tue, 17 Jul 2018 10:46:39 -0400
From:   Pavel Tatashin <pasha.tatashin@...cle.com>
To:     dan.j.williams@...el.com
Cc:     Andrew Morton <akpm@...ux-foundation.org>, tony.luck@...el.com,
        yehs1@...ovo.com, vishal.l.verma@...el.com, jack@...e.cz,
        willy@...radead.org, dave.jiang@...el.com, hpa@...or.com,
        tglx@...utronix.de, dalias@...c.org, fenghua.yu@...el.com,
        Daniel Jordan <daniel.m.jordan@...cle.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        benh@...nel.crashing.org, Michal Hocko <mhocko@...e.com>,
        paulus@...ba.org, hch@....de, jglisse@...hat.com, mingo@...hat.com,
        mpe@...erman.id.au, Heiko Carstens <heiko.carstens@...ibm.com>,
        x86@...nel.org, logang@...tatee.com, ross.zwisler@...ux.intel.com,
        jmoyer@...hat.com, jthumshirn@...e.de, schwidefsky@...ibm.com,
        Linux Memory Management List <linux-mm@...ck.org>,
        linux-nvdimm@...ts.01.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 00/14] mm: Asynchronous + multithreaded memmap init for ZONE_DEVICE

> > Hi Dan,
> >
> > I am worried that this work adds another way to multi-thread struct
> > page initialization without re-use of already existing method. The
> > code is already a mess, and leads to bugs [1] because of the number of
> > different memory layouts, architecture specific quirks, and different
> > struct page initialization methods.
>
> Yes, the lamentations about the complexity of the memory hotplug code
> are known. I didn't think this set made it irretrievably worse, but
> I'm biased and otherwise certainly want to build consensus with other
> mem-hotplug folks.
>
> >
> > So, when DEFERRED_STRUCT_PAGE_INIT is used we initialize struct pages
> > on demand until page_alloc_init_late() is called, and at that time we
> > initialize all the rest of struct pages by calling:
> >
> > page_alloc_init_late()
> >   deferred_init_memmap() (a thread per node)
> >     deferred_init_pages()
> >        __init_single_page()
> >
> > This is because memmap_init_zone() is not multi-threaded. However,
> > this work makes memmap_init_zone() multi-threaded. So, I think we
> > should really be either be using deferred_init_memmap() here, or teach
> > DEFERRED_STRUCT_PAGE_INIT to use new multi-threaded memmap_init_zone()
> > but not both.
>
> I agree it would be good to look at unifying the 2 async
> initialization approaches, however they have distinct constraints. All
> of the ZONE_DEVICE memmap initialization work happens as a hotplug
> event where the deferred_init_memmap() threads have already been torn
> down. For the memory capacities where it takes minutes to initialize
> the memmap it is painful to incur a global flush of all initialization
> work. So, I think that a move to rework deferred_init_memmap() in
> terms of memmap_init_async() is warranted because memmap_init_async()
> avoids a global sync and supports the hotplug case.
>
> Unfortunately, the work to unite these 2 mechanisms is going to be
> 4.20 material, at least for me, since I'm taking an extended leave,
> and there is little time for me to get this in shape for 4.19. I
> wouldn't be opposed to someone judiciously stealing from this set and
> taking a shot at the integration, I likely will not get back to this
> until September.

Hi Dan,

I do not want to hold your work, so if Michal or Andrew are OK with
the general approach of teaching    memmap_init_zone() to be async
without re-using deferred_init_memmap() or without changing
deferred_init_memmap() to use the new memmap_init_async() I will
review your patches.

Thank you,
Pavel

>

Powered by blists - more mailing lists