>> + 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? Yes, sorry for the silly mistakes, thanks for catching this! > 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 :) Okay, Yes I will wait for feedback from Jens and send the changes together Thanks for the review. Regards, Harshal Chavan On Fri, Jun 26, 2026 at 2:03 AM Gabriel Krisman Bertazi <[email protected]> wrote: > > 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
