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]

Message-ID: <alpine.DEB.2.10.1612261639550.99744@chino.kir.corp.google.com>
Date:   Mon, 26 Dec 2016 16:53:39 -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 Mon, 26 Dec 2016, Michal Hocko wrote:

> But my primary argument is that if you tweak "defer" value behavior
> then you lose the only "stall free yet allow background compaction"
> option. That option is really important.

Important to who?

What regresses if we kick a background kthread to compact memory for 
order-9 pageblocks?

Why don't we allow userspace to clear __GFP_KSWAPD_RECLAIM if we don't 
want background reclaim for allocations?

> 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.

> But there are others where the administrator is not under
> the control of what application asks for yet he is responsible for the
> overal "experience" if you will.

The administrator is in charge of an "experience" and wants to avoid 
background compaction for thp allocations but not background reclaim for 
any other allocation?  (Why am I even replying to this?)  If the admin is 
concerned about anybody doing compaction, they can set defrag to "never".  
They have had this ability since thp was introduced.

> 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.

> I definitely _agree_ that this is a very important usecase! I am just
> trying to think long term and a more sophisticated background compaction
> is something that we definitely lack and _want_ longterm. There are more
> high order users than THP. I believe we really want to teach kcompactd
> to maintain configurable amount of highorder pages.
> 

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.

> If there is really a need for an immediate solution^Wworkaround then I
> think that tweaking the madvise option should be reasonably safe. Admins
> are really prepared for stalls because they are explicitly opting in for
> madvise behavior and they will get a background compaction on top. This
> is a new behavior but I do not see how it would be harmful. If an
> excessive compaction is a problem then THP can be reduced to madvise
> only vmas.
> 
> But, I really _do_ care about having a stall free option which is not a
> complete disable of the background compaction for THP.
> 

This is completely wrong.  Before the "defer" option has been introduced, 
we had "madvise" and should maintain its behavior as much as possible so 
there are no surprises.  We don't change behavior for a tunable out from 
under existing users because you think you know better.  With the new 
"defer" option, we can make this a stronger variant of "madvise", which 
Kirill acked, so that existing users of MADV_HUGEPAGE have no change in 
behavior and we can configure whether we do direct or background 
compaction for everybody else.  If people don't want background 
compaction, they can set defrag to "madvise".  If they want it, they can 
set it to "defer".  It's very simple.

That said, I simply don't have the time to continue in circular arguments 
and would respectfully ask Andrew to apply this acked patch.