Message-ID: <65bd664f-efcb-4bec-bf6f-218bc992e6de@amd.com>
Date: Tue, 18 Mar 2025 09:39:17 +0530
From: Bharata B Rao <bharata@....com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 AneeshKumar.KizhakeVeetil@....com, Hasan.Maruf@....com, Michael.Day@....com,
 akpm@...ux-foundation.org, dave.hansen@...el.com, david@...hat.com,
 feng.tang@...el.com, gourry@...rry.net, hannes@...xchg.org,
 honggyu.kim@...com, hughd@...gle.com, jhubbard@...dia.com,
 k.shutemov@...il.com, kbusch@...a.com, kmanaouil.dev@...il.com,
 leesuyeon0506@...il.com, leillc@...gle.com, liam.howlett@...cle.com,
 mgorman@...hsingularity.net, mingo@...hat.com, nadav.amit@...il.com,
 nphamcs@...il.com, peterz@...radead.org, raghavendra.kt@....com,
 riel@...riel.com, rientjes@...gle.com, rppt@...nel.org, shivankg@....com,
 shy828301@...il.com, sj@...nel.org, vbabka@...e.cz, weixugc@...gle.com,
 willy@...radead.org, ying.huang@...ux.alibaba.com, ziy@...dia.com,
 dave@...olabs.net, yuanchu@...gle.com, hyeonggon.yoo@...com
Subject: Re: [RFC PATCH 2/4] mm: kpromoted: Hot page info collection and
 promotion daemon

On 14-Mar-25 8:58 PM, Jonathan Cameron wrote:
> On Thu, 6 Mar 2025 11:15:30 +0530
> Bharata B Rao <bharata@....com> wrote:
> 
>> Subsystems that generate hot page access info can report that
>> to kpromoted via this API:
>>
>> int kpromoted_record_access(u64 pfn, int nid, int src,
>> 			    unsigned long time)
> 
> This perhaps works as an interface for aggregating methods
> that produce per access events.  Any hardware counter solution
> is going to give you data that is closer to what you used for
> the promotion decision.

Right.

> 
> We might need to aggregate at different levels.  So access
> counting promotes to a hot list and we can inject other events
> at that level.  The data I have from the CXL HMU is typically
> after an epoch (period of time) these N pages were accessed more
> than M times.  I can sort of map that to the internal storage
> you have.

Even for IBS source, I am aggregating data in per-cpu buffers before 
presenting them one by one to kpromoted. Guess CXL HMU aggregated data 
could be presented in a similar manner.

> 
> Would be good to evaluate approximate trackers on top of access
> counts. I've no idea if sketches or similar would be efficient
> enough (they have a bit of a write amplification problem) but
> they may give good answers with much lower storage cost at the
> risk of occasionally saying something is hot when it's not.

Could me point me to some information about sketches??

> 
>>
>> @pfn: The PFN of the memory accessed
>> @nid: The accessing NUMA node ID
>> @src: The temperature source (subsystem) that generated the
>>        access info
>> @time: The access time in jiffies
>>
>> Some temperature sources may not provide the nid from which
>> the page was accessed. This is true for sources that use
>> page table scanning for PTE Accessed bit. Currently the toptier
>> node to which such pages should be promoted to is hard coded.
> 
> For those cases (CXL HMU included) maybe we need to
> consider how to fill in missing node info with at least a vague chance
> of getting a reasonable target for migration.  We can always fall
> back to random top tier node, or nearest one to where we are coming
> from (on basis we maybe landed in this node based on a fallback
> list when the top tier was under memory pressure).

Yes. For A-bit scanners, Raghu has devised a scheme to obtain the best 
possible list of target nodes for promotion. He should be sharing more 
about it soon.

> 
>  From an interface point of view is that a problem for this layer,
> or for the underlying tracking mechanism? (maybe with some helpers)

It is not a problem from this interface point of view as this interface 
expects a nid(or default value) and would use that for promotion. It is 
up to the underlying tracking mechanism to provide the most appropriate 
target nid.

> Also, see later discussion of consistency of hotness tracking and
> that the best solution for that differs from that to get
> potential targets.  The answer to Is this page consistently hot?" can be
> approximated with "Was this page once hot and is it not now cold?"
> 
> Access time is something some measurement techniques will only
> give you wrt to a measurement was in a window (potentially a long
> one if you are looking for consistent hotness over minutes).
> 
>>
>> Also, the access time provided some sources may at best be
>> considered approximate. This is especially true for hot pages
>> detected by PTE A bit scanning.
>>
>> kpromoted currently maintains the hot PFN records in hash lists
>> hashed by PFN value. Each record stores the following info:
>>
>> struct page_hotness_info {
>> 	unsigned long pfn;
>>
>> 	/* Time when this record was updated last */
>> 	unsigned long last_update;
>>
>> 	/*
>> 	 * Number of times this page was accessed in the
>> 	 * current window
> I'd express here how that window was defined (I read on
> to answer the question I had here at first!)

Currently the number of accesses that occur within an observation window 
of 5s are considered for hotness calculation and access count is reset 
when the window elapses. This needs to factor in history etc.

> 
>> 	 */
>> 	int frequency;
>>
>> 	/* Most recent access time */
>> 	unsigned long recency;
> 
> Put next to the last_update so all the times are together

Sure.

> 
>>
>> 	/* Most recent access from this node */
>> 	int hot_node;
> 
> Probably want to relax the most recent part.  I'd guess
> the ideal here would be if this is the node accessing it the most
> 'recently'.

You mean the node that did most number of accesses in the given 
observation window and not necessarily the last (or most recent) 
accessed node.

> 
>>
>> 	struct hlist_node hnode;
>> };
>>
>> The way in which a page is categorized as hot enough to be
>> promoted is pretty primitive now.
> 
> That bit is very hard even if we solve everything else and heavily dependent
> on workload access pattern stability and migration impact.  Maybe for
> 'very hot' pages a fairly short consistency of hotness period is
> good enough, but it gets much messier if we care about warm pages.
> I guess we solve the 'very hot' first though and maybe avoid the phase
> transition from an application starting to when it is at steady state
> by considering a wait time for any new userspace process before we
> consider moving anything?
> 
> Also worth noting that the mechanism that makes sense to check if a
> detected hot page is 'stable hot' might use entirely different tracking
> approach to that used to find it as a candidate.
> 
> Whether that requires passing data between hotness trackers is an
> interesting question, or whether there is a natural ordering to trackers.

I was envisioning that different hotness trackers would reinforce the 
page hotness by reporting the same to kpromoted and there would be no 
need to again pass data between hotness trackers.

