linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jonathan Corbet <corbet@lwn.net>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Mel Gorman <mgorman@techsingularity.net>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, thp: always direct reclaim for MADV_HUGEPAGE even when deferred
Date: Tue, 27 Dec 2016 13:36:54 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.10.1612271324300.67790@chino.kir.corp.google.com> (raw)
In-Reply-To: <20161227094008.GC1308@dhcp22.suse.cz>

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2016-12-27 21:36 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-22  0:21 David Rientjes
2016-12-22  8:31 ` Kirill A. Shutemov
2016-12-22 10:00 ` Michal Hocko
2016-12-22 21:05   ` David Rientjes
2016-12-23  8:51     ` Michal Hocko
2016-12-23 10:01       ` David Rientjes
2016-12-23 11:18         ` Michal Hocko
2016-12-23 22:46           ` David Rientjes
2016-12-26  9:02             ` Michal Hocko
2016-12-27  0:53               ` David Rientjes
2016-12-27  2:32                 ` Kirill A. Shutemov
2016-12-27  9:41                 ` Michal Hocko
2016-12-27 21:36                   ` David Rientjes [this message]
2016-12-28  8:48                     ` Michal Hocko
2016-12-28 21:33                       ` David Rientjes
2016-12-29  8:24                         ` Michal Hocko
2016-12-30 12:36     ` Mel Gorman
2016-12-30 12:56       ` Michal Hocko
2016-12-30 14:08         ` Mel Gorman
2016-12-30 22:30       ` David Rientjes
2017-01-03 10:37         ` Mel Gorman
2017-01-03 21:57           ` David Rientjes
2017-01-04 10:12             ` Mel Gorman
2017-01-04 21:53               ` David Rientjes
2017-01-02  8:38 ` Vlastimil Babka
2017-01-03 22:44   ` David Rientjes
2017-01-04  8:32     ` Vlastimil Babka
2017-01-04  9:46       ` Michal Hocko
2017-01-04 22:04       ` David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1612271324300.67790@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox