Re: [PATCH v3 13/26] mm: introduce freetype_t

[Brendan Jackman]

On Tue Aug 4, 2026 at 11:23 PM BST, Yosry Ahmed wrote:
>> @@ -179,24 +180,62 @@ static inline bool migratetype_is_mergeable(int mt)
>>  
>>  #define for_each_free_list(list, zone, order) 				\
>>  	for (order = 0; order < NR_PAGE_ORDERS; order++) 		\
>> -		for (unsigned int __type = 0; 				\
>> -		     __type < MIGRATE_TYPES &&				\
>> -			(list = &(zone)->free_area[order].free_list[__type], 1); \
>> -		     __type++)
>> +		for (unsigned int __idx = 0; 				\
>> +		     __idx < NR_FREETYPE_IDXS &&			\
>> +			(list = &(zone)->free_area[order].free_list[__idx], 1); \
>> +		     __idx++)
>> +
>> +static inline freetype_t migrate_to_freetype(enum migratetype mt,
>> +					     unsigned int flags)
>> +{
>> +	freetype_t freetype;
>> +
>> +	/* No flags supported yet. */
>> +	VM_WARN_ON_ONCE(flags);
>> +
>> +	freetype.migratetype = mt;
>> +	return freetype;
>> +}
>> +
>> +static inline enum migratetype free_to_migratetype(freetype_t freetype)
>> +{
>> +	return freetype.migratetype;
>> +}
>> +
>> +/* Convenience helper, return the freetype modified to have the migratetype. */
>> +static inline freetype_t freetype_with_migrate(freetype_t freetype,
>> +					       enum migratetype migratetype)
>> +{
>> +	return migrate_to_freetype(migratetype, freetype_flags(freetype));
>> +}
>>  
>>  extern int page_group_by_mobility_disabled;
>>  
>> +freetype_t get_pfnblock_freetype(const struct page *page, unsigned long pfn);
>> +
>>  #define get_pageblock_migratetype(page) \
>>  	get_pfnblock_migratetype(page, page_to_pfn(page))
>>  
>> +#define get_pageblock_freetype(page) \
>> +	get_pfnblock_freetype(page, page_to_pfn(page))
>> +
>>  #define folio_migratetype(folio) \
>>  	get_pageblock_migratetype(&folio->page)
>>  
>>  struct free_area {
>> -	struct list_head	free_list[MIGRATE_TYPES];
>> +	struct list_head	free_list[NR_FREETYPE_IDXS];
>>  	unsigned long		nr_free;
>>  };
>>  
>> +static inline
>> +struct list_head *free_area_list(struct free_area *area, freetype_t type)
>> +{
>> +	int idx = freetype_idx(type);
>> +
>> +	VM_WARN_ON(idx < 0);
>> +	return &area->free_list[idx];
>
> Should we return NULL here if idx < 0 instead of an out of bounds
> access?

TBH my descending order of preference is:

1.

BUG_ON(idx < 0);
return &area->free_list[idx];

2.

if (WARN_ON(idx < 0)) // OR VM_WARN_ON
  return NULL;
return &area->free_list[idx];

3.

VM_WARN_ON(idx < 0);
return &area->free_list[idx];

4. 

return &area->free_list[idx];

But I suspect[0] Vlastimil (and Linus) would order it the exact opposite
way.

[0]: https://lore.kernel.org/all/bd36e972-8900-4476-a66b-4dc218b21a4d@xxxxxxxxxx/

And I care more about making Vlastimil (and Linus) happy than this tiny
detail of the code, so I'll defer to him. The current style is a
compromise, but maybe it's just a compromise that makes nobody happy.