Re: [PATCH 03/14] liveupdate: luo_file: Add internal APIs for file preservation

[Samiullah Khawaja]

On Wed, Mar 18, 2026 at 10:00:57AM +0000, Pranjal Shrivastava wrote:
> On Tue, Feb 03, 2026 at 10:09:37PM +0000, Samiullah Khawaja wrote:
> > From: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
> >
> > The core liveupdate mechanism allows userspace to preserve file
> > descriptors. However, kernel subsystems often manage struct file
> > objects directly and need to participate in the preservation process
> > programmatically without relying solely on userspace interaction.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
> > ---
> >  include/linux/liveupdate.h       | 21 ++++++++++
> >  kernel/liveupdate/luo_file.c     | 71 ++++++++++++++++++++++++++++++++
> >  kernel/liveupdate/luo_internal.h | 16 +++++++
> >  3 files changed, 108 insertions(+)
> >
> > diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
> > index fe82a6c3005f..8e47504ba01e 100644
> > --- a/include/linux/liveupdate.h
> > +++ b/include/linux/liveupdate.h
> > @@ -23,6 +23,7 @@ struct file;
> >  /**
> >   * struct liveupdate_file_op_args - Arguments for file operation callbacks.
> >   * @handler:          The file handler being called.
> > + * @session:          The session this file belongs to.
> >   * @retrieved:        The retrieve status for the 'can_finish / finish'
> >   *                    operation.
> >   * @file:             The file object. For retrieve: [OUT] The callback sets
> > @@ -40,6 +41,7 @@ struct file;
> >   */
> >  struct liveupdate_file_op_args {
> >  	struct liveupdate_file_handler *handler;
> > +	struct liveupdate_session *session;
> >  	bool retrieved;
>
> Nit: I don't think this is on the latest tree. I see `int retrieved` [1]
> in the latest tree. I guess we'd need to rebase it on the latest?

Will rebase this.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/linux/liveupdate.h#n46
>
> >  	struct file *file;
> >  	u64 serialized_data;
> > @@ -234,6 +236,13 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> >
> >  int liveupdate_flb_get_incoming(struct liveupdate_flb *flb, void **objp);
> >  int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb, void **objp);
> > +/* kernel can internally retrieve files */
> > +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> > +				 struct file **filep);
> > +
> > +/* Get a token for an outgoing file, or -ENOENT if file is not preserved */
> > +int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> > +				  struct file *file, u64 *tokenp);
> >
> >  #else /* CONFIG_LIVEUPDATE */
> >
> > @@ -281,5 +290,17 @@ static inline int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb,
> >  	return -EOPNOTSUPP;
> >  }
> >
> > +static inline int liveupdate_get_file_incoming(struct liveupdate_session *s,
> > +					       u64 token, struct file **filep)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +static inline int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> > +						struct file *file, u64 *tokenp)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> >  #endif /* CONFIG_LIVEUPDATE */
> >  #endif /* _LINUX_LIVEUPDATE_H */
> > diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> > index 32759e846bc9..7ac591542059 100644
> > --- a/kernel/liveupdate/luo_file.c
> > +++ b/kernel/liveupdate/luo_file.c
> > @@ -302,6 +302,7 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
> >  	mutex_init(&luo_file->mutex);
> >
> >  	args.handler = fh;
> > +	args.session = luo_session_from_file_set(file_set);
> >  	args.file = file;
> >  	err = fh->ops->preserve(&args);
> >  	if (err)
> > @@ -355,6 +356,7 @@ void luo_file_unpreserve_files(struct luo_file_set *file_set)
> >  					   struct luo_file, list);
> >
> >  		args.handler = luo_file->fh;
> > +		args.session = luo_session_from_file_set(file_set);
> >  		args.file = luo_file->file;
> >  		args.serialized_data = luo_file->serialized_data;
> >  		args.private_data = luo_file->private_data;
> > @@ -383,6 +385,7 @@ static int luo_file_freeze_one(struct luo_file_set *file_set,
> >  		struct liveupdate_file_op_args args = {0};
> >
> >  		args.handler = luo_file->fh;
> > +		args.session = luo_session_from_file_set(file_set);
> >  		args.file = luo_file->file;
> >  		args.serialized_data = luo_file->serialized_data;
> >  		args.private_data = luo_file->private_data;
> > @@ -404,6 +407,7 @@ static void luo_file_unfreeze_one(struct luo_file_set *file_set,
> >  		struct liveupdate_file_op_args args = {0};
> >
> >  		args.handler = luo_file->fh;
> > +		args.session = luo_session_from_file_set(file_set);
> >  		args.file = luo_file->file;
> >  		args.serialized_data = luo_file->serialized_data;
> >  		args.private_data = luo_file->private_data;
> > @@ -590,6 +594,7 @@ int luo_retrieve_file(struct luo_file_set *file_set, u64 token,
> >  	}
> >
> >  	args.handler = luo_file->fh;
> > +	args.session = luo_session_from_file_set(file_set);
> >  	args.serialized_data = luo_file->serialized_data;
> >  	err = luo_file->fh->ops->retrieve(&args);
> >  	if (!err) {
> > @@ -615,6 +620,7 @@ static int luo_file_can_finish_one(struct luo_file_set *file_set,
> >  		struct liveupdate_file_op_args args = {0};
> >
> >  		args.handler = luo_file->fh;
> > +		args.session = luo_session_from_file_set(file_set);
> >  		args.file = luo_file->file;
> >  		args.serialized_data = luo_file->serialized_data;
> >  		args.retrieved = luo_file->retrieved;
> > @@ -632,6 +638,7 @@ static void luo_file_finish_one(struct luo_file_set *file_set,
> >  	guard(mutex)(&luo_file->mutex);
> >
> >  	args.handler = luo_file->fh;
> > +	args.session = luo_session_from_file_set(file_set);
> >  	args.file = luo_file->file;
> >  	args.serialized_data = luo_file->serialized_data;
> >  	args.retrieved = luo_file->retrieved;
> > @@ -919,3 +926,67 @@ int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
> >  	return err;
> >  }
> >  EXPORT_SYMBOL_GPL(liveupdate_unregister_file_handler);
> > +
> > +/**
> > + * liveupdate_get_token_outgoing - Get the token for a preserved file.
> > + * @s:      The outgoing liveupdate session.
> > + * @file:   The file object to search for.
> > + * @tokenp: Output parameter for the found token.
> > + *
> > + * Searches the list of preserved files in an outgoing session for a matching
> > + * file object. If found, the corresponding user-provided token is returned.
> > + *
> > + * This function is intended for in-kernel callers that need to correlate a
> > + * file with its liveupdate token.
> > + *
> > + * Context: Can be called from any context that can acquire the session mutex.

I will reword this to convey that the session mutex should be acquired.
> > + * Return: 0 on success, -ENOENT if the file is not preserved in this session.
> > + */
> > +int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> > +				  struct file *file, u64 *tokenp)
> > +{
> > +	struct luo_file_set *file_set = luo_file_set_from_session(s);
> > +	struct luo_file *luo_file;
> > +	int err = -ENOENT;
> > +
> > +	list_for_each_entry(luo_file, &file_set->files_list, list) {
>
> Shouldn't we hold a lock while traversing the file_set? Couldn't this
> race with an unpreserve? If a concurrent unpreserve (writer) calls
> list_del while this reader is mid-iteration, we'll likely follow a stale
> pointer..

The session mutex is supposed to be acquired when this function is
called. I will add a lockdep in this function to enforce that.

Also, we call this in preserve() of vfio-cdev and the session mutex is
already acquired by luo in that context.
> I noticed that luo_preserve_file / upreserve and retrieve also don't
> take locks while manipulating / iterating  over file_set->files_list..
>
> Shouldn't we protect these with a lock? Otherwise, how do we avoid
> races in situations like:
>
> 1. CPU 0 (reader) is iterating the list A->B->C ..
> 2. CPU 1 (writer) is handling an unpreserve which remove file B
>
> Am I missing something? If we're expecting the callers to hold a lock,
> we should have a lockdep_assert in these functions..

The LUO acquires session mutex before calling preserve/unpreserve to
protect agains race conditions. See luo_session.c for details.
> > +		if (luo_file->file == file) {
> > +			if (tokenp)
> > +				*tokenp = luo_file->token;
> > +			err = 0;
> > +			break;
> > +		}
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +/**
> > + * liveupdate_get_file_incoming - Retrieves a preserved file for in-kernel use.
> > + * @s:      The incoming liveupdate session (restored from the previous kernel).
> > + * @token:  The unique token identifying the file to retrieve.
> > + * @filep:  On success, this will be populated with a pointer to the retrieved
> > + *          'struct file'.
> > + *
> > + * Provides a kernel-internal API for other subsystems to retrieve their
> > + * preserved files after a live update. This function is a simple wrapper
> > + * around luo_retrieve_file(), allowing callers to find a file by its token.
> > + *
> > + * The operation is idempotent; subsequent calls for the same token will return
> > + * a pointer to the same 'struct file' object.
> > + *
> > + * The caller receives a new reference to the file and must call fput() when it
> > + * is no longer needed. The file's lifetime is managed by LUO and any userspace
> > + * file descriptors. If the caller needs to hold a reference to the file beyond
> > + * the immediate scope, it must call get_file() itself.
> > + *
>
> I'm little confused here, we say the op is idempotent, but also mention
> that the caller receives a new reference. I'm wondering of a situation
> where a driver calls this multiple times, incrementing the refcount with
> each call. Do we rely on flb_file_finish to drop all the refcounts?
>
> We should clarify the lifecycle requirements here: is the driver
> expected to call fput() for every single call to
> liveupdate_get_file_incoming(), or is the flb_finish callback intended
> to be a 'catch-all' that reaps these?

The user is supposed to call fput to release the reference as mentioned
in the kernel-doc above.

But I agree it is a little confusing with the idempotent. I will reword
the text.
> > + * Context: Can be called from any context in the new kernel that has a handle
> > + *          to a restored session.
> > + * Return: 0 on success. Returns -ENOENT if no file with the matching token is
> > + *         found, or any other negative errno on failure.
> > + */
> > +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> > +				 struct file **filep)
> > +{
> > +	return luo_retrieve_file(luo_file_set_from_session(s), token, filep);
> > +}
> > diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h
> > index 8083d8739b09..a24933d24fd9 100644
> > --- a/kernel/liveupdate/luo_internal.h
> > +++ b/kernel/liveupdate/luo_internal.h
> > @@ -77,6 +77,22 @@ struct luo_session {
> >  	struct mutex mutex;
> >  };
> >
> > +static inline struct liveupdate_session *luo_session_from_file_set(struct luo_file_set *file_set)
> > +{
> > +	struct luo_session *session;
> > +
> > +	session = container_of(file_set, struct luo_session, file_set);
> > +
> > +	return (struct liveupdate_session *)session;
> > +}
> > +
> > +static inline struct luo_file_set *luo_file_set_from_session(struct liveupdate_session *s)
> > +{
> > +	struct luo_session *session = (struct luo_session *)s;
> > +
> > +	return &session->file_set;
> > +}
> > +
> >  int luo_session_create(const char *name, struct file **filep);
> >  int luo_session_retrieve(const char *name, struct file **filep);
> >  int __init luo_session_setup_outgoing(void *fdt);
> > --
>
> Thanks,
> Praan