[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aRsBHy5aQ_Ypyy9r@kernel.org>
Date: Mon, 17 Nov 2025 13:03:59 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com,
dmatlack@...gle.com, rientjes@...gle.com, corbet@....net,
rdunlap@...radead.org, ilpo.jarvinen@...ux.intel.com,
kanie@...ux.alibaba.com, ojeda@...nel.org, aliceryhl@...gle.com,
masahiroy@...nel.org, akpm@...ux-foundation.org, tj@...nel.org,
yoann.congal@...le.fr, mmaurer@...gle.com, roman.gushchin@...ux.dev,
chenridong@...wei.com, axboe@...nel.dk, mark.rutland@....com,
jannh@...gle.com, vincent.guittot@...aro.org, hannes@...xchg.org,
dan.j.williams@...el.com, david@...hat.com,
joel.granados@...nel.org, rostedt@...dmis.org,
anna.schumaker@...cle.com, song@...nel.org, linux@...ssschuh.net,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-mm@...ck.org, gregkh@...uxfoundation.org, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, hpa@...or.com, rafael@...nel.org, dakr@...nel.org,
bartosz.golaszewski@...aro.org, cw00.choi@...sung.com,
myungjoo.ham@...sung.com, yesanishhere@...il.com,
Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com,
aleksander.lobakin@...el.com, ira.weiny@...el.com,
andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de,
bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com,
stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net,
brauner@...nel.org, linux-api@...r.kernel.org,
linux-fsdevel@...r.kernel.org, saeedm@...dia.com,
ajayachandra@...dia.com, jgg@...dia.com, parav@...dia.com,
leonro@...dia.com, witu@...dia.com, hughd@...gle.com,
skhawaja@...gle.com, chrisl@...nel.org
Subject: Re: [PATCH v6 15/20] mm: memfd_luo: allow preserving memfd
On Sat, Nov 15, 2025 at 06:34:01PM -0500, Pasha Tatashin wrote:
> From: Pratyush Yadav <ptyadav@...zon.de>
>
> The ability to preserve a memfd allows userspace to use KHO and LUO to
> transfer its memory contents to the next kernel. This is useful in many
> ways. For one, it can be used with IOMMUFD as the backing store for
> IOMMU page tables. Preserving IOMMUFD is essential for performing a
> hypervisor live update with passthrough devices. memfd support provides
> the first building block for making that possible.
>
> For another, applications with a large amount of memory that takes time
> to reconstruct, reboots to consume kernel upgrades can be very
> expensive. memfd with LUO gives those applications reboot-persistent
> memory that they can use to quickly save and reconstruct that state.
>
> While memfd is backed by either hugetlbfs or shmem, currently only
> support on shmem is added. To be more precise, support for anonymous
> shmem files is added.
>
> The handover to the next kernel is not transparent. All the properties
> of the file are not preserved; only its memory contents, position, and
> size. The recreated file gets the UID and GID of the task doing the
> restore, and the task's cgroup gets charged with the memory.
>
> Once preserved, the file cannot grow or shrink, and all its pages are
> pinned to avoid migrations and swapping. The file can still be read from
> or written to.
>
> Use vmalloc to get the buffer to hold the folios, and preserve
> it using kho_preserve_vmalloc(). This doesn't have the size limit.
>
> Co-developed-by: Pasha Tatashin <pasha.tatashin@...een.com>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
> Signed-off-by: Pratyush Yadav <ptyadav@...zon.de>
The order of signed-offs seems wrong, Pasha's should be the last one.
> ---
...
> +/**
> + * DOC: memfd Live Update ABI
> + *
> + * This header defines the ABI for preserving the state of a memfd across a
> + * kexec reboot using the LUO.
> + *
> + * The state is serialized into a Flattened Device Tree which is then handed
> + * over to the next kernel via the KHO mechanism. The FDT is passed as the
> + * opaque `data` handle in the file handler callbacks.
> + *
> + * This interface is a contract. Any modification to the FDT structure,
> + * node properties, compatible string, or the layout of the serialization
> + * structures defined here constitutes a breaking change. Such changes require
> + * incrementing the version number in the MEMFD_LUO_FH_COMPATIBLE string.
The same comment about contract as for the generic LUO documentation
applies here (https://lore.kernel.org/all/aRnG8wDSSAtkEI_z@kernel.org/)
> + *
> + * FDT Structure Overview:
> + * The memfd state is contained within a single FDT with the following layout:
...
> +static struct memfd_luo_folio_ser *memfd_luo_preserve_folios(struct file *file, void *fdt,
> + u64 *nr_foliosp)
> +{
If we are already returning nr_folios by reference, we might do it for
memfd_luo_folio_ser as well and make the function return int.
> + struct inode *inode = file_inode(file);
> + struct memfd_luo_folio_ser *pfolios;
> + struct kho_vmalloc *kho_vmalloc;
> + unsigned int max_folios;
> + long i, size, nr_pinned;
> + struct folio **folios;
pfolios and folios read like the former is a pointer to latter.
I'd s/pfolios/folios_ser/
> + int err = -EINVAL;
> + pgoff_t offset;
> + u64 nr_folios;
...
> + kvfree(folios);
> + *nr_foliosp = nr_folios;
> + return pfolios;
> +
> +err_unpreserve:
> + i--;
> + for (; i >= 0; i--)
Maybe a single line
for (--i; i >= 0; --i)
> + kho_unpreserve_folio(folios[i]);
> + vfree(pfolios);
> +err_unpin:
> + unpin_folios(folios, nr_folios);
> +err_free_folios:
> + kvfree(folios);
> + return ERR_PTR(err);
> +}
> +
> +static void memfd_luo_unpreserve_folios(void *fdt, struct memfd_luo_folio_ser *pfolios,
> + u64 nr_folios)
> +{
> + struct kho_vmalloc *kho_vmalloc;
> + long i;
> +
> + if (!nr_folios)
> + return;
> +
> + kho_vmalloc = (struct kho_vmalloc *)fdt_getprop(fdt, 0, MEMFD_FDT_FOLIOS, NULL);
> + /* The FDT was created by this kernel so expect it to be sane. */
> + WARN_ON_ONCE(!kho_vmalloc);
The FDT won't have FOLIOS property if size was zero, will it?
I think that if we add kho_vmalloc handle to struct memfd_luo_private and
pass that around it will make things easier and simpler.
> + kho_unpreserve_vmalloc(kho_vmalloc);
> +
> + for (i = 0; i < nr_folios; i++) {
> + const struct memfd_luo_folio_ser *pfolio = &pfolios[i];
> + struct folio *folio;
> +
> + if (!pfolio->foliodesc)
> + continue;
How can this happen? Can pfolios be a sparse array?
> + folio = pfn_folio(PRESERVED_FOLIO_PFN(pfolio->foliodesc));
> +
> + kho_unpreserve_folio(folio);
> + unpin_folio(folio);
> + }
> +
> + vfree(pfolios);
> +}
...
> +static void memfd_luo_finish(struct liveupdate_file_op_args *args)
> +{
> + const struct memfd_luo_folio_ser *pfolios;
> + struct folio *fdt_folio;
> + const void *fdt;
> + u64 nr_folios;
> +
> + if (args->retrieved)
> + return;
> +
> + fdt_folio = memfd_luo_get_fdt(args->serialized_data);
> + if (!fdt_folio) {
> + pr_err("failed to restore memfd FDT\n");
> + return;
> + }
> +
> + fdt = folio_address(fdt_folio);
> +
> + pfolios = memfd_luo_fdt_folios(fdt, &nr_folios);
> + if (!pfolios)
> + goto out;
> +
> + memfd_luo_discard_folios(pfolios, nr_folios);
Does not this free the actual folios that were supposed to be preserved?
> + vfree(pfolios);
> +
> +out:
> + folio_put(fdt_folio);
> +}
...
> +static int memfd_luo_retrieve(struct liveupdate_file_op_args *args)
> +{
> + struct folio *fdt_folio;
> + const u64 *pos, *size;
> + struct file *file;
> + int len, ret = 0;
> + const void *fdt;
> +
> + fdt_folio = memfd_luo_get_fdt(args->serialized_data);
Why do we need to kho_restore_folio() twice? Here and in
memfd_luo_finish()?
> + if (!fdt_folio)
> + return -ENOENT;
> +
> + fdt = page_to_virt(folio_page(fdt_folio, 0));
folio_address()
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists