Harshal Chavan <[email protected]> writes:
> Currently, if an application wants to duplicate registered file
> descriptors from one io_uring instance to another, it must manually
> unregister and re-register them, incurring unnecessary overhead.
>
> Add IORING_REGISTER_CLONE_FILES to allow direct cloning of the file
> table from a source ring to a destination ring. This implementation
> strictly mirrors the io_clone_buffers UAPI, supporting partial offsets
> and the IORING_REGISTER_DST_REPLACE flag.
>
> To ensure lock synchronization safety, destination nodes are strictly
> allocated as new, private io_rsrc_nodes rather than sharing references
> across rings.
>
> Signed-off-by: Harshal Chavan <[email protected]>
>
> ---
> Sorry for the noise on the previous email! I accidentally sent the patch
> before running checkpatch and missed a whitespace error. This v6 corrects it.
>
> v6:
> - Fixed trailing whitespace checkpatch error.
> v5:
> - Added missing spacing in comment (Gabriel).
> - Removed ctx->user and mm_account checks (Gabriel).
> - Used !! for boolean conversion (Gabriel).
> - Moved mutex_unlock unconditionally above the out label (Gabriel).
> - liburing implementation and tests:
> https://github.com/axboe/liburing/pull/1606
> v4:
> - Updated Signed-off-by to use real name and moved above the scissors line
> (Greg KH).
> v3:
> - Rewrote the cloning loop to allocate private destination nodes via
> io_rsrc_node_alloc to fix non-atomic ref lock synchronization (Jens).
> - Maintained partial offset/copy support to mirror io_clone_buffers UAPI
> (Jens).
> - Gated the replacement free check on ctx->file_table.data.nr (Gabriel).
> - Prevented self-cloning by checking ctx == src_ctx (Gabriel).
> - Removed submitter_task check to allow cross-thread pooling setups
> (Gabriel).
> v2:
> - Dropped unrelated whitespace formatting changes from v1
> +static int io_clone_file_node(struct io_ring_ctx *ctx,
> + struct io_rsrc_node *src_node,
> + int dst_index,
> + struct io_file_table *new_table)
> +{
> + struct io_rsrc_node *dst_node;
> + struct file *file;
> +
> + dst_node = io_rsrc_node_alloc(ctx, IORING_RSRC_FILE);
> + if (!dst_node)
> + return -ENOMEM;
> +
> + file = io_slot_file(src_node);
> + get_file(file);
> + io_fixed_file_set(dst_node, file);
don't you need to copy the src_node->tag here as well?
I didn't get a chance to run it yet, sorry. I'd suggest you wait for
Jens feedback before pushing the v7 too, so you don't need to keep
iterating drop by drop :)
> +
> + new_table->data.nodes[dst_index] = dst_node;
> + io_file_bitmap_set(new_table, dst_index);
> +
> + return 0;
> +}
> +
> +static int io_clone_files(struct io_ring_ctx *ctx, struct io_ring_ctx
> *src_ctx,
> + struct io_uring_clone_files *arg)
> +{
> + struct io_file_table new_file_table;
> + unsigned int dst_nr = ctx->file_table.data.nr;
> + unsigned int src_nr = src_ctx->file_table.data.nr;
> + unsigned int new_nr, i;
> +
> + lockdep_assert_held(&ctx->uring_lock);
> + lockdep_assert_held(&src_ctx->uring_lock);
> +
> + if (dst_nr && !(arg->flags & IORING_REGISTER_DST_REPLACE))
> + return -EBUSY;
> +
> + if (!src_nr)
> + return -ENXIO;
> +
> + if (!arg->nr)
> + arg->nr = src_nr;
> + else if (arg->nr > src_nr)
> + return -EINVAL;
> +
> + if (check_add_overflow(arg->src_off, arg->nr, &i) || i > src_nr)
> + return -EINVAL;
> + if (check_add_overflow(arg->dst_off, arg->nr, &i))
> + return -EINVAL;
> +
> + new_nr = max(dst_nr, arg->dst_off + arg->nr);
> + if (new_nr > IORING_MAX_FIXED_FILES)
> + return -EINVAL;
> +
> + memset(&new_file_table, 0, sizeof(new_file_table));
> + if (!io_alloc_file_tables(ctx, &new_file_table, new_nr))
> + return -ENOMEM;
> +
> + /* Copy original nodes from before the cloned range */
> + for (i = 0; i < min(arg->dst_off, dst_nr); i++) {
> + struct io_rsrc_node *src_node =
> io_rsrc_node_lookup(&ctx->file_table.data, i);
> +
> + if (!src_node)
> + continue;
> + if (io_clone_file_node(ctx, src_node, i, &new_file_table))
> + goto out;
> + }
> +
> + /* Copy the actual cloned range from the source ring */
> + for (i = 0; i < arg->nr; i++) {
> + struct io_rsrc_node *src_node =
> io_rsrc_node_lookup(&src_ctx->file_table.data,
> + arg->src_off + i);
> +
> + if (!src_node)
> + continue;
> + if (io_clone_file_node(ctx, src_node, arg->dst_off + i,
> &new_file_table))
> + goto out;
> + }
> +
> + /* Copy original nodes from after the cloned range */
> + for (i = arg->dst_off + arg->nr; i < dst_nr; i++) {
> + struct io_rsrc_node *src_node =
> io_rsrc_node_lookup(&ctx->file_table.data, i);
> +
> + if (!src_node)
> + continue;
> + if (io_clone_file_node(ctx, src_node, i, &new_file_table))
> + goto out;
> + }
> +
> + /* free the old file table if there is any data present */
> + if (dst_nr)
> + io_free_file_tables(ctx, &ctx->file_table);
> +
> + WARN_ON_ONCE(ctx->file_table.data.nr);
> + ctx->file_table = new_file_table;
> + io_file_table_set_alloc_range(ctx, 0, ctx->file_table.data.nr);
> + return 0;
> +
> +out:
> + /* Error Path: Safely destroy whatever we partially built */
> + io_free_file_tables(ctx, &new_file_table);
> + return -ENOMEM;
> +}
> +
> +int io_register_clone_files(struct io_ring_ctx *ctx, void __user *arg)
> +{
> + struct io_uring_clone_files clone_arg;
> + struct io_ring_ctx *src_ctx;
> + bool registered_src;
> + struct file *file;
> + int ret;
> +
> + if (copy_from_user(&clone_arg, arg, sizeof(clone_arg)))
> + return -EFAULT;
> + if (clone_arg.flags &
> + ~(IORING_REGISTER_SRC_REGISTERED | IORING_REGISTER_DST_REPLACE))
> + return -EINVAL;
> +
> + if (memchr_inv(clone_arg.pad, 0, sizeof(clone_arg.pad)))
> + return -EINVAL;
> +
> + registered_src = !!(clone_arg.flags & IORING_REGISTER_SRC_REGISTERED);
> + file = io_uring_ctx_get_file(clone_arg.src_fd, registered_src);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> +
> + src_ctx = file->private_data;
> + /* Same ring clone is not allowed */
> + if (src_ctx == ctx) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + mutex_unlock(&ctx->uring_lock);
> + lock_two_rings(ctx, src_ctx);
> +
> + ret = io_clone_files(ctx, src_ctx, &clone_arg);
> +
> + mutex_unlock(&src_ctx->uring_lock);
> +
> +out:
> + if (!registered_src)
> + fput(file);
> + return ret;
> +}
> +
> void io_vec_free(struct iou_vec *iv)
> {
> if (!iv->iovec)
> diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
> index 44e3386f7c1c..32f5c47c46af 100644
> --- a/io_uring/rsrc.h
> +++ b/io_uring/rsrc.h
> @@ -75,6 +75,7 @@ int io_prep_reg_iovec(struct io_kiocb *req, struct iou_vec
> *iv,
> const struct iovec __user *uvec, size_t uvec_segs);
>
> int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg);
> +int io_register_clone_files(struct io_ring_ctx *ctx, void __user *arg);
> int io_sqe_buffers_unregister(struct io_ring_ctx *ctx);
> int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
> unsigned int nr_args, u64 __user *tags);
> --
> 2.54.0
>
--
Gabriel Krisman Bertazi
