linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Pasha Tatashin <pasha.tatashin@soleen.com>
To: brauner@kernel.org, linux-kselftest@vger.kernel.org,
	rppt@kernel.org,  jack@suse.cz, shuah@kernel.org,
	linux-fsdevel@vger.kernel.org,  akpm@linux-foundation.org,
	linux-mm@kvack.org, viro@zeniv.linux.org.uk,
	 linux-kernel@vger.kernel.org, pasha.tatashin@soleen.com,
	dmatlack@google.com,  pratyush@kernel.org, skhawaja@google.com
Subject: Re: [PATCH 1/2] liveupdate: prevent double management of files
Date: Sat, 21 Mar 2026 21:04:53 -0400	[thread overview]
Message-ID: <CA+CK2bD2aNu-NByET4KkBP0n2j8WQE2zjxb4g-1cv2hYfjaRZQ@mail.gmail.com> (raw)
In-Reply-To: <20260321175808.57942-2-pasha.tatashin@soleen.com>

On Sat, Mar 21, 2026 at 1:58 PM Pasha Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Currently, LUO does not prevent the same file from being managed twice
> across different active sessions.
>
> Add a new i_state flag I_LUO_MANAGED and update luo_preserve_file()
> to check and set this flag when a file is preserved, and clear it in
> luo_file_unpreserve_files() when it is released.
>
> Additionally, set this flag in luo_retrieve_file() after a file is
> successfully restored in the new kernel, and clear it in
> luo_file_finish() when the LUO session is finalized.
>
> This ensures that the same file (inode) cannot be managed by multiple
> sessions. If another session attempts to preserve an already managed
> file, it will now fail with -EBUSY.
>
> Acked-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> Acked-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  include/linux/fs.h           |  5 ++++-
>  kernel/liveupdate/luo_file.c | 27 ++++++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 23f36a2613a3..692a8be56f3c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -712,6 +712,8 @@ is_uncached_acl(struct posix_acl *acl)
>   * I_LRU_ISOLATING     Inode is pinned being isolated from LRU without holding
>   *                     i_count.
>   *
> + * I_LUO_MANAGED       Inode is being managed by a live update session.
> + *
>   * Q: What is the difference between I_WILL_FREE and I_FREEING?
>   *
>   * __I_{SYNC,NEW,LRU_ISOLATING} are used to derive unique addresses to wait
> @@ -744,7 +746,8 @@ enum inode_state_flags_enum {
>         I_CREATING              = (1U << 15),
>         I_DONTCACHE             = (1U << 16),
>         I_SYNC_QUEUED           = (1U << 17),
> -       I_PINNING_NETFS_WB      = (1U << 18)
> +       I_PINNING_NETFS_WB      = (1U << 18),
> +       I_LUO_MANAGED           = (1U << 19),
>  };
>
>  #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index 5acee4174bf0..86911beeff71 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
> @@ -248,6 +248,7 @@ static bool luo_token_is_used(struct luo_file_set *file_set, u64 token)
>   * Context: Can be called from an ioctl handler during normal system operation.
>   * Return: 0 on success. Returns a negative errno on failure:
>   *         -EEXIST if the token is already used.
> + *         -EBUSY if the file descriptor is already preserved by another session.
>   *         -EBADF if the file descriptor is invalid.
>   *         -ENOSPC if the file_set is full.
>   *         -ENOENT if no compatible handler is found.
> @@ -276,6 +277,14 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>         if (err)
>                 goto  err_fput;
>
> +       scoped_guard(spinlock, &file_inode(file)->i_lock) {
> +               if (inode_state_read(file_inode(file)) & I_LUO_MANAGED) {
> +                       err = -EBUSY;
> +                       goto err_free_files_mem;
> +               }
> +               inode_state_set(file_inode(file), I_LUO_MANAGED);
> +       }
> +
>         err = -ENOENT;
>         list_private_for_each_entry(fh, &luo_file_handler_list, list) {
>                 if (fh->ops->can_preserve(fh, file)) {
> @@ -286,11 +295,11 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>
>         /* err is still -ENOENT if no handler was found */
>         if (err)
> -               goto err_free_files_mem;
> +               goto err_unpreserve_inode;
>
>         err = luo_flb_file_preserve(fh);
>         if (err)
> -               goto err_free_files_mem;
> +               goto err_unpreserve_inode;
>
>         luo_file = kzalloc_obj(*luo_file);
>         if (!luo_file) {
> @@ -320,6 +329,9 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>         kfree(luo_file);
>  err_flb_unpreserve:
>         luo_flb_file_unpreserve(fh);
> +err_unpreserve_inode:
> +       scoped_guard(spinlock, &file_inode(file)->i_lock)
> +               inode_state_clear(file_inode(file), I_LUO_MANAGED);
>  err_free_files_mem:
>         luo_free_files_mem(file_set);
>  err_fput:
> @@ -363,6 +375,9 @@ void luo_file_unpreserve_files(struct luo_file_set *file_set)
>                 luo_file->fh->ops->unpreserve(&args);
>                 luo_flb_file_unpreserve(luo_file->fh);
>
> +               scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +                       inode_state_clear(file_inode(luo_file->file), I_LUO_MANAGED);
> +
>                 list_del(&luo_file->list);
>                 file_set->count--;
>
> @@ -609,6 +624,9 @@ int luo_retrieve_file(struct luo_file_set *file_set, u64 token,
>         *filep = luo_file->file;
>         luo_file->retrieve_status = 1;
>
> +       scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +               inode_state_set(file_inode(luo_file->file), I_LUO_MANAGED);
> +
>         return 0;
>  }
>
> @@ -701,8 +719,11 @@ int luo_file_finish(struct luo_file_set *file_set)
>
>                 luo_file_finish_one(file_set, luo_file);
>
> -               if (luo_file->file)
> +               if (luo_file->file) {
> +                       scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +                               inode_state_clear(file_inode(luo_file->file), I_LUO_MANAGED);
>                         fput(luo_file->file);
> +               }
>                 list_del(&luo_file->list);
>                 file_set->count--;
>                 mutex_destroy(&luo_file->mutex);
> --
> 2.43.0
>

> Sashiko: https://sashiko.dev/#/patchset/20260321175808.57942-1-pasha.tatashin@soleen.com

Sashiko reported two problems:

1. Are there any issues with mixing goto-based error handling and scope-based
cleanups like scoped_guard() in the same function?

Initially, I thought that there should not be any problems, however,
after looking this up  I found in include/linux/cleanup.h the
following comment:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function.

Well, good to know, will not use goto inside scoped_guards.

2. Additionally, does setting I_LUO_MANAGED on the inode break the preservation
of anonymous inodes? Many file types (like eventfd, epoll, timerfd,
signalfd)

This is actually a very good point. It looks like everyone who uses
anon_inode_getfd() has one shared inode. This is not a problem for the
existing LUO user memfd, or for the upcoming vfiofd and memfd, but
kvm-vmfd and kvm-cpufd also use it, and that might be a problem in the
future once we add support for Orphaned VMs.

Therefore, we have two choices: either use a hash table, which adds
performance and memory overhead, or delegate this double-check to the
LUO file handlers, as they can use a private context to know if the FD
is already preserved.

Pasha


  reply	other threads:[~2026-03-22  1:05 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-21 17:58 [PATCH 0/2] " Pasha Tatashin
2026-03-21 17:58 ` [PATCH 1/2] " Pasha Tatashin
2026-03-22  1:04   ` Pasha Tatashin [this message]
2026-03-23 11:55     ` Christian Brauner
2026-03-23 13:18       ` Pasha Tatashin
2026-03-24  8:51         ` Christian Brauner
2026-03-24 18:40           ` Andrew Morton
2026-03-25  2:43             ` Pasha Tatashin
2026-03-21 17:58 ` [PATCH 2/2] selftests: liveupdate: add test for double preservation Pasha Tatashin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+CK2bD2aNu-NByET4KkBP0n2j8WQE2zjxb4g-1cv2hYfjaRZQ@mail.gmail.com \
    --to=pasha.tatashin@soleen.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=dmatlack@google.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pratyush@kernel.org \
    --cc=rppt@kernel.org \
    --cc=shuah@kernel.org \
    --cc=skhawaja@google.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox