Re: [PATCH v3 18/30] liveupdate: luo_files: luo_ioctl: Add ioctls for per-file state management
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2025-08-14 14:02:57
Also in:
linux-api, linux-fsdevel, linux-mm, lkml
On Thu, Aug 07, 2025 at 01:44:24AM +0000, Pasha Tatashin wrote:
+struct liveupdate_ioctl_get_fd_state {
+ __u32 size;
+ __u8 incoming;
+ __aligned_u64 token;
+ __u32 state;
+};Same remark about explicit padding and checking padding for 0
+ * luo_file_get_state - Get the preservation state of a specific file.
+ * @token: The token of the file to query.
+ * @statep: Output pointer to store the file's current live update state.
+ * @incoming: If true, query the state of a restored file from the incoming
+ * (previous kernel's) set. If false, query a file being prepared
+ * for preservation in the current set.
+ *
+ * Finds the file associated with the given @token in either the incoming
+ * or outgoing tracking arrays and returns its current LUO state
+ * (NORMAL, PREPARED, FROZEN, UPDATED).
+ *
+ * Return: 0 on success, -ENOENT if the token is not found.
+ */
+int luo_file_get_state(u64 token, enum liveupdate_state *statep, bool incoming)
+{
+ struct luo_file *luo_file;
+ struct xarray *target_xa;
+ int ret = 0;
+
+ luo_state_read_enter();Less globals, at this point everything should be within memory attached to the file descriptor and not in globals. Doing this will promote good maintainable structure and not a spaghetti Also I think a BKL design is not a good idea for new code. We've had so many bad experiences with this pattern promoting uncontrolled incomprehensible locking. The xarray already has a lock, why not have reasonable locking inside the luo_file? Probably just a refcount?
+ target_xa = incoming ? &luo_files_xa_in : &luo_files_xa_out;
+ luo_file = xa_load(target_xa, token);
+
+ if (!luo_file) {
+ ret = -ENOENT;
+ goto out_unlock;
+ }
+
+ scoped_guard(mutex, &luo_file->mutex)
+ *statep = luo_file->state;
+
+out_unlock:
+ luo_state_read_exit();If we are using cleanup.h then use it for this too.. But it seems kind of weird, why not just xa_lock() xa_load() *statep = READ_ONCE(luo_file->state); xa_unlock() ?
+static int luo_ioctl_set_fd_event(struct luo_ucmd *ucmd)
+{
+ struct liveupdate_ioctl_set_fd_event *argp = ucmd->cmd;
+ int ret;
+
+ switch (argp->event) {
+ case LIVEUPDATE_PREPARE:
+ ret = luo_file_prepare(argp->token);
+ break;
+ case LIVEUPDATE_FREEZE:
+ ret = luo_file_freeze(argp->token);
+ break;
+ case LIVEUPDATE_FINISH:
+ ret = luo_file_finish(argp->token);
+ break;
+ case LIVEUPDATE_CANCEL:
+ ret = luo_file_cancel(argp->token);
+ break;The token should be converted to a file here instead of duplicated in each function
quoted hunk ↗ jump to hunk
static int luo_open(struct inode *inodep, struct file *filep) { if (atomic_cmpxchg(&luo_device_in_use, 0, 1))@@ -149,6 +191,8 @@ union ucmd_buffer { struct liveupdate_ioctl_fd_restore restore; struct liveupdate_ioctl_get_state state; struct liveupdate_ioctl_set_event event; + struct liveupdate_ioctl_get_fd_state fd_state; + struct liveupdate_ioctl_set_fd_event fd_event; }; struct luo_ioctl_op {@@ -179,6 +223,10 @@ static const struct luo_ioctl_op luo_ioctl_ops[] = { struct liveupdate_ioctl_get_state, state), IOCTL_OP(LIVEUPDATE_IOCTL_SET_EVENT, luo_ioctl_set_event, struct liveupdate_ioctl_set_event, event), + IOCTL_OP(LIVEUPDATE_IOCTL_GET_FD_STATE, luo_ioctl_get_fd_state, + struct liveupdate_ioctl_get_fd_state, token), + IOCTL_OP(LIVEUPDATE_IOCTL_SET_FD_EVENT, luo_ioctl_set_fd_event, + struct liveupdate_ioctl_set_fd_event, token), };
Keep sorted Jason