Re: [PATCH v7 06/22] liveupdate: luo_file: implement file systems callbacks

On Mon, Nov 24, 2025 at 3:18 AM Mike Rapoport [off-list ref] wrote:

On Sat, Nov 22, 2025 at 05:23:33PM -0500, Pasha Tatashin wrote:

quoted

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@soleen.com>

With some comments below
Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

quoted

---
 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

...

quoted

+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?

Moved up.

quoted

+             err = -ENOSPC;
+             goto err_files_mem;

The goto label should say what it does, not what the error was.

Changed to err_free_files_mem;

quoted

+     }
+
+     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.

Removed.

quoted

+     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);

Yeap, done like this.

quoted

+
+     return err;
+}

...

quoted

+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()?

In this case I prefer while(!list_empty(...))
It emphasizes to  a reader that we are emptying the full list. _safe
is good to use when some items are removed from a list while
traversing.

quoted

+             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);
+}

...

quoted

+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()?

Same

quoted

+             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);
+     }
+

...

quoted

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 :)

Exactly, I am planning to propose this as a separate change for
list.h, but I suspect we will need to take care of other variants as
well, reverse, cont, safe etc.

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))

This does not work, because list_next_entry also accesses private,
what works is this:
#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_entry(ACCESS_PRIVATE(pos, member).next, typeof(*pos), member))

But that extra ACCESS_PRIVATE bothers me, so let's keep it as-is for
now. And solve it once in list.h, and then update the private macro.

--
Sincerely yours,
Mike.