> 
> 
> 
>> diff --git a/mm/kpromoted.c b/mm/kpromoted.c
>> new file mode 100644
>> index 000000000000..2a8b8495b6b3
>> --- /dev/null
>> +++ b/mm/kpromoted.c
> 
>> +static int page_should_be_promoted(struct page_hotness_info *phi)
>> +{
>> +	struct page *page = pfn_to_online_page(phi->pfn);
>> +	unsigned long now = jiffies;
>> +	struct folio *folio;
>> +
>> +	if (!page || is_zone_device_page(page))
>> +		return false;
>> +
>> +	folio = page_folio(page);
>> +	if (!folio_test_lru(folio)) {
>> +		count_vm_event(KPROMOTED_MIG_NON_LRU);
>> +		return false;
>> +	}
>> +	if (folio_nid(folio) == phi->hot_node) {
>> +		count_vm_event(KPROMOTED_MIG_RIGHT_NODE);
>> +		return false;
>> +	}
>> +
>> +	/* If the page was hot a while ago, don't promote */
> 
> 	/* If the known record of hotness is old, don't promote */ ?
> 
> Otherwise this says don't move a page just because it was hot a long time
> back. Maybe it is still hot and we just don't have an update yet?

Agreed.

> 
>> +	if ((now - phi->last_update) > 2 * msecs_to_jiffies(KPROMOTED_FREQ_WINDOW)) {
>> +		count_vm_event(KPROMOTED_MIG_COLD_OLD);
>> +		return false;
>> +	}
>> +
>> +	/* If the page hasn't been accessed enough number of times, don't promote */
>> +	if (phi->frequency < KPRMOTED_FREQ_THRESHOLD) {
>> +		count_vm_event(KPROMOTED_MIG_COLD_NOT_ACCESSED);
>> +		return false;
>> +	}
>> +	return true;
>> +}
>> +
>> +/*
>> + * Go thro' page hotness information and migrate pages if required.
>> + *
>> + * Promoted pages are not longer tracked in the hot list.
>> + * Cold pages are pruned from the list as well.
> 
> When we say cold here why did we ever see them?

Those hot pages that couldn't be migrated for different reasons are no 
longer tracked by kpromoted and I called such pages as "cold". Guess not 
the right nomenclature to represent them.

> 
>> + *
>> + * TODO: Batching could be done
>> + */
>> +static void kpromoted_migrate(pg_data_t *pgdat)
>> +{
>> +	int nid = pgdat->node_id;
>> +	struct page_hotness_info *phi;
>> +	struct hlist_node *tmp;
>> +	int nr_bkts = HASH_SIZE(page_hotness_hash);
>> +	int bkt;
>> +
>> +	for (bkt = 0; bkt < nr_bkts; bkt++) {
>> +		mutex_lock(&page_hotness_lock[bkt]);
>> +		hlist_for_each_entry_safe(phi, tmp, &page_hotness_hash[bkt], hnode) {
>> +			if (phi->hot_node != nid)
>> +				continue;
>> +
>> +			if (page_should_be_promoted(phi)) {
>> +				count_vm_event(KPROMOTED_MIG_CANDIDATE);
>> +				if (!kpromote_page(phi)) {
>> +					count_vm_event(KPROMOTED_MIG_PROMOTED);
>> +					hlist_del_init(&phi->hnode);
>> +					kfree(phi);
>> +				}
>> +			} else {
>> +				/*
>> +				 * Not a suitable page or cold page, stop tracking it.
>> +				 * TODO: Identify cold pages and drive demotion?
> 
> Coldness tracking is really different from hotness as we need to track what we
> didn't see to get the really cold pages. Maybe there is some hint to be had
> from the exit of this tracker but I'd definitely not try to tackle both ends
> with one approach!

Okay.

> 
>> +				 */
>> +				count_vm_event(KPROMOTED_MIG_DROPPED);
>> +				hlist_del_init(&phi->hnode);
>> +				kfree(phi);
>> +			}
>> +		}
>> +		mutex_unlock(&page_hotness_lock[bkt]);
>> +	}
>> +}
> 
> 
>> +/*
>> + * Called by subsystems that generate page hotness/access information.
>> + *
>> + * Records the memory access info for futher action by kpromoted.
>> + */
>> +int kpromoted_record_access(u64 pfn, int nid, int src, unsigned long now)
>> +{
> 
>> +	bkt = hash_min(pfn, KPROMOTED_HASH_ORDER);
>> +	mutex_lock(&page_hotness_lock[bkt]);
>> +	phi = kpromoted_lookup(pfn, bkt, now);
>> +	if (!phi) {
>> +		ret = PTR_ERR(phi);
>> +		goto out;
>> +	}
>> +
>> +	if ((phi->last_update - now) > msecs_to_jiffies(KPROMOTED_FREQ_WINDOW)) {
>> +		/* New window */
>> +		phi->frequency = 1; /* TODO: Factor in the history */
>> +		phi->last_update = now;
>> +	} else {
>> +		phi->frequency++;
>> +	}
>> +	phi->recency = now;
>> +
>> +	/*
>> +	 * TODOs:
>> +	 * 1. Source nid is hard-coded for some temperature sources
> 
> Hard coded rather than unknown? I'm curious, what source has that issue?

I meant that source didn't provide a nid and hence kpromoted ended up 
promoting to a fixed (hard-coded for now) toptier node.

> 
>> +	 * 2. Take action if hot_node changes - may be a shared page?
>> +	 * 3. Maintain node info for every access within the window?
> 
> I guess some sort of saturating counter set might not be too bad.

Yes.

> 
>> +	 */
>> +	phi->hot_node = (nid == NUMA_NO_NODE) ? 1 : nid;
>> +	mutex_unlock(&page_hotness_lock[bkt]);
>> +out:
>> +	return 0;
> 
> why store ret and not return it?

Will fix.

Thanks for your review!

Regards,
Bharata.

Powered by blists - more mailing lists