linux-kernel - Re: [patch] mm, thp: always direct reclaim for MADV

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, 27 Dec 2016 13:36:54 -0800 (PST)
From:   David Rientjes <rientjes@...gle.com>
To:     Michal Hocko <mhocko@...nel.org>
cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Mel Gorman <mgorman@...hsingularity.net>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [patch] mm, thp: always direct reclaim for MADV_HUGEPAGE even
 when deferred

On Tue, 27 Dec 2016, Michal Hocko wrote:

> > Important to who?
> 
> To all users who want to have THP without stalls experience. This was
> the whole point of 444eb2a449ef ("mm: thp: set THP defrag by default to
> madvise and add a stall-free defrag option").
> 

THEY DO NOT STALL.  If the application is not using 
madvise(MADV_HUGEPAGE), all we do is kick kcompactd.  Nothing else.  We 
don't need any kernel tunable for an admin to override an application 
doing madvise(MADV_HUGEPAGE) when it wants hugepages and is perfectly 
happy stalling for them.

> > > You seem to think that it
> > > is the application which is under the control. And I am not all that
> > > surprised because you are under control of the whole userspace in your
> > > deployments.
> > 
> > I have no control over the userspace that runs on my "deployments," I 
> > caution you to not make any inferences.
> 
> the usecase you have described suggested otherwise. The way how you are
> using madvise sounds pretty much intentional to me. This is quite a
> different thing than running an application which uses madivise because
> it _thinks_ it is a good idea and you are left with that decision and
> cannot do anything about that.
> 

I literally cannot believe I am reading this on lkml.  I am legitimately 
stunned by this.  You are saying that the admin thinks it knows better 
than the application writer and that its madvise(MADV_HUGEPAGE) wasn't 
actually intentional?  We don't introduce tunables so that admins can 
control the intentional behavior of an application, unless that behavior 
is a security concern.  The application has specified it wants to wait for 
hugepages and has backwards compatibility with defrag=madvise settings 
since thp was introduced, which introduced MADV_HUGEPAGE.  It's the entire 
point of MADV_HUGEPAGE existing.

> > > Long stalls during the page faults are
> > > often seen as bugs and users might not really care whether the
> > > application writer really wanted THP or not...
> > > 
> > 
> > There are no long stalls during page faults introduced by this patch, we 
> > are waking up a kthread to do the work.
> 
> Yes there _are_. All madvised vmas can stall now which was not the case
> before. This is breaking the semantic of the defer option as it was
> introduced and intended (which should be pretty clear from its name).
>  

All madvised VMAs stall now because THEY WANT TO STALL.  It is 
unbelievable that you would claim otherwise or think that you know better 
than the application writer about their application.

> > We are addressing thp defrag here, not any other use for background 
> > compaction for other high-order allocations.  I'd prefer that we stay on 
> > topic, please.  This is only about setting thp defrag to "defer" and if it 
> > is possible to kick background compaction and defer direct compaction.  We 
> > need this patch, Kirill has acked it, and I simply have no more time to 
> > talk in circles.
> 
> You seem to completely ignore the review feedback and given arguments
> which is really sad...
> 

I am perfectly satisfied with Kirill's review feedback because (1) it 
makes sense and (2) it supports allowing users who do MADV_HUGEPAGE to 
actually try to get hugepages, which is the point of the madvise.

> > That said, I simply don't have the time to continue in circular arguments 
> > and would respectfully ask Andrew to apply this acked patch.
> 
> for reasons mentioned already
> Nacked-by: Michal Hocko <mhocko@...e.com>

I hope I'm not being unrealistically optimistic in assuming that this will 
be the end of this thread.  The patch should be merged.