Re: [PATCH 1/3] mm/memfd_luo: don't use memfd_pin_folios()

[Pratyush Yadav <pratyush@kernel.org>]

On Wed, Oct 28 2026, Pasha Tatashin wrote:

>> memfd_luo takes a snapshot of the folios in the file's mapping. To make
>> sure the snapshot does not go out of sync, it pins the folios in the
>> file via memfd_pin_folios(). The pinning also migrates the folios out of
>> MIGRATE_MOVABLE to make sure doesn't land in KHO boot memory or in a CMA
>> region.
>> 
>> memfd_pin_folios() does not lock the folios. So it can race with
>> shmem_writeout(). A folio returned by memfd_pin_folios() might already
>> be in swap cache. This happens to work but is fragile. memfd_luo looks
>> at folio flags and index on a folio that isn't in the memfd's page
>> cache.
>> 
>> In addition, memfd_pin_folios() needs the caller to allocate an array of
>> folios. memfd_luo does not really need that since it will vmalloc
>> folios_ser and store the information there anyway. The folios array is
>> unnecessary overhead.
>> 
>> Finally, memfd_pin_folios() fills in holes in the memfd. This makes
>> preserving sparse memfds very expensive in both compute and memory.
>> While this patch doesn't yet stop filling in the holes, it sets up the
>> infrastructure to make it simpler.
>
> +1, this is problematic and should be resolved. In some setups, VMMs 
> leave holes for PCI/IO space; with the current approach, single memfd 
> preservation does not work for such use cases.

Yep, agreed. There are a couple parts to this. First is this patch. Next
is KHO xarray. The patchset is ready, I'll send it after -rc1.

I think I will move this patch to that series and then make the series
also support holes.

>
>> Fix all this by walking the page cache directly using shmem_get_folio()
>> with SGP_CACHE. This returns a locked folio with an elevated refcount.
>> memfd_luo holds that reference until the memfd is unpreserved. This
>> blocks compaction, migration, etc.
>> 
>> There is still a need to migrate folios out of MIGRATE_MOVABLE page
>> blocks. This is because those might be CMA or KHO scratch. memfd_luo
>> will hold a longterm reference to the folios so it will block migration
>> in that pageblock. Introduce memfd_luo_migrate_folio() to do this.
>> 
>> Also factor out folio unpreservation logic into memfd_luo_put_folios()
>> and use it from memfd_luo_unpreserve_folios() and from error paths in
>> memfd_luo_preserve_folios().
>> 
>> Since shmem_get_folio() always returns uptodate folios, drop the folio
>> zeroing logic.
>> 
>> This change also improves preservation performance, shown in the table
>> below.
>> 
>> |------------------+----------+------------+-------|
>> | memfd            | baseline | this patch | delta |
>> |------------------+----------+------------+-------|
>> | 4K pages, 32 GiB | 748 ms   | 600 ms     |  -20% |
>> | THP, 4 GiB       | 0.59 ms  | 0.59 ms    |    0% |
>> | THP, 16 GiB      | 2.99 ms  | 2.61 ms    |  -13% |
>> | THP, 32 GiB      | 4.83 ms  | 5.21 ms    |   +8% |
>> | sparse, 32 GiB   | 8695 ms  | 7269 ms    |  -16% |
>> |------------------+----------+------------+-------|
>
> For THP 16 GiB / 32 GiB, the -13% / +8% swings look like high run-to-run 
> variance. However, maybe not: is it because vmalloc_array() is 
> overallocating memory for folios_ser?

Could be, I only did a handful of runs. But I also reckon this is
vmalloc_array() over-allocating. It over-allocated before this patch
too, for the folios array, but that allocation was half as big since
folios_ser is 16 bytes each.

>
> Also, could you clarify what "sparse, 32 GiB" means in this benchmark? 
> Was the memfd completely unfaulted before preserve (so preserve faults 
> in all 4K pages), or partially populated?

It was completely unfaulted. Size was set via ftruncate() and then
preserved. So it is the worse case of "sparse".

[...]
>> @@ -113,99 +213,77 @@ static int memfd_luo_preserve_folios(struct file *file,
>>  	 * Guess the number of folios based on inode size. Real number might end
>>  	 * up being smaller if there are higher order folios.
>>  	 */
>> -	max_folios = PAGE_ALIGN(size) / PAGE_SIZE;
>> -	folios = kvmalloc_objs(*folios, max_folios);
>> -	if (!folios)
>> +	end = DIV_ROUND_UP_ULL(size, PAGE_SIZE);
>> +	max_folios = end;
>> +	folios_ser = vmalloc_array(max_folios, sizeof(*folios_ser));
>> +	if (!folios_ser)
>>  		return -ENOMEM;
>
> I bet in practice memfds almost always contain folios of the same size, 
> so why not estimate the folios_ser size based on the first folio's 
> order, and only if that ends up being too small, vrealloc() to match the 
> maximum possible remaining size?

That's a good idea! But I think the switch to KHO Xarray will remove the
need for this.

>
> It would be a different change though.
>
[...]
>> +
>> +		if (unlikely(!folio_is_longterm_pinnable(folio))) {
>> +			folio_unlock(folio);
>> +			err = memfd_luo_migrate_folio(folio);
>> +			/* Someone else might have it isolated, retry a bit. */
>> +			if (err == -EBUSY && ++busy_retries < MEMFD_LUO_MIGRATE_RETRIES)
>> +				continue;
>> +			if (err)
>> +				goto err_restore;
>> +			continue;
>>  		}
>
> Nit:
>
> I would suggest adding another helper, memfd_luo_get_folio(inode, index, 
> &folio), that wraps:
>
> - shmem_get_folio()
> - The !folio_is_longterm_pinnable() migration retry loop logic
>
> I think this would make the final code a bit more manageable.

Sounds good, will do.

>
> Other than that the patch looks alright, feel free to add
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> Once the nit is addressed.

Thanks!

I think the MM side might take some time to gather reviews so I will
send this out as part of a liveupdate-only series. This patch is useful
on its own anyway.

-- 
Regards,
Pratyush Yadav