Re: [PATCH 3/4] mm: page_alloc: move capture_control to the page allocator

[Vlastimil Babka (SUSE)]

On 7/9/26 19:45, Johannes Weiner wrote:
> On Thu, Jul 02, 2026 at 12:20:48PM +0200, Vlastimil Babka (SUSE) wrote:
>> Now my version (only compile tested):
> 
> Ah, now I get it. Thanks.
> 
>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
>> index f29ef0653546..66a2f70e9e01 100644
>> --- a/include/linux/compaction.h
>> +++ b/include/linux/compaction.h
>> @@ -58,6 +58,7 @@ enum compact_result {
>>  };
>>  
>>  struct alloc_context; /* in mm/internal.h */
>> +struct capture_control; /* in mm/internal.h */
>>  
>>  /*
>>   * Number of free order-0 pages that should be available above given watermark
>> @@ -92,7 +93,7 @@ extern int fragmentation_index(struct zone *zone, unsigned int order);
>>  extern enum compact_result try_to_compact_pages(gfp_t gfp_mask,
>>  		unsigned int order, unsigned int alloc_flags,
>>  		const struct alloc_context *ac, enum compact_priority prio,
>> -		struct page **page);
>> +		struct capture_control *capc);
>>  extern void reset_isolation_suitable(pg_data_t *pgdat);
>>  extern bool compaction_suitable(struct zone *zone, int order,
>>  				unsigned long watermark, int highest_zoneidx);
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index b7878e4a2c4b..902573797f99 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -2792,7 +2792,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
>>  static enum compact_result compact_zone_order(struct zone *zone, int order,
>>  		gfp_t gfp_mask, enum compact_priority prio,
>>  		unsigned int alloc_flags, int highest_zoneidx,
>> -		struct page **capture)
>> +		struct capture_control *capc)
>>  {
>>  	enum compact_result ret;
>>  	struct compact_control cc = {
>> @@ -2809,36 +2809,8 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
>>  		.ignore_skip_hint = (prio == MIN_COMPACT_PRIORITY),
>>  		.ignore_block_suitable = (prio == MIN_COMPACT_PRIORITY)
>>  	};
>> -	struct capture_control capc = {
>> -		.cc = &cc,
>> -		.page = NULL,
>> -	};
>> -
>> -	/*
>> -	 * Make sure the structs are really initialized before we expose the
>> -	 * capture control, in case we are interrupted and the interrupt handler
>> -	 * frees a page.
>> -	 */
>> -	barrier();
>> -	WRITE_ONCE(current->capture_control, &capc);
>>  
>> -	ret = compact_zone(&cc, &capc);
>> -
>> -	/*
>> -	 * Make sure we hide capture control first before we read the captured
>> -	 * page pointer, otherwise an interrupt could free and capture a page
>> -	 * and we would leak it.
>> -	 */
>> -	WRITE_ONCE(current->capture_control, NULL);
>> -	*capture = READ_ONCE(capc.page);
>> -	/*
>> -	 * Technically, it is also possible that compaction is skipped but
>> -	 * the page is still captured out of luck(IRQ came and freed the page).
>> -	 * Returning COMPACT_SUCCESS in such cases helps in properly accounting
>> -	 * the COMPACT[STALL|FAIL] when compaction is skipped.
>> -	 */
>> -	if (*capture)
>> -		ret = COMPACT_SUCCESS;
>> +	ret = compact_zone(&cc, capc);
> 
> &capc, and I switched it to just return compact_zone(&cc, &capc);
> 
> But otherwise it looks clean to me. I'd just take this as-is with your
> From: if you don't mind. Can I add your S-O-B?

Sure, but I don't remember if I started from scratch or your version. At
least add a co-developed-by yourself or something :)

Signed-off-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>