Re: [PATCH v4 18/25] fuse: support buffer copying for kernel addresses

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Jan 28, 2026 at 1:14 PM Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
>
> On 1/28/26 01:23, Joanne Koong wrote:
> > On Tue, Jan 27, 2026 at 3:40 PM Bernd Schubert <bschubert@ddn.com> wrote:
> >>
> >> On 1/17/26 00:30, Joanne Koong wrote:
> >>> This is a preparatory patch needed to support kernel-managed ring
> >>> buffers in fuse-over-io-uring. For kernel-managed ring buffers, we get
> >>> the vmapped address of the buffer which we can directly use.
> >>>
> >>> Currently, buffer copying in fuse only supports extracting underlying
> >>> pages from an iov iter and kmapping them. This commit allows buffer
> >>> copying to work directly on a kaddr.
> >>>
> >>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> >>> ---
> >>>  fs/fuse/dev.c        | 23 +++++++++++++++++------
> >>>  fs/fuse/fuse_dev_i.h |  7 ++++++-
> >>>  2 files changed, 23 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> >>> index 6d59cbc877c6..ceb5d6a553c0 100644
> >>> --- a/fs/fuse/dev.c
> >>> +++ b/fs/fuse/dev.c
> >>> @@ -848,6 +848,9 @@ void fuse_copy_init(struct fuse_copy_state *cs, bool write,
> >>>  /* Unmap and put previous page of userspace buffer */
> >>>  void fuse_copy_finish(struct fuse_copy_state *cs)
> >>>  {
> >>> +     if (cs->is_kaddr)
> >>> +             return;
> >>> +
> >>>       if (cs->currbuf) {
> >>>               struct pipe_buffer *buf = cs->currbuf;
> >>>
> >>> @@ -873,6 +876,9 @@ static int fuse_copy_fill(struct fuse_copy_state *cs)
> >>>       struct page *page;
> >>>       int err;
> >>>
> >>> +     if (cs->is_kaddr)
> >>> +             return 0;
> >>> +
> >>>       err = unlock_request(cs->req);
> >>>       if (err)
> >>>               return err;
> >>> @@ -931,15 +937,20 @@ static int fuse_copy_do(struct fuse_copy_state *cs, void **val, unsigned *size)
> >>>  {
> >>>       unsigned ncpy = min(*size, cs->len);
> >>>       if (val) {
> >>> -             void *pgaddr = kmap_local_page(cs->pg);
> >>> -             void *buf = pgaddr + cs->offset;
> >>> +             void *pgaddr, *buf;
> >>> +             if (!cs->is_kaddr) {
> >>> +                     pgaddr = kmap_local_page(cs->pg);
> >>> +                     buf = pgaddr + cs->offset;
> >>> +             } else {
> >>> +                     buf = cs->kaddr + cs->offset;
> >>> +             }
> >>>
> >>>               if (cs->write)
> >>>                       memcpy(buf, *val, ncpy);
> >>>               else
> >>>                       memcpy(*val, buf, ncpy);
> >>> -
> >>> -             kunmap_local(pgaddr);
> >>> +             if (!cs->is_kaddr)
> >>> +                     kunmap_local(pgaddr);
> >>>               *val += ncpy;
> >>>       }
> >>>       *size -= ncpy;
> >>> @@ -1127,7 +1138,7 @@ static int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
> >>>       }
> >>>
> >>>       while (count) {
> >>> -             if (cs->write && cs->pipebufs && folio) {
> >>> +             if (cs->write && cs->pipebufs && folio && !cs->is_kaddr) {
> >>>                       /*
> >>>                        * Can't control lifetime of pipe buffers, so always
> >>>                        * copy user pages.
> >>> @@ -1139,7 +1150,7 @@ static int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
> >>>                       } else {
> >>>                               return fuse_ref_folio(cs, folio, offset, count);
> >>>                       }
> >>> -             } else if (!cs->len) {
> >>> +             } else if (!cs->len && !cs->is_kaddr) {
> >>>                       if (cs->move_folios && folio &&
> >>>                           offset == 0 && count == size) {
> >>>                               err = fuse_try_move_folio(cs, foliop);
> >>> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> >>> index 134bf44aff0d..aa1d25421054 100644
> >>> --- a/fs/fuse/fuse_dev_i.h
> >>> +++ b/fs/fuse/fuse_dev_i.h
> >>> @@ -28,12 +28,17 @@ struct fuse_copy_state {
> >>>       struct pipe_buffer *currbuf;
> >>>       struct pipe_inode_info *pipe;
> >>>       unsigned long nr_segs;
> >>> -     struct page *pg;
> >>> +     union {
> >>> +             struct page *pg;
> >>> +             void *kaddr;
> >>> +     };
> >>>       unsigned int len;
> >>>       unsigned int offset;
> >>>       bool write:1;
> >>>       bool move_folios:1;
> >>>       bool is_uring:1;
> >>> +     /* if set, use kaddr; otherwise use pg */
> >>> +     bool is_kaddr:1;
> >>>       struct {
> >>>               unsigned int copied_sz; /* copied size into the user buffer */
> >>>       } ring;
> >>
> >>
> >> I'm confused here, how cs->len will get initialized. So far that was
> >> done from fuse_copy_fill?
> >
> > With kaddrs, cs->len is initialized when the copy state is set up (in
> > setup_fuse_copy_state()) before we do any copying to/from the ring.
> > The changes for that are in the later patch that adds the ringbuffer
> > logic ("fuse: add io-uring kernel-managed buffer ring"). The kaddr and
> > len correspond to the address and length of the buffer that was
> > selected from the ring buffer (in fuse_uring_select_buffer()).
>
>
>
> Maybe we could add a sanity check into fuse_copy_do() or even into
> fuse_copy_fill in the cs->is_kaddr condition that cs->len is > 0?

I will add a WARN_ON for this in the next version. Thanks for reviewing this.

Thanks,
Joanne
>
> Otherwise looks good.
>
> Reviewed-by: Bernd Schubert <bernd@bsbernd.com>