Re: [PATCH v6 5/6] fuse: add zero-copy over io-uring

On Wed, Aug 12, 2026 at 3:30 PM Bernd Schubert <[email protected]> wrote:
>
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index d50792162d8b..3d1910a9db52 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -1247,11 +1247,20 @@ int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
> >
> >       if (folio) {
> >               size = folio_size(folio);
> > -             if (zeroing && count < size)
> > -                     folio_zero_range(folio, 0, size);
> > +             if (zeroing && count < size) {
>
> This function is quite complex and long, maybe a comment here
>
> /* io-uring zery-copy */
>
> > +                     if (cs->skip_folio_copy) {
> > +                             if (offset)
> > +                                     folio_zero_range(folio, 0, offset);
> > +                             if (offset + count < size)
> > +                                     folio_zero_range(folio, offset + count,
> > +                                                      size - offset - count);
>
> Maybe
> folio_zero_segments(folio, 0, offset, offset + count, size);

Nice, didn't realize folio_zero_segments existed. I'll use this API.

>
> > +                     } else {
> > +                             folio_zero_range(folio, 0, size);
> > +                     }
> > +             }
> >       }
> >
> > -     while (count) {
> > +     while (!cs->skip_folio_copy && count) {
>
> For readibility and also to let the compiler know that
> cs->skip_folio_copy never changes, maybe at the top of the function
>
> const bool uring_zero_copy = cs->skip_folio_copy;
>
> and then
>
> while (!uring_zero_copy && count) {
>
>
> Also would avoid the comment I asked for above.

Sounds good, will do.

>
> >               if (cs->write && cs->pipebufs && folio) {
> >                       /*
> >                        * Can't control lifetime of pipe buffers, so always
> > @@ -1344,6 +1353,10 @@ int fuse_copy_args(struct fuse_copy_state *cs, unsigned numargs,
> >       for (i = 0; !err && i < numargs; i++)  {
> >               struct fuse_arg *arg = &args[i];
> >               if (i == numargs - 1 && argpages)
> > +                     /*
> > +                      * if cs->skip_folio_copy is set, this just does any
> > +                      * needed zeroing. No copying is involved.
> > +                      */
> >                       err = fuse_copy_folios(cs, arg->size, zeroing);
> >               else
> >                       err = fuse_copy_one(cs, arg->value, arg->size);
> > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> > index ed7e783bd68d..d7d517497a1e 100644
> > --- a/fs/fuse/dev_uring.c
> > +++ b/fs/fuse/dev_uring.c
> > @@ -23,6 +23,7 @@ MODULE_PARM_DESC(enable_uring,
> >  #define FUSE_URING_IOV_PAYLOAD 1
> >
> >  #define FUSE_URING_ADD_BUFPOOL_FLAGS (FUSE_URING_REGISTERED_BUFPOOL)
> > +#define FUSE_URING_ADD_QUEUE_FLAGS   (FUSE_URING_ZERO_COPY)
> >
> >  bool fuse_uring_enabled(void)
> >  {
> > @@ -33,6 +34,11 @@ struct fuse_uring_pdu {
> >       struct fuse_ring_ent *ent;
> >  };
> >
> > +struct fuse_zero_copy_bvs {
> > +     unsigned int nr_bvs;
> > +     struct bio_vec bvs[];
> > +};
> > +
> >  static const struct fuse_iqueue_ops fuse_io_uring_ops;
> >
> >  enum fuse_uring_header_type {
> > @@ -115,8 +121,36 @@ static void fuse_uring_flush_bg(struct fuse_ring_queue *queue)
> >       }
> >  }
> >
> > +static bool can_zero_copy_req(struct fuse_ring_ent *ent, struct fuse_req *req)
> > +{
> > +     struct fuse_args *args = req->args;
> > +
> > +     if (!ent->queue->zero_copy || !args->zero_copy)
> > +             return false;
> > +
> > +     if (args->opcode != FUSE_READ && args->opcode != FUSE_WRITE)
> > +             return false;
> > +
> > +     return args->in_pages || args->out_pages;
> > +}
> > +
> > +static void zero_copy_unregister(struct io_uring_cmd *cmd,
> > +                              struct fuse_ring_ent *ent,
> > +                              unsigned int issue_flags)
> > +{
> > +     if (ent->zero_copied) {
> > +             int err = io_buffer_unregister(cmd, ent->zero_copy_index,
> > +                                            issue_flags);
> > +
> > +             if (err)
> > +                     pr_warn_ratelimited("qid=%d zero-copy unregister failed: %d\n",
> > +                                         ent->queue->qid, err);
> > +             ent->zero_copied = false;
> > +     }
> > +}
> > +
> >  static void fuse_uring_req_end(struct fuse_ring_ent *ent, struct fuse_req *req,
> > -                            int error)
> > +                            int error, unsigned int issue_flags)
> >  {
> >       struct fuse_ring_queue *queue = ent->queue;
> >       struct fuse_ring *ring = queue->ring;
> > @@ -136,6 +170,8 @@ static void fuse_uring_req_end(struct fuse_ring_ent *ent, struct fuse_req *req,
> >
> >       spin_unlock(&queue->lock);
> >
> > +     zero_copy_unregister(ent->cmd, ent, issue_flags);
> > +
> >       if (error)
> >               req->out.h.error = error;
> >
> > @@ -311,7 +347,7 @@ void fuse_uring_conn_init(struct fuse_chan *fch)
> >  }
> >
> >  static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> > -                                                    int qid,
> > +                                                    int qid, bool zero_copy,
> >                                                      bool fail_if_exists)
> >  {
> >       struct fuse_chan *fch = ring->chan;
> > @@ -330,6 +366,7 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> >       queue->qid = qid;
> >       queue->ring = ring;
> >       spin_lock_init(&queue->lock);
> > +     queue->zero_copy = zero_copy;
> >
> >       INIT_LIST_HEAD(&queue->ent_avail_queue);
> >       INIT_LIST_HEAD(&queue->ent_commit_queue);
> > @@ -715,6 +752,9 @@ static int setup_fuse_copy_state(struct fuse_copy_state *cs,
> >
> >       fuse_copy_init(cs, dir == ITER_DEST, iter);
> >
> > +     if (ent->zero_copied)
> > +             cs->skip_folio_copy = true;
> > +
> >       cs->is_uring = true;
> >       cs->req = req;
> >
> > @@ -746,6 +786,62 @@ static int fuse_uring_copy_from_ring(struct fuse_req *req,
> >       return err;
> >  }
> >
> > +static void fuse_zero_copy_release(void *priv)
> > +{
> > +     struct fuse_zero_copy_bvs *zc_bvs = priv;
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < zc_bvs->nr_bvs; i++)
> > +             folio_put(page_folio(zc_bvs->bvs[i].bv_page));
> > +
> > +     kfree(zc_bvs);
>
> kvfree (see below)?
>
> > +}
> > +
> > +static int fuse_uring_set_up_zero_copy(struct fuse_ring_ent *ent,
> > +                                    struct fuse_req *req,
> > +                                    unsigned int issue_flags)
> > +{
> > +     struct fuse_args_pages *ap;
> > +     int err, i, ddir = 0;
> > +     struct fuse_zero_copy_bvs *zc_bvs;
> > +     struct bio_vec *bvs;
> > +
> > +     /* out_pages indicates a read, in_pages indicates a write */
> > +     if (req->args->out_pages)
> > +             ddir |= IO_BUF_DEST;
> > +     if (req->args->in_pages)
> > +             ddir |= IO_BUF_SOURCE;
> > +
> > +     ap = container_of(req->args, typeof(*ap), args);
> > +
> > +     zc_bvs = kmalloc(struct_size(zc_bvs, bvs, ap->num_folios),
> > +                      GFP_KERNEL_ACCOUNT);
>
> Assuming one folio per page and 1MB max_nr_pages:
>
> struct_size(zc_bvs, bvs, n) == 8 + 16 * 256 = 4104
>
> Shouldn't this be kvmalloc()?

I'm not sure what the upper limit is for a kmalloc, but I'll change
this to kvmalloc().

>
>
> (otherwise looks good, will do another review in the morning, though).

Thanks for reviewing the patches!

Thanks,
Joanne