linux-kernel - Re: [External] Re: [PATCH v6 09/16] mm/hugetlb: Defer freeing of HugeTLB pages

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:   Tue, 24 Nov 2020 20:45:30 +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>, 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>, 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>,
        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>
Subject: Re: [External] Re: [PATCH v6 09/16] mm/hugetlb: Defer freeing of
 HugeTLB pages

On Tue, Nov 24, 2020 at 7:51 PM Michal Hocko <mhocko@...e.com> wrote:
>
> On Tue 24-11-20 17:52:52, Muchun Song wrote:
> > In the subsequent patch, we will allocate the vmemmap pages when free
> > HugeTLB pages. But update_and_free_page() is called from a non-task
> > context(and hold hugetlb_lock), so we can defer the actual freeing in
> > a workqueue to prevent use GFP_ATOMIC to allocate the vmemmap pages.
>
> This has been brought up earlier without any satisfying answer. Do we
> really have bother with the freeing from the pool and reconstructing the
> vmemmap page tables? Do existing usecases really require such a dynamic
> behavior? In other words, wouldn't it be much simpler to allow to use

If someone wants to free a HugeTLB page, there is no way to do that if we
do not allow this behavior. When do we need this? On our server, we will
allocate a lot of HugeTLB pages for SPDK or virtualization. Sometimes,
we want to debug some issues and want to apt install some debug tools,
but if the host has little memory and the install operation can be failed
because of no memory. In this time, we can try to free some HugeTLB
pages to buddy in order to continue debugging. So maybe we need this.

> hugetlb pages with sparse vmemmaps only for the boot time reservations
> and never allow them to be freed back to the allocator. This is pretty
> restrictive, no question about that, but it would drop quite some code

Yeah, if we do not allow freeing the HugeTLB page to buddy, it actually
can drop some code. But I think that it only drop this one and next one
patch. It seems not a lot. And if we drop this patch, we need to add some
another code to do the boot time reservations and other code to disallow
freeing HugeTLB pages. So why not support freeing now.

> AFAICS and the resulting series would be much easier to review really
> carefully. Additional enhancements can be done on top with specifics
> about usecases which require more flexibility.

The code of allocating vmemmap pages for the HugeTLB page is very
similar to the freeing vmemmap pages. The two operations are opposite.
I think that if someone can understand the freeing path, it is also easy
for him to understand the allcating path. If you look at close to this patch,
I believe that it is easy for you.

>
> > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> > ---
> >  mm/hugetlb.c         | 96 ++++++++++++++++++++++++++++++++++++++++++++++------
> >  mm/hugetlb_vmemmap.c |  5 ---
> >  mm/hugetlb_vmemmap.h | 10 ++++++
> >  3 files changed, 95 insertions(+), 16 deletions(-)
> --
> Michal Hocko
> SUSE Labs



-- 
Yours,
Muchun