[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aSQU1LlPDDsN2rUw@kernel.org>
Date: Mon, 24 Nov 2025 10:18:28 +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 v7 06/22] liveupdate: luo_file: implement file systems
callbacks
On Sat, Nov 22, 2025 at 05:23:33PM -0500, Pasha Tatashin wrote:
> This patch implements the core mechanism for managing preserved
> files throughout the live update lifecycle. It provides the logic to
> invoke the file handler callbacks (preserve, unpreserve, freeze,
> unfreeze, retrieve, and finish) at the appropriate stages.
>
> During the reboot phase, luo_file_freeze() serializes the final
> metadata for each file (handler compatible string, token, and data
> handle) into a memory region preserved by KHO. In the new kernel,
> luo_file_deserialize() reconstructs the in-memory file list from this
> data, preparing the session for retrieval.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
With some comments below
Reviewed-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> ---
> include/linux/kho/abi/luo.h | 39 +-
> include/linux/liveupdate.h | 98 ++++
> kernel/liveupdate/Makefile | 1 +
> kernel/liveupdate/luo_file.c | 882 +++++++++++++++++++++++++++++++
> kernel/liveupdate/luo_internal.h | 38 ++
> 5 files changed, 1057 insertions(+), 1 deletion(-)
> create mode 100644 kernel/liveupdate/luo_file.c
>
...
> +int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
> +{
> + struct liveupdate_file_op_args args = {0};
> + struct liveupdate_file_handler *fh;
> + struct luo_file *luo_file;
> + struct file *file;
> + int err;
> +
> + if (luo_token_is_used(file_set, token))
> + return -EEXIST;
> +
> + file = fget(fd);
> + if (!file)
> + return -EBADF;
> +
> + err = luo_alloc_files_mem(file_set);
> + if (err)
> + goto err_files_mem;
> +
> + if (file_set->count == LUO_FILE_MAX) {
This can be checked before getting the file and allocating memory, can't it?
> + err = -ENOSPC;
> + goto err_files_mem;
The goto label should say what it does, not what the error was.
> + }
> +
> + err = -ENOENT;
> + luo_list_for_each_private(fh, &luo_file_handler_list, list) {
> + if (fh->ops->can_preserve(fh, file)) {
> + err = 0;
> + break;
> + }
> + }
> +
> + /* err is still -ENOENT if no handler was found */
> + if (err)
> + goto err_files_mem;
> +
> + luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> + if (!luo_file) {
> + err = -ENOMEM;
> + goto err_files_mem;
> + }
> +
> + luo_file->file = file;
> + luo_file->fh = fh;
> + luo_file->token = token;
> + luo_file->retrieved = false;
> + mutex_init(&luo_file->mutex);
> +
> + args.handler = fh;
> + args.file = file;
> + err = fh->ops->preserve(&args);
> + if (err)
> + goto err_kfree;
> +
> + luo_file->serialized_data = args.serialized_data;
> + list_add_tail(&luo_file->list, &file_set->files_list);
> + file_set->count++;
> +
> + return 0;
> +
> +err_kfree:
> + mutex_destroy(&luo_file->mutex);
Don't think we need this, luo_file is freed in the next line.
> + kfree(luo_file);
> +err_files_mem:
> + fput(file);
> + luo_free_files_mem(file_set);
I'd have the error path as
err_free_luo_file:
kfree(luo_file);
err_free_files_mem:
luo_free_files_mem(file_set);
err_put_file:
fput(file);
> +
> + return err;
> +}
...
> +void luo_file_unpreserve_files(struct luo_file_set *file_set)
> +{
> + struct luo_file *luo_file;
> +
> + while (!list_empty(&file_set->files_list)) {
list_for_each_entry_safe_reverse()?
> + struct liveupdate_file_op_args args = {0};
> +
> + luo_file = list_last_entry(&file_set->files_list,
> + struct luo_file, list);
> +
> + args.handler = luo_file->fh;
> + args.file = luo_file->file;
> + args.serialized_data = luo_file->serialized_data;
> + luo_file->fh->ops->unpreserve(&args);
> +
> + list_del(&luo_file->list);
> + file_set->count--;
> +
> + fput(luo_file->file);
> + mutex_destroy(&luo_file->mutex);
> + kfree(luo_file);
> + }
> +
> + luo_free_files_mem(file_set);
> +}
...
> +int luo_file_finish(struct luo_file_set *file_set)
> +{
> + struct list_head *files_list = &file_set->files_list;
> + struct luo_file *luo_file;
> + int err;
> +
> + if (!file_set->count)
> + return 0;
> +
> + list_for_each_entry(luo_file, files_list, list) {
> + err = luo_file_can_finish_one(file_set, luo_file);
> + if (err)
> + return err;
> + }
> +
> + while (!list_empty(&file_set->files_list)) {
list_for_each_entry_safe_reverse()?
> + luo_file = list_last_entry(&file_set->files_list,
> + struct luo_file, list);
> +
> + luo_file_finish_one(file_set, luo_file);
> +
> + if (luo_file->file)
> + fput(luo_file->file);
> + list_del(&luo_file->list);
> + file_set->count--;
> + mutex_destroy(&luo_file->mutex);
> + kfree(luo_file);
> + }
> +
...
> diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h
> index 1292ac47eef8..c8973b543d1d 100644
> --- a/kernel/liveupdate/luo_internal.h
> +++ b/kernel/liveupdate/luo_internal.h
> @@ -40,6 +40,28 @@ static inline int luo_ucmd_respond(struct luo_ucmd *ucmd,
> */
> #define luo_restore_fail(__fmt, ...) panic(__fmt, ##__VA_ARGS__)
>
> +/* Mimics list_for_each_entry() but for private list head entries */
> +#define luo_list_for_each_private(pos, head, member) \
> + for (struct list_head *__iter = (head)->next; \
> + __iter != (head) && \
> + ({ pos = container_of(__iter, typeof(*(pos)), member); 1; }); \
> + __iter = __iter->next)
Ideally something like this should go to include/linux/list.h, but it can
be done later to avoid bikeshedding about the name :)
And you can reuse most of list_for_each_entry, just replace the line that
accesses __private member:
#define luo_list_for_each_private(pos, head, member) \
for (pos = list_first_entry(head, typeof(*pos), member); \
&ACCESS_PRIVATE(pos, member) != head; \
pos = list_next_entry(pos, member))
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists