Re: [PATCH v3 06/10] mm/shmem_luo: Don't make early retrieve failures permanent

[Pratyush Yadav <pratyush@kernel.org>]

On Wed, Nov 04 2026, David Matlack wrote:

> On 2026-11-04 04:01 PM, Pratyush Yadav wrote:
>> On Tue, Nov 03 2026, David Matlack wrote:
>> 
>> > Mark failures in shmem_luo_retrieve() before it starts restoring the
>> > folios as retryable, and return the error without freeing the serialized
>> > state. If no retrieve succeeds, finish() frees the preserved folios,
>> > like it does for a file that was never retrieved.
>> >
>> > Nothing has consumed the preserved state before the folios are
>> > restored, so the retrieve can be tried again.
>> >
>> > This also fixes leaking the preserved folios when one of these early
>> > steps fails, e.g. because the file has seals that this kernel doesn't
>> > support. The retrieve freed the serialized state but not the folios, and
>> > finish() then skipped the file because its retrieve had failed. There is
>> > no Fixes tag, since the fix relies on args->retryable, which was only
>> > added earlier in this series.
>> 
>> If the seals don't match, the retrieve is not retryable. It will always
>> fail.
>> 
>> I think the only retriable part is memfd_alloc_file(), although even
>> that is quite unlikely to fail. But I think that also fits pretty well
>> with the most important type of error you want to retry on, which is bad
>> arguments. In later patches, you check the argument on
>> shmem_luo_create_tmpfs_file() or shmem_luo_create_memfd(), so perhaps
>> set retriable in those functions?
>
> When retryable is set, LUO leaves the file unretrieved and finish()
> frees the preserved folios. If the seal failure did not set retryable,
> finish() would skip the file and the folios would leak.

Yes, but in that case you free the folios on the error path of
retrieve().

>
> In v4 I can move the seal checks into shmem_luo_create_memfd() and
> shmem_luo_create_tmpfs_file(), since the valid seals differ between the
> two, and reword the commit message so it doesn't suggest that a bad seal
> can be fixed by retrying.

I think it is important to signal to _userspace_ too that bad seals
aren't retriable either. So I would really suggest doing some sort of
non-retriable errors for bad payloads.

-- 
Regards,
Pratyush Yadav