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

[Donet Tom <donettom@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]

Hi Gregory

On 4/24/26 6:51 PM, Gregory Price wrote:
> 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;;;


Apologies for that, and thanks for pointing it out. I’ll keep the 
responses concise and include only the relevant parts going forward.

>
>>> +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

Sure.

> 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.
>
Thanks, Gregory.

I might be missing something, but since kmigrated_run() is not marked 
inline (static int kmigrated_run()), will it still be optimized or 
inlined by the compiler without the inline keyword?- Donet


>
> ~Gregory
>

[-- Attachment #2: Type: text/html, Size: 4782 bytes --]