On Mon, Mar 23, 2026 at 09:18:03AM -0400, Pasha Tatashin wrote: > On Mon, Mar 23, 2026 at 7:55 AM Christian Brauner <[email protected]> wrote: > > > > On Sat, Mar 21, 2026 at 09:04:53PM -0400, Pasha Tatashin wrote: > > > On Sat, Mar 21, 2026 at 1:58 PM Pasha Tatashin > > > <[email protected]> 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) <[email protected]> > > > > Acked-by: Jan Kara <[email protected]> > > > > Signed-off-by: Pasha Tatashin <[email protected]> > > > > --- > > > > 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/[email protected] > > > > > > 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. > > > > There's a compile-time switch you might want to turn on when > > test-compiling code like this. I forget exactly what it is. Something > > like jump-over-uninit or something. > > > > > > > > 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. > > > > So, I'm not happy about I_LUO_MANAGED. I don't think we need driver > > specific stuff in struct inode and not in i_state. Track this in the > > driver please. I don't want this precedent and I'd rather have you get > > I am planning to use an xarray in the next version. > > > used to implementing such things in the driver right away rather than > > offloading this on general infrastructure. If we let this slide struct > > inode will be 2MB 1 in year. > > Claiming that a single flag bit precedent would cause the overall > struct to grow by 2MB in a year is a slight exaggeration. :-)
Hm, you say that. But then you don't get ~5-10 patches a year that "just add a new member into struct inode with 4-8 bytes"... I'm just making an exaggerated point ofc. :) But struct inode is used everywhere and I want it contained and small and whatever lands in it - even flags - better be VFS generic stuff. We sometimes do carve out exceptions for _filesystem drivers_ where no other way is possible ofc. But I don't think this should extend to drivers/.
