On Thu, May 15, 2025 at 11:23 AM Pasha Tatashin
<pasha.tatashin@xxxxxxxxxx> wrote:
> +/**
> + * luo_retrieve_file - Find a registered file instance by its token.
> + * @token: The unique token of the file instance to retrieve.
> + * @file: Output parameter. On success (return value 0), this will point
> + * to the retrieved "struct file".
> + *
> + * Searches the global list for a &struct luo_file matching the @token. Uses a
> + * read lock, allowing concurrent retrievals.
> + *
> + * Return: 0 on success. Negative errno on failure.
> + */
> +int luo_retrieve_file(u64 token, struct file **file)
> +{
> + struct luo_file *luo_file;
> + int ret = 0;
> +
> + luo_files_recreate_luo_files_xa_in();
> + luo_state_read_enter();
> + if (!liveupdate_state_updated()) {
> + pr_warn("File can be retrieved only in updated state\n");
> + luo_state_read_exit();
> + return -EBUSY;
> + }
> +
> + luo_file = xa_load(&luo_files_xa_in, token);
> + if (luo_file && !luo_file->reclaimed) {
> + luo_file->reclaimed = true;
I haven't been able to pay too much attention to the series yet, and I
know this was posted as an RFC, so pardon my nit-picking.
I think you need to have xchg here for this not to be racy, so something like:
`if (luo_file && !xchg(&luo_file->reclaimed, true))`
Or maybe you meant to avoid this race some other way; IIUC,
luo_state_read_enter() is not sufficient.
Thanks!
> + ret = luo_file->fs->retrieve(luo_file->fs->arg,
> + luo_file->private_data,
> + file);
> + if (!ret)
> + luo_file->file = *file;
> + } else if (luo_file && luo_file->reclaimed) {
> + pr_err("The file descriptor for token %lld has already been retrieved\n",
> + token);
> + ret = -EINVAL;
> + } else {
> + ret = -ENOENT;
> + }
> +
> + luo_state_read_exit();
> +
> + return ret;
> +}
