Re: [RFC PATCH v6 3/5] mm: Hot page tracking and promotion - pghot

[Gregory Price]

On Fri, Apr 24, 2026 at 06:27:02PM +0530, Donet Tom wrote:
> > +
> > +	/*
> > +	 * Record only accesses from lower tiers.
> > +	 */
> > +	if (node_is_toptier(pfn_to_nid(pfn)))
> > +		return 0;
> 
> 
> Just a thought—could we check this at the beginning of the function, before
> the switch case?
> 
> 

Can you please trim responses to omit un-related code? Not all of us use
mail clients that auto-collapse quoted regions and it takes a lot of
scrolling to reach here n_n;;;

> > +static void kmigrated_do_work(pg_data_t *pgdat)
> > +{
> > +	unsigned long section_nr, s_begin, start_pfn;
> > +	struct mem_section *ms;
> > +	int nid;
> > +
> > +	clear_bit(PGDAT_KMIGRATED_ACTIVATE, &pgdat->flags);
> > +	s_begin = next_present_section_nr(-1);
> > +	for_each_present_section_nr(s_begin, section_nr) {
> > +		start_pfn = section_nr_to_pfn(section_nr);
> 
> 
> I may be missing something, but in pghot_setup_hot_map() and
> kmigrated_do_work() we seem to iterate over all memory sections. On large
> memory systems, could this become a bottleneck right?
>

I think this is going to end up being a function of page / section size.

I would hesitate to over-optimize on this early-on, otherwise we'll find
ourselves re-building a bunch of DAMON functionality that chunks memory
into regions and such.

But it is a concern.

>
> Since hot_map is allocated only for lower-tier memory and the hotness
> information is primarily used there, would it make sense to skip scanning
> higher-tier sections?
> 
> for_each_online_node(nid) {
>         if (node_is_toptier(nid))
>             continue;
> 
>         start_pfn = node_start_pfn(nid);
>         end_pfn = node_end_pfn(nid);
> 
>         s_begin = pfn_to_section_nr(start_pfn);
>         for_each_present_section_nr(s_begin, section_nr) {
>     }
> }
> 
> Would this approach be reasonable, or am I overlooking something?
>

re: your note later on - kmigrated doesn't run for top tier nodes

kmigrated_run() { if (node_is_toptier(nid)) return 0; }

> > +
> > +static int kmigrated_run(int nid)
> > +{
> > +	pg_data_t *pgdat = NODE_DATA(nid);
> > +	int ret;
> > +
> > +	if (node_is_toptier(nid))
> > +		return 0;
> 
> I might be missing something, but since this function is only called from
> pghot_init(), would it make sense to check the condition before calling
> kmigrated_run() to avoid the function call overhead?
> 

this gets optimized out when static is inlined and optimized. 

~Gregory