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: <20210107114806.GI13207@dhcp22.suse.cz>
Date:   Thu, 7 Jan 2021 12:48:06 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     Muchun Song <songmuchun@...edance.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>,
        Yang Shi <shy828301@...il.com>
Subject: Re: [External] Re: [PATCH v2 1/6] mm: migrate: do not migrate
 HugeTLB page whose refcount is one

On Thu 07-01-21 19:24:59, Muchun Song wrote:
> On Thu, Jan 7, 2021 at 7:16 PM Michal Hocko <mhocko@...e.com> wrote:
> >
> > On Thu 07-01-21 10:52:21, Muchun Song wrote:
> > > On Thu, Jan 7, 2021 at 12:13 AM Michal Hocko <mhocko@...e.com> wrote:
> > > >
> > > > On Wed 06-01-21 16:47:34, Muchun Song wrote:
> > > > > If the refcount is one when it is migrated, it means that the page
> > > > > was freed from under us. So we are done and do not need to migrate.
> > > >
> > > > Is this common enough that it would warrant the explicit check for each
> > > > migration?
> > >
> > > Are you worried about the overhead caused by the check? Thanks.
> >
> > I am not as worried as I would like to see some actual justification for
> > this optimization.
> 
> OK. The HugeTLB page can be freed after it was isolated but before
> being migrated. Right? If we catch this case, it is an optimization.
> I do this just like unmap_and_move() does for a base page.

If your only justification is that this path to be consistent with the
regular pages then make it explicit in the changelog. Please think of
people reading this code in the future and scratching their heads
whether this is something that can be altered and fail to find the
reasoning. Was this a huge optimization and some workload might suffer?
Can this happen in the first place or how likely it is?

-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