| Message ID | 20240529-fuse-uring-for-6-9-rfc2-out-v1-11-d149476b1d65@ddn.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: fuse-over-io-uring | expand |
On Wed, May 29, 2024 at 08:00:46PM +0200, Bernd Schubert wrote: > This adds support to existing fuse copy code to copy > from/to the ring buffer. The ring buffer is here mmaped > shared between kernel and userspace. > > This also fuse_ prefixes the copy_out_args function > > Signed-off-by: Bernd Schubert <bschubert@ddn.com> > --- > fs/fuse/dev.c | 60 ++++++++++++++++++++++++++++++---------------------- > fs/fuse/fuse_dev_i.h | 38 +++++++++++++++++++++++++++++++++ > 2 files changed, 73 insertions(+), 25 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 05a87731b5c3..a7d26440de39 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -637,21 +637,7 @@ static int unlock_request(struct fuse_req *req) > return err; > } > > -struct fuse_copy_state { > - int write; > - struct fuse_req *req; > - struct iov_iter *iter; > - struct pipe_buffer *pipebufs; > - struct pipe_buffer *currbuf; > - struct pipe_inode_info *pipe; > - unsigned long nr_segs; > - struct page *pg; > - unsigned len; > - unsigned offset; > - unsigned move_pages:1; > -}; > - > -static void fuse_copy_init(struct fuse_copy_state *cs, int write, > +void fuse_copy_init(struct fuse_copy_state *cs, int write, > struct iov_iter *iter) > { > memset(cs, 0, sizeof(*cs)); > @@ -662,6 +648,7 @@ static void fuse_copy_init(struct fuse_copy_state *cs, int write, > /* Unmap and put previous page of userspace buffer */ > static void fuse_copy_finish(struct fuse_copy_state *cs) > { > + Extraneous newline. > if (cs->currbuf) { > struct pipe_buffer *buf = cs->currbuf; > > @@ -726,6 +713,10 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) > cs->pipebufs++; > cs->nr_segs++; > } > + } else if (cs->is_uring) { > + if (cs->ring.offset > cs->ring.buf_sz) > + return -ERANGE; > + cs->len = cs->ring.buf_sz - cs->ring.offset; > } else { > size_t off; > err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off); > @@ -744,21 +735,35 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) > 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 = NULL; > + void *buf; > + > + if (cs->is_uring) { > + buf = cs->ring.buf + cs->ring.offset; > + cs->ring.offset += ncpy; > + > + } else { > + pgaddr = kmap_local_page(cs->pg); > + buf = pgaddr + cs->offset; > + } > > if (cs->write) > memcpy(buf, *val, ncpy); > else > memcpy(*val, buf, ncpy); > > - kunmap_local(pgaddr); > + if (pgaddr) > + kunmap_local(pgaddr); > + > *val += ncpy; > } > *size -= ncpy; > cs->len -= ncpy; > cs->offset += ncpy; > + Extraneous newline. Once those nits are fixed you can add Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On 5/30/24 21:59, Josef Bacik wrote: > On Wed, May 29, 2024 at 08:00:46PM +0200, Bernd Schubert wrote: >> This adds support to existing fuse copy code to copy >> from/to the ring buffer. The ring buffer is here mmaped >> shared between kernel and userspace. >> >> This also fuse_ prefixes the copy_out_args function >> >> Signed-off-by: Bernd Schubert <bschubert@ddn.com> >> --- >> fs/fuse/dev.c | 60 ++++++++++++++++++++++++++++++---------------------- >> fs/fuse/fuse_dev_i.h | 38 +++++++++++++++++++++++++++++++++ >> 2 files changed, 73 insertions(+), 25 deletions(-) >> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >> index 05a87731b5c3..a7d26440de39 100644 >> --- a/fs/fuse/dev.c >> +++ b/fs/fuse/dev.c >> @@ -637,21 +637,7 @@ static int unlock_request(struct fuse_req *req) >> return err; >> } >> >> -struct fuse_copy_state { >> - int write; >> - struct fuse_req *req; >> - struct iov_iter *iter; >> - struct pipe_buffer *pipebufs; >> - struct pipe_buffer *currbuf; >> - struct pipe_inode_info *pipe; >> - unsigned long nr_segs; >> - struct page *pg; >> - unsigned len; >> - unsigned offset; >> - unsigned move_pages:1; >> -}; >> - >> -static void fuse_copy_init(struct fuse_copy_state *cs, int write, >> +void fuse_copy_init(struct fuse_copy_state *cs, int write, >> struct iov_iter *iter) >> { >> memset(cs, 0, sizeof(*cs)); >> @@ -662,6 +648,7 @@ static void fuse_copy_init(struct fuse_copy_state *cs, int write, >> /* Unmap and put previous page of userspace buffer */ >> static void fuse_copy_finish(struct fuse_copy_state *cs) >> { >> + > > Extraneous newline. > >> if (cs->currbuf) { >> struct pipe_buffer *buf = cs->currbuf; >> >> @@ -726,6 +713,10 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) >> cs->pipebufs++; >> cs->nr_segs++; >> } >> + } else if (cs->is_uring) { >> + if (cs->ring.offset > cs->ring.buf_sz) >> + return -ERANGE; >> + cs->len = cs->ring.buf_sz - cs->ring.offset; >> } else { >> size_t off; >> err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off); >> @@ -744,21 +735,35 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) >> 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 = NULL; >> + void *buf; >> + >> + if (cs->is_uring) { >> + buf = cs->ring.buf + cs->ring.offset; >> + cs->ring.offset += ncpy; >> + >> + } else { >> + pgaddr = kmap_local_page(cs->pg); >> + buf = pgaddr + cs->offset; >> + } >> >> if (cs->write) >> memcpy(buf, *val, ncpy); >> else >> memcpy(*val, buf, ncpy); >> >> - kunmap_local(pgaddr); >> + if (pgaddr) >> + kunmap_local(pgaddr); >> + >> *val += ncpy; >> } >> *size -= ncpy; >> cs->len -= ncpy; >> cs->offset += ncpy; >> + > > Extraneous newline. > > Once those nits are fixed you can add > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks again for your reviews! I won't add this for now, as there are too many changes after removing mmap. Thanks, Bernd
On 5/30/24 21:59, Josef Bacik wrote: > On Wed, May 29, 2024 at 08:00:46PM +0200, Bernd Schubert wrote: >> This adds support to existing fuse copy code to copy >> from/to the ring buffer. The ring buffer is here mmaped >> shared between kernel and userspace. >> >> This also fuse_ prefixes the copy_out_args function >> >> Signed-off-by: Bernd Schubert <bschubert@ddn.com> >> --- >> fs/fuse/dev.c | 60 ++++++++++++++++++++++++++++++---------------------- >> fs/fuse/fuse_dev_i.h | 38 +++++++++++++++++++++++++++++++++ >> 2 files changed, 73 insertions(+), 25 deletions(-) >> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >> index 05a87731b5c3..a7d26440de39 100644 >> --- a/fs/fuse/dev.c >> +++ b/fs/fuse/dev.c >> @@ -637,21 +637,7 @@ static int unlock_request(struct fuse_req *req) >> return err; >> } >> >> -struct fuse_copy_state { >> - int write; >> - struct fuse_req *req; >> - struct iov_iter *iter; >> - struct pipe_buffer *pipebufs; >> - struct pipe_buffer *currbuf; >> - struct pipe_inode_info *pipe; >> - unsigned long nr_segs; >> - struct page *pg; >> - unsigned len; >> - unsigned offset; >> - unsigned move_pages:1; >> -}; >> - >> -static void fuse_copy_init(struct fuse_copy_state *cs, int write, >> +void fuse_copy_init(struct fuse_copy_state *cs, int write, >> struct iov_iter *iter) >> { >> memset(cs, 0, sizeof(*cs)); >> @@ -662,6 +648,7 @@ static void fuse_copy_init(struct fuse_copy_state *cs, int write, >> /* Unmap and put previous page of userspace buffer */ >> static void fuse_copy_finish(struct fuse_copy_state *cs) >> { >> + > > Extraneous newline. > >> if (cs->currbuf) { >> struct pipe_buffer *buf = cs->currbuf; >> >> @@ -726,6 +713,10 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) >> cs->pipebufs++; >> cs->nr_segs++; >> } >> + } else if (cs->is_uring) { >> + if (cs->ring.offset > cs->ring.buf_sz) >> + return -ERANGE; >> + cs->len = cs->ring.buf_sz - cs->ring.offset; >> } else { >> size_t off; >> err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off); >> @@ -744,21 +735,35 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) >> 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 = NULL; >> + void *buf; >> + >> + if (cs->is_uring) { >> + buf = cs->ring.buf + cs->ring.offset; >> + cs->ring.offset += ncpy; >> + >> + } else { >> + pgaddr = kmap_local_page(cs->pg); >> + buf = pgaddr + cs->offset; >> + } >> >> if (cs->write) >> memcpy(buf, *val, ncpy); >> else >> memcpy(*val, buf, ncpy); >> >> - kunmap_local(pgaddr); >> + if (pgaddr) >> + kunmap_local(pgaddr); >> + >> *val += ncpy; >> } >> *size -= ncpy; >> cs->len -= ncpy; >> cs->offset += ncpy; >> + > > Extraneous newline. > > Once those nits are fixed you can add > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks again for your reviews! I won't add this for now, as there are too many changes after removing mmap. Thanks, Bernd
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 05a87731b5c3..a7d26440de39 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -637,21 +637,7 @@ static int unlock_request(struct fuse_req *req) return err; } -struct fuse_copy_state { - int write; - struct fuse_req *req; - struct iov_iter *iter; - struct pipe_buffer *pipebufs; - struct pipe_buffer *currbuf; - struct pipe_inode_info *pipe; - unsigned long nr_segs; - struct page *pg; - unsigned len; - unsigned offset; - unsigned move_pages:1; -}; - -static void fuse_copy_init(struct fuse_copy_state *cs, int write, +void fuse_copy_init(struct fuse_copy_state *cs, int write, struct iov_iter *iter) { memset(cs, 0, sizeof(*cs)); @@ -662,6 +648,7 @@ static void fuse_copy_init(struct fuse_copy_state *cs, int write, /* Unmap and put previous page of userspace buffer */ static void fuse_copy_finish(struct fuse_copy_state *cs) { + if (cs->currbuf) { struct pipe_buffer *buf = cs->currbuf; @@ -726,6 +713,10 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) cs->pipebufs++; cs->nr_segs++; } + } else if (cs->is_uring) { + if (cs->ring.offset > cs->ring.buf_sz) + return -ERANGE; + cs->len = cs->ring.buf_sz - cs->ring.offset; } else { size_t off; err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off); @@ -744,21 +735,35 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) 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 = NULL; + void *buf; + + if (cs->is_uring) { + buf = cs->ring.buf + cs->ring.offset; + cs->ring.offset += ncpy; + + } else { + pgaddr = kmap_local_page(cs->pg); + buf = pgaddr + cs->offset; + } if (cs->write) memcpy(buf, *val, ncpy); else memcpy(*val, buf, ncpy); - kunmap_local(pgaddr); + if (pgaddr) + kunmap_local(pgaddr); + *val += ncpy; } *size -= ncpy; cs->len -= ncpy; cs->offset += ncpy; + return ncpy; } @@ -1006,9 +1011,9 @@ static int fuse_copy_one(struct fuse_copy_state *cs, void *val, unsigned size) } /* Copy request arguments to/from userspace buffer */ -static int fuse_copy_args(struct fuse_copy_state *cs, unsigned numargs, - unsigned argpages, struct fuse_arg *args, - int zeroing) +int fuse_copy_args(struct fuse_copy_state *cs, unsigned int numargs, + unsigned int argpages, struct fuse_arg *args, + int zeroing) { int err = 0; unsigned i; @@ -1873,10 +1878,15 @@ static struct fuse_req *request_find(struct fuse_pqueue *fpq, u64 unique) return NULL; } -static int copy_out_args(struct fuse_copy_state *cs, struct fuse_args *args, - unsigned nbytes) +int fuse_copy_out_args(struct fuse_copy_state *cs, struct fuse_args *args, + unsigned int nbytes) { - unsigned reqsize = sizeof(struct fuse_out_header); + + unsigned int reqsize = 0; + + /* Uring has the out header outside of args */ + if (!cs->is_uring) + reqsize = sizeof(struct fuse_out_header); reqsize += fuse_len_args(args->out_numargs, args->out_args); @@ -1976,7 +1986,7 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud, if (oh.error) err = nbytes != sizeof(oh) ? -EINVAL : 0; else - err = copy_out_args(cs, req->args, nbytes); + err = fuse_copy_out_args(cs, req->args, nbytes); fuse_copy_finish(cs); spin_lock(&fpq->lock); diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h index e6289bafb788..f3e69ab4c2be 100644 --- a/fs/fuse/fuse_dev_i.h +++ b/fs/fuse/fuse_dev_i.h @@ -13,6 +13,36 @@ #define FUSE_INT_REQ_BIT (1ULL << 0) #define FUSE_REQ_ID_STEP (1ULL << 1) +struct fuse_arg; +struct fuse_args; + +struct fuse_copy_state { + int write; + struct fuse_req *req; + struct iov_iter *iter; + struct pipe_buffer *pipebufs; + struct pipe_buffer *currbuf; + struct pipe_inode_info *pipe; + unsigned long nr_segs; + struct page *pg; + unsigned int len; + unsigned int offset; + unsigned int move_pages:1, is_uring:1; + struct { + /* pointer into the ring buffer */ + char *buf; + + /* for copy to the ring request buffer, the buffer size - must + * not be exceeded, for copy from the ring request buffer, + * the size filled in by user space + */ + unsigned int buf_sz; + + /* offset within buf while it is copying from/to the buf */ + unsigned int offset; + } ring; +}; + static inline struct fuse_dev *fuse_get_dev(struct file *file) { /* @@ -24,6 +54,14 @@ static inline struct fuse_dev *fuse_get_dev(struct file *file) void fuse_dev_end_requests(struct list_head *head); +void fuse_copy_init(struct fuse_copy_state *cs, int write, + struct iov_iter *iter); +int fuse_copy_args(struct fuse_copy_state *cs, unsigned int numargs, + unsigned int argpages, struct fuse_arg *args, + int zeroing); +int fuse_copy_out_args(struct fuse_copy_state *cs, struct fuse_args *args, + unsigned int nbytes); + #endif
This adds support to existing fuse copy code to copy from/to the ring buffer. The ring buffer is here mmaped shared between kernel and userspace. This also fuse_ prefixes the copy_out_args function Signed-off-by: Bernd Schubert <bschubert@ddn.com> --- fs/fuse/dev.c | 60 ++++++++++++++++++++++++++++++---------------------- fs/fuse/fuse_dev_i.h | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 25 deletions(-)