Re: [PATCH v2 2/6] mm/page_owner: use MIGRATE_REASON_NONE instead of -1 for last_migrate_reason

[Ye Liu]

在 2026/6/27 02:45, Zi Yan 写道:
> On Thu Jun 25, 2026 at 10:45 PM EDT, Ye Liu wrote:
>> The last_migrate_reason field uses -1 as a sentinel value to mean "no
>> migration has happened".  Replace the four bare -1 occurrences with a
>> local MIGRATE_REASON_NONE define so the intent is explicit and the
>> magic number is eliminated.
>>
>> No functional change.
>>
>> Signed-off-by: Ye Liu <ye.liu@xxxxxxxxx>
>> ---
>>  mm/page_owner.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/page_owner.c b/mm/page_owner.c
>> index 342549891a8d..ebafa9d7ff07 100644
>> --- a/mm/page_owner.c
>> +++ b/mm/page_owner.c
>> @@ -21,6 +21,13 @@
>>   */
>>  #define PAGE_OWNER_STACK_DEPTH (16)
>>  
>> +/*
>> + * Used to indicate that a page has never been migrated, as the valid
>> + * migrate_reason values are non-negative enum members (MR_* in
>> + * include/linux/migrate_mode.h).
> 
> Why not change last_migrate_reason to enum migrate_reason and functions
> pass migrate reasons? MR_TYPES probably can be used to indicate no
> migration has happened if we do not want to add MR_NO_MIGRATION.
> 
> folio_set_owner_migrate_reason(),
> __folio_set_owner_migrate_reason(),
> move_hugetlb_state(), and
> __update_page_owner_handle() need to update their signatures for this.
> 
Thanks for the review.                                                        
                                                                              
Using MR_TYPES as a sentinel is problematic - it's used kernel-wide as an     
array-size marker (migrate_reason_names[MR_TYPES]). Overloading it to also    
mean "never migrated" conflates two unrelated semantics. It also has a        
subtle out-of-bounds risk: if anyone ever uses the field as an array index    
without the guard check, MR_TYPES silently points past the end of the array,  
while -1 faults immediately.                                                  
                                                                              
Adding MR_NONE = -1 to the enum is the right long-term fix, but that needs    
proper enum definition changes and cascading signature updates across multiple
headers — better done as a separate series.                                   
The current patch is a minimal improvement: it kills the magic number and     
makes intent explicit without touching any header.                            

> 
>> + */
>> +#define MIGRATE_REASON_NONE (-1)
>> +
>>  struct page_owner {
>>  	unsigned short order;
>>  	short last_migrate_reason;
>> @@ -339,7 +346,7 @@ noinline void __set_page_owner(struct page *page, unsigned short order,
>>  	depot_stack_handle_t handle;
>>  
>>  	handle = save_stack(gfp_mask);
>> -	__update_page_owner_handle(page, handle, order, gfp_mask, -1,
>> +	__update_page_owner_handle(page, handle, order, gfp_mask, MIGRATE_REASON_NONE,
>>  				   ts_nsec, current->pid, current->tgid,
>>  				   current->comm);
>>  	inc_stack_record_count(handle, gfp_mask, 1 << order);
>> @@ -596,7 +603,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>>  	if (ret >= count)
>>  		goto err;
>>  
>> -	if (page_owner->last_migrate_reason != -1) {
>> +	if (page_owner->last_migrate_reason != MIGRATE_REASON_NONE) {
>>  		ret += scnprintf(kbuf + ret, count - ret,
>>  			"Page has been migrated, last migrate reason: %s\n",
>>  			migrate_reason_names[page_owner->last_migrate_reason]);
>> @@ -667,7 +674,7 @@ void __dump_page_owner(const struct page *page)
>>  		stack_depot_print(handle);
>>  	}
>>  
>> -	if (page_owner->last_migrate_reason != -1)
>> +	if (page_owner->last_migrate_reason != MIGRATE_REASON_NONE)
>>  		pr_alert("page has been migrated, last migrate reason: %s\n",
>>  			migrate_reason_names[page_owner->last_migrate_reason]);
>>  	page_ext_put(page_ext);
>> @@ -826,7 +833,7 @@ static void init_pages_in_zone(struct zone *zone)
>>  
>>  			/* Found early allocated page */
>>  			__update_page_owner_handle(page, early_handle, 0, 0,
>> -						   -1, local_clock(), current->pid,
>> +						   MIGRATE_REASON_NONE, local_clock(), current->pid,
>>  						   current->tgid, current->comm);
>>  			count++;
>>  ext_put_continue:
> 
> 
> 
> 

-- 
Thanks,
Ye Liu