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, 21 Apr 2023 17:03:20 +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 12/26] mm: page_alloc: per-migratetype free counts
Message-ID: <20230421160320.zz2he2iprq37vnl3@techsingularity.net>
References: <20230418191313.268131-1-hannes@cmpxchg.org>
 <20230418191313.268131-13-hannes@cmpxchg.org>
 <20230421142841.parju3gmqmpefigq@techsingularity.net>
 <20230421153501.GE320347@cmpxchg.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <20230421153501.GE320347@cmpxchg.org>
Precedence: bulk

On Fri, Apr 21, 2023 at 11:35:01AM -0400, Johannes Weiner wrote:
> On Fri, Apr 21, 2023 at 03:28:41PM +0100, Mel Gorman wrote:
> > On Tue, Apr 18, 2023 at 03:12:59PM -0400, Johannes Weiner wrote:
> > > Increase visibility into the defragmentation behavior by tracking and
> > > reporting per-migratetype free counters.
> > > 
> > > Subsequent patches will also use those counters to make more targeted
> > > reclaim/compaction decisions.
> > > 
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > 
> > Visibility into fragmentation behaviour is information that is
> > almost certainly only useful to a developer and even then, there is
> > /proc/pagetypeinfo. At minimum, move this patch to later in the series
> > but I'm skeptical about its benefit.
> 
> Having them available in the memory dump (OOM, sysrq) was essential
> while debugging problems in later patches. For OOMs or lockups,
> pagetypeinfo isn't available. It would be useful to have them included
> in user reports if any issues pop up.
> 

OOM+sysrq could optionally take the very expensive step of traversing the
lists to get the count so yes, it helps debugging, but not necessarily
critical.

> They're used internally in several places later on, too.
> 

I did see that for deciding the suitability for compaction. Minimally, put
the patches adjacent in the series and later if possible so that the series
can be taken in parts. There are a lot of patches that should be relatively
uncontroversial so maybe make "mm: page_alloc: introduce MIGRATE_FREE" the
pivot point between incremental improvements and "everything on and after
this patch is relatively high risk, could excessively compact/reclaim,
could livelock etc".

-- 
Mel Gorman
SUSE Labs