Message-ID: <20161223085150.GA23109@dhcp22.suse.cz>
Date:   Fri, 23 Dec 2016 09:51:50 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     David Rientjes <rientjes@...gle.com>
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 Thu 22-12-16 13:05:27, David Rientjes wrote:
> On Thu, 22 Dec 2016, Michal Hocko wrote:
> 
> > > Currently, when defrag is set to "madvise", thp allocations will direct
> > > reclaim.  However, when defrag is set to "defer", all thp allocations do
> > > not attempt reclaim regardless of MADV_HUGEPAGE.
> > > 
> > > This patch always directly reclaims for MADV_HUGEPAGE regions when defrag
> > > is not set to "never."  The idea is that MADV_HUGEPAGE regions really
> > > want to be backed by hugepages and are willing to endure the latency at
> > > fault as it was the default behavior prior to commit 444eb2a449ef ("mm:
> > > thp: set THP defrag by default to madvise and add a stall-free defrag
> > > option").
> > 
> > AFAIR "defer" is implemented exactly as intended. To offer a never-stall
> > but allow to form THP in the background option. The patch description
> > doesn't explain why this is not good anymore. Could you give us more
> > details about the motivation and why "madvise" doesn't work for
> > you? This is a user visible change so the reason should better be really
> > documented and strong.
> > 
> 
> The offering of defer breaks backwards compatibility with previous 
> settings of defrag=madvise, where we could set madvise(MADV_HUGEPAGE) on 
> .text segment remap and try to force thp backing if available but not 
> directly reclaim for non VM_HUGEPAGE vmas.

I do not understand the backwards compatibility issue part here. Maybe I
am missing something but the semantic of defrag=madvise hasn't changed
and a new flag can hardly break backward compatibility.

> This was very advantageous.  
> We prefer that to stay unchanged and allow kcompactd compaction to be 
> triggered in background by everybody else as opposed to direct reclaim.  
> We do not have that ability without this patch.

So why don't you use defrag=madvise?

> Without this patch, we will be forced to offer multiple sysfs tunables to 
> define (1) direct vs background compact, (2) madvise behavior, (3) always, 
> (4) never and we cannot have 2^4 settings for "defrag" alone.

I disagree. I think the current set of defrag values should be
sufficient. We can completely disable direct reclaim, enable it only for
opt-in, enable for all and never allow to stall. The advantage of this
set of values is that they have _clear_ semantic and behave
consistently. If you change defer to "almost never stall except when
MADV_HUGEPAGE" then the semantic is less clear. Admin might have a good
reason to never allow stalls - especially when he doesn't have a control
over the code he is running. Your patch would break this usecase.

If we want to provide a better background THP availability we should
focus more on kcompactd and the way how it is invoked. Currently we only
wake it up during the page allocation path. Long term we want to make it
more watermak based I believe. Similar to kswapd. Vlastimil is already
playing with this idea. I would prefer such a long term plan more than
tweaking THP configuration.

-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists