Re: [RFC PATCH 1/5] misc: introduce FDBox

[Pratyush Yadav <ptyadav@amazon.de>]

On Thu, Mar 20 2025, Jason Gunthorpe wrote:

> On Wed, Mar 19, 2025 at 01:35:31PM +0000, Pratyush Yadav wrote:
>> On Tue, Mar 18 2025, Jason Gunthorpe wrote:
>> 
>> > On Tue, Mar 18, 2025 at 11:02:31PM +0000, Pratyush Yadav wrote:
>> >
>> >> I suppose we can serialize all FDs when the box is sealed and get rid of
>> >> the struct file. If kexec fails, userspace can unseal the box, and FDs
>> >> will be deserialized into a new struct file. This way, the behaviour
>> >> from userspace perspective also stays the same regardless of whether
>> >> kexec went through or not. This also helps tie FDBox closer to KHO.
>> >
>> > I don't think we can do a proper de-serialization without going
>> > through kexec. The new stuff Mike is posting for preserving memory
>> > will not work like that.
>> 
>> Why not? If the next kernel can restore the file from the serialized
>> content, so can the current kernel. What stops this from working with
>> the new memory preservation scheme (which I assume is the idea you
>> proposed in [0])? 
>
> It is because the current kernel does not destroy the struct page
> before the kexec and the new kernel assumes a zero'd fresh struct page
> at restore.
>
> So it would be very easy to corrupt the struct page information if you
> attempt to deserialize without going through the kexec step.
>
> There would be a big risk of getting things like refcounts out of
> sync.

Ideally, kho_preserve_folio() should be similar to freeing the folio,
except that it doesn't go to buddy for re-allocation. In that case,
re-using those pages should not be a problem as long as the driver made
sure the page was properly "freed", and there are no stale references to
it. They should be doing that anyway since they should make sure the
file doesn't change after it has been serialized.

Doing that might be easier said than done though. On a quick look, most
of the clearing of struct page seems to be happening in
free_pages_prepare(). This is usually followed by free_one_page(), which
gives the page back to buddy. Though I am not sure how much sense it
would make to use free_pages_prepare() outside of page free path. I need
to look deeper...

>
> Then you have the issue that I don't actually imagine shutting down
> something like iommufd, I was intending to leave it frozen in place
> with all its allocations and so on. If you try to de-serialize you
> can't de-serialize into the thing that is frozen, you'd create a new
> one from empty. Now you have two things pointing at the same stuff,
> what a mess.

What do you mean by "frozen in place"? Isn't that the same as being
serialized? Considering that we want to make sure a file is not opened
by any process before we serialize it, what do we get by keeping the
struct file around (assuming we can safely deserialize it without going
through kexec)?

>
>> The seal operation does bulk serialize/deserialize for _one_ box. You
>> can have multiple boxes and distribute your FDs in the boxes based on
>> the serialize or deserialize order you want. Userspace decides when to
>> seal or unseal a particular box, which gives it full control over the
>> order in which things happen.
>
> Why have more than one box? What is the point? I've been thinking we
> should just have a KHO control char dev FD for serializing and you can
> do all the operations people have been talking about in sysfs, as well
> as record FDs for serializing.

Main idea is for logical grouping and dependency management. If some FDs
have a dependency between them, grouping them in different boxes makes
it easy to let userspace choose the order of operations, but still have
a way to make sure all dependencies are met when the FDs are serialized.
Similarly, on the deserialize side, this ensures that all dependent FDs
are deserialized together.

[...]

-- 
Regards,
Pratyush Yadav