linux-kernel - Re: [External] Re: [PATCH v18 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem

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: <CAMZfGtX37yBkKJjmBBSBeDeVAM6XywAJuEXjTSm7apOmQ-FOxA@mail.gmail.com>
Date:   Thu, 11 Mar 2021 16:45:51 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, bp@...en8.de, x86@...nel.org,
        hpa@...or.com, dave.hansen@...ux.intel.com, luto@...nel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>, paulmck@...nel.org,
        mchehab+huawei@...nel.org, pawan.kumar.gupta@...ux.intel.com,
        Randy Dunlap <rdunlap@...radead.org>, oneukum@...e.com,
        anshuman.khandual@....com, jroedel@...e.de,
        Mina Almasry <almasrymina@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Oscar Salvador <osalvador@...e.de>,
        "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>,
        David Hildenbrand <david@...hat.com>,
        HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>,
        Joao Martins <joao.m.martins@...cle.com>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        linux-doc@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Chen Huang <chenhuang5@...wei.com>,
        Bodeddula Balasubramaniam <bodeddub@...zon.com>
Subject: Re: [External] Re: [PATCH v18 1/9] mm: memory_hotplug: factor out
 bootmem core functions to bootmem_info.c

On Thu, Mar 11, 2021 at 10:58 AM Muchun Song <songmuchun@...edance.com> wrote:
>
> On Wed, Mar 10, 2021 at 10:14 PM Michal Hocko <mhocko@...e.com> wrote:
> >
> > [I am sorry for a late review]
>
> Thanks for your review.
>
> >
> > On Mon 08-03-21 18:27:59, Muchun Song wrote:
> > > Move bootmem info registration common API to individual bootmem_info.c.
> > > And we will use {get,put}_page_bootmem() to initialize the page for the
> > > vmemmap pages or free the vmemmap pages to buddy in the later patch.
> > > So move them out of CONFIG_MEMORY_HOTPLUG_SPARSE. This is just code
> > > movement without any functional change.
> > >
> > > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> > > Acked-by: Mike Kravetz <mike.kravetz@...cle.com>
> > > Reviewed-by: Oscar Salvador <osalvador@...e.de>
> > > Reviewed-by: David Hildenbrand <david@...hat.com>
> > > Reviewed-by: Miaohe Lin <linmiaohe@...wei.com>
> > > Tested-by: Chen Huang <chenhuang5@...wei.com>
> > > Tested-by: Bodeddula Balasubramaniam <bodeddub@...zon.com>
> >
> > Separation from memory_hotplug.c is definitely a right step. I am
> > wondering about the config dependency though
> > [...]
> > > diff --git a/mm/Makefile b/mm/Makefile
> > > index 72227b24a616..daabf86d7da8 100644
> > > --- a/mm/Makefile
> > > +++ b/mm/Makefile
> > > @@ -83,6 +83,7 @@ obj-$(CONFIG_SLUB) += slub.o
> > >  obj-$(CONFIG_KASAN)  += kasan/
> > >  obj-$(CONFIG_KFENCE) += kfence/
> > >  obj-$(CONFIG_FAILSLAB) += failslab.o
> > > +obj-$(CONFIG_HAVE_BOOTMEM_INFO_NODE) += bootmem_info.o
> >
> > I would have expected this would depend on CONFIG_SPARSE.
> > BOOTMEM_INFO_NODE is really an odd thing to depend on here. There is
> > some functionality which requires the node info but that can be gated
> > specifically. Or what is the thinking behind?

I have tried this. And I find that it is better to depend on
BOOTMEM_INFO_NODE instead of SPARSEMEM.

If we enable SPARSEMEM but disable HAVE_BOOTMEM_INFO_NODE,
the bootmem_info.c also is compiled. Actually, we do not
need those functions on other architectures. And these
functions are also related to bootmem info. So it may be
more reasonable to depend on BOOTMEM_INFO_NODE.
Just my thoughts.

Thanks.


>
> At first my idea was to free vmemmap pages through the bootmem
> interface. My first instinct is to rely on BOOTMEM_INFO_NODE.
> It makes sense to me to depend on CONFIG_SPARSE. I will
> update this in the next version.
>
> Thanks.
>
> >
> > This doesn't matter right now because it seems that the *_page_bootmem
> > is only used by x86 outside of the memory hotplug.
> >
> > Other than that looks good to me.
> > --
> > Michal Hocko
> > SUSE Labs