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

[Pratyush Yadav]

Hi Pasha,

Sorry, I accidentally sent this without trimming the context first.

Below is the version with the relevant bits:

 On Sat, Nov 22 2025, 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@xxxxxxxxxx>
> ---
>  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
>
> diff --git a/include/linux/kho/abi/luo.h b/include/linux/kho/abi/luo.h
> index a2d2940eca6b..fc143f243871 100644
> --- a/include/linux/kho/abi/luo.h
> +++ b/include/linux/kho/abi/luo.h
> @@ -65,6 +65,11 @@
>   *     Metadata for a single session, including its name and a physical pointer
>   *     to another preserved memory block containing an array of
>   *     `struct luo_file_ser` for all files in that session.
> + *
> + *   - struct luo_file_ser:
> + *     Metadata for a single preserved file. Contains the `compatible` string to
> + *     find the correct handler in the new kernel, a user-provided `token` for
> + *     identification, and an opaque `data` handle for the handler to use.
>   */
>  
>  #ifndef _LINUX_KHO_ABI_LUO_H
> @@ -82,13 +87,43 @@
>  #define LUO_FDT_COMPATIBLE	"luo-v1"
>  #define LUO_FDT_LIVEUPDATE_NUM	"liveupdate-number"
>  
> +#define LIVEUPDATE_HNDL_COMPAT_LENGTH	48
> +
> +/**
> + * struct luo_file_ser - Represents the serialized preserves files.
> + * @compatible:  File handler compatible string.
> + * @data:        Private data
> + * @token:       User provided token for this file
> + *
> + * If this structure is modified, LUO_SESSION_COMPATIBLE must be updated.
> + */
> +struct luo_file_ser {
> +	char compatible[LIVEUPDATE_HNDL_COMPAT_LENGTH];
> +	u64 data;
> +	u64 token;
> +} __packed;
> +
> +/**
> + * struct luo_file_set_ser - Represents the serialized metadata for file set
> + * @files:   The physical address of a contiguous memory block that holds
> + *           the serialized state of files (array of luo_file_ser) in this file
> + *           set.
> + * @count:   The total number of files that were part of this session during
> + *           serialization. Used for iteration and validation during
> + *           restoration.
> + */
> +struct luo_file_set_ser {
> +	u64 files;
> +	u64 count;
> +} __packed;

 The change to using file_set looks a lot nicer than what the previous
 version was doing!

> +
>  /*
>   * LUO FDT session node
>   * LUO_FDT_SESSION_HEADER:  is a u64 physical address of struct
>   *                          luo_session_header_ser
>   */
>  #define LUO_FDT_SESSION_NODE_NAME	"luo-session"
> -#define LUO_FDT_SESSION_COMPATIBLE	"luo-session-v1"
> +#define LUO_FDT_SESSION_COMPATIBLE	"luo-session-v2"
>  #define LUO_FDT_SESSION_HEADER		"luo-session-header"
>  
>  /**
[...]
> +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;

 Nit:                ^^ two spaces here.

> +
> +	if (file_set->count == LUO_FILE_MAX) {
> +		err = -ENOSPC;
> +		goto err_files_mem;
> +	}
> +
> +	err = -ENOENT;
> +	luo_list_for_each_private(fh, &luo_file_handler_list, list) {
> +		if (fh->ops->can_preserve(fh, file)) {
> +			err = 0;
> +			break;
> +		}
> +	}
> +
[...]
> +int liveupdate_register_file_handler(struct liveupdate_file_handler *fh)
> +{
> +	struct liveupdate_file_handler *fh_iter;
> +	int err;
> +
> +	if (!liveupdate_enabled())
> +		return -EOPNOTSUPP;
> +
> +	/* Sanity check that all required callbacks are set */
> +	if (!fh->ops->preserve || !fh->ops->unpreserve ||
> +	    !fh->ops->retrieve || !fh->ops->finish) {

 You are still missing a check for can_preserve() here. It is a mandatory
 callback and luo_preserve_file() calls it without checking for NULL.

 With these and Mike's comments addressed,

 Reviewed-by: Pratyush Yadav <pratyush@xxxxxxxxxx>

> +		return -EINVAL;
> +	}
> +
[...]

-- 
Regards,
Pratyush Yadav