Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 28 Apr 2023 11:41:51 +0100
From:   Mel Gorman <mgorman@techsingularity.net>
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     linux-mm@kvack.org, Kaiyang Zhao <kaiyang2@cs.cmu.edu>,
        Vlastimil Babka <vbabka@suse.cz>,
        David Rientjes <rientjes@google.com>,
        linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [RFC PATCH 10/26] mm: page_alloc: allow compaction capturing
 from larger blocks
Message-ID: <20230428104151.tau2mru65rdsisyg@techsingularity.net>
References: <20230418191313.268131-1-hannes@cmpxchg.org>
 <20230418191313.268131-11-hannes@cmpxchg.org>
 <20230421141447.2cw5cfwibb7jxf6n@techsingularity.net>
 <20230425154026.GC17132@cmpxchg.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <20230425154026.GC17132@cmpxchg.org>
Precedence: bulk

On Tue, Apr 25, 2023 at 11:40:26AM -0400, Johannes Weiner wrote:
> On Fri, Apr 21, 2023 at 03:14:47PM +0100, Mel Gorman wrote:
> > On Tue, Apr 18, 2023 at 03:12:57PM -0400, Johannes Weiner wrote:
> > > Currently, capturing only works on matching orders and matching
> > > migratetypes. However, if capturing is initially skipped on the
> > > migratetype, it's possible that merging continues up to a full
> > > pageblock, in which case the migratetype is up for grabs again.
> > > 
> > > Allow capturing to grab smaller chunks from claimed pageblocks, and
> > > expand the remainder of the block back onto the freelists.
> > > 
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > 
> > No objections other than we're still in the preparation phase and the
> > series needs to be split. Out of curiousity, how often does this actually
> > trigger in practice? I ask because superficially, I would expect capture to
> > happen while pages are being merged and I'm not sure how much this actually
> > helps. If anything the anomaly would be merging !MOVABLE types, capturing
> > one pageblock and leaving the adjacent block eligible for splitting as
> > UNMOVABLE/RECLAIMABLE which is not necessarily desirable.
> 
> Looking at this patch independently, once merging continues to the
> full block, a fallback would be allowed to claim it anyway
> (can_steal_fallback() returns true). I don't quite see a downside
> letting capture apply in this case. The plus is of course avoiding the
> indirection through the freelist which risks an opportunist request of
> a smaller order fragmenting the block and wasting the contiguity work.
> 
> In the context of the full series, this becomes even more
> important. Once watermarks are required to be met in MIGRATE_FREE
> blocks, and reclaim/compaction recycle full blocks, merging up to
> pageblock_order happens all the time - and needs to happen for
> allocations to succeed. This applies to all types of direct reclaim:
> unmovable request freeing reclaimable/movable blocks, reclaimable
> freeing movable blocks, movable freeing reclaimable blocks.
> 
> I see your point about smaller orders now always ending the merge at
> the pageblock, even when there could be additional merging
> opportunities beyond. However, I'm not sure these accidental larger
> merges beyond what's needed to fulfill the request at hand are a
> preferable aspect over reclaimer fairness, and thus ultimately the
> reliability of orders up to the pageblock size.
> 
> I'll try to get some numbers for this patch independently, though.
> This should manifest in p99 allocation latencies and near-OOM
> behavior. Is there anything else you'd want me to look for?
> 

Any major change in the number of the mm_page_alloc_extfrag trace event
triggering. Also put the patch at the end of the preparation series of
possible or even do it as a separate follow-up patch after the bulk of
the series has been handled. While I cannot 100% convince myself either
way, I wonder if variable high-order allocation requests smaller than a
pageblock could cause premature mixing due to capture.

-- 
Mel Gorman
SUSE Labs