linux-kernel - Re: [External] Re: [PATCH v2 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge

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: <CAMZfGtWwHOVCvFUvm-r74k1GEEujW_HniLFOMKbykny7Cu09eA@mail.gmail.com>
Date:   Thu, 7 Jan 2021 17:01:16 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     Mike Kravetz <mike.kravetz@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [External] Re: [PATCH v2 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

On Thu, Jan 7, 2021 at 4:39 PM Michal Hocko <mhocko@...e.com> wrote:
>
> On Thu 07-01-21 11:11:41, Muchun Song wrote:
> > On Thu, Jan 7, 2021 at 1:07 AM Michal Hocko <mhocko@...e.com> wrote:
> > >
> > > On Wed 06-01-21 16:47:37, Muchun Song wrote:
> > > > When dissolve_free_huge_page() races with __free_huge_page(), we can
> > > > do a retry. Because the race window is small.
> > >
> > > Is this a bug fix or mere optimization. I have hard time to tell from
> > > the description.
> >
> > It is optimization. Thanks.
> >
> > >
> > > > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> > > > ---
> > > >  mm/hugetlb.c | 26 +++++++++++++++++++++-----
> > > >  1 file changed, 21 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > > [...]
> > > > @@ -1825,6 +1828,14 @@ int dissolve_free_huge_page(struct page *page)
> > > >       }
> > > >  out:
> > > >       spin_unlock(&hugetlb_lock);
> > > > +
> > > > +     /*
> > > > +      * If the freeing of the HugeTLB page is put on a work queue, we should
> > > > +      * flush the work before retrying.
> > > > +      */
> > > > +     if (unlikely(rc == -EAGAIN))
> > > > +             flush_work(&free_hpage_work);
> > >
> > > Is it safe to wait for the work to finish from this context?
> >
> > Yes. It is safe.
>
> Please expand on why in the changelog. Same for the optimization
> including some numbers showing it really helps.

OK. Changelog should be updated. Do you agree the race window
is quite small? If so, why is it not an optimization? Don’t we dissolve
the page as successfully as possible when we call
dissolve_free_huge_page()? I am confused about numbers showing.
Because this is not a performance optimization, but an increase in
the success rate of dissolving.

Thanks.

>
> --
> Michal Hocko
> SUSE Labs