Re: [RFC PATCH v0 2/2] mm: sched: Batch-migrate misplaced pages

[Gregory Price <gourry@gourry.net>]

On Wed, May 21, 2025 at 02:40:53PM -0400, Zi Yan wrote:
> On 21 May 2025, at 14:25, Donet Tom wrote:
> > Hi Bharatha,
> >
> > This is target node ID right?
> 
> In memory tiering mode, folio_last_cpupid() gives page access time
> for slow memory folios. In !folio_use_access_time() case,
> folio_last_cpupid() gives last cpupid. Now it is reused for node
> id. It is too confusing. At least, a new function like folio_get_target_nid()
> should be added to return a nid only if folio is isolated.
>

The really annoying part of all of this is

#ifdef CONFIG_NUMA_BALANCING
#ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
static inline int folio_last_cpupid(struct folio *folio)
{
        return folio->_last_cpupid;
}
#else
static inline int folio_last_cpupid(struct folio *folio)
{
        return (folio->flags >> LAST_CPUPID_PGSHIFT) & LAST_CPUPID_MASK;
}
#endif
#else /* !CONFIG_NUMA_BALANCING */
static inline int folio_last_cpupid(struct folio *folio)
{
        return folio_nid(folio); /* XXX */
}
...
#endif

Obviously we don't have to care about the !NUMAB case, but what a silly
muxing we have going on here (I get it, space is tight - the interfaces
are just confusing is all).

My question is whether there's some kind of race condition here if the
mode changes between isolate and fetch.  Can we "fetch a node id" and
end up with a cpupid because someone toggled the between tiering and
balancing?

If we can answer that, then implementing folio_last_cpupid and
folio_last_access_nid can return -1 if called in the wrong mode
(assuming this check isn't too expensive).  That would be a nice cleanup
for readability sake.

~Gregory