OpenVZ Forum - RDF feed
type: channel
items: [PATCH v2 00/11] fuse: optimize scatter-gather direct IO
items: [PATCH 01/11] fuse: general infrastructure for pages[] of variable size
items: [PATCH 02/11] fuse: categorize fuse_get_req()
items: [PATCH 03/11] fuse: rework fuse_retrieve()
items: [PATCH 04/11] fuse: rework fuse_readpages()
items: [PATCH 05/11] fuse: rework fuse_perform_write()
items: [PATCH 06/11] fuse: rework fuse_do_ioctl()
items: [PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req
items: [PATCH 08/11] fuse: use req-&gt;page_descs[] for argpages cases
items: [PATCH 09/11] fuse: pass iov[] to fuse_get_user_pages()
items: [PATCH 10/11] fuse: optimize fuse_get_user_pages()
items: [PATCH 11/11] fuse: optimize __fuse_direct_io()
items: [PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req (v2)
items: [PATCH 10/11] fuse: optimize fuse_get_user_pages() - v2
items: Re: [PATCH v2 00/11] fuse: optimize scatter-gather direct IO
items: Re: [PATCH 01/11] fuse: general infrastructure for pages[] of variable size
items: Re: [PATCH 02/11] fuse: categorize fuse_get_req()
items: Re: [PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req (v2)
items: Re: [PATCH 08/11] fuse: use req-&gt;page_descs[] for argpages cases
items: Re: [PATCH 09/11] fuse: pass iov[] to fuse_get_user_pages()
items: Re: [PATCH 10/11] fuse: optimize fuse_get_user_pages() - v2

[PATCH v2 00/11] fuse: optimize scatter-gather direct IO
type: item
description: Hi, Existing fuse implementation processes scatter-gather direct IO in suboptimal way: fuse_direct_IO passes iovec[] to fuse_loop_dio and the latter calls fuse_direct_read/write for each iovec from iovec[] array. Thus we have as many submitted fuse-requests as the number of elements in iovec[] array. This is pure waste of resources and affects performance negatively especially for the case of many small chunks (e.g. page-size) packed in one iovec[] array. The patch-set amends situation in a natural way: let's simply pack as many iovec[] segments to every fuse-request as possible. To estimate performance improvement I used slightly modified fusexmp over tmpfs (clearing O_DIRECT bit from fi->flags in xmp_open). The test opened a file with O_DIRECT, then called readv/writev in a loop. An iovec[] for readv/writev consisted of 32 segments of 4K each. The throughput on some commodity (rather feeble) server was (in MB/sec): original / patched writev: ~107 / ~480 readv: ~114 / ~569 We're exploring possibility to use fuse for our own distributed storage implementation and big iovec[] arrays of many page-size chunks is typical use-case for device virtualization thread performing i/o on behalf of virtual-machine it serves. Changed in v2: - inline array of page pointers req->pages[] is replaced with dynamically allocated one; the number of elements is calculated a bit more intelligently than being equal to FUSE_MAX_PAGES_PER_REQ; this is done for the sake of memory economy. - a dynamically allocated array of so-called 'page descriptors' - an offset in page plus the length of fragment - is added to fuse_req; this is done to simplify processing fuse requests covering several iov-s. Thanks, Maxim --- Maxim Patlasov (11): fuse: general infrastructure for pages[] of variable size fuse: categorize fuse_get_req() fuse: rework fuse_retrieve() fuse: rework fuse_readpages() fuse: rework fuse_perform_write() fuse: rework fuse_do_ioctl() fuse: add per-page descriptor <offset, length> to fu…
creator: Maxim Patlasov
date: 2012-09-19T16:31:10-00:00

[PATCH 01/11] fuse: general infrastructure for pages[] of variable size
type: item
description: The patch removes inline array of FUSE_MAX_PAGES_PER_REQ page pointers from fuse_req. Instead of that, req->pages may now point either to small inline array or to an array allocated dynamically. This essentially means that all callers of fuse_request_alloc[_nofs] should pass the number of pages needed explicitly. The patch doesn't make any logic changes. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/dev.c | 46 +++++++++++++++++++++++++++++++++------------- fs/fuse/file.c | 4 ++-- fs/fuse/fuse_i.h | 15 ++++++++++++--- fs/fuse/inode.c | 4 ++-- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 7df2b5e..1b5fc73 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -34,34 +34,54 @@ static struct fuse_conn *fuse_get_conn(struct file *file) return file->private_data; } -static void fuse_request_init(struct fuse_req *req) +static void fuse_request_init(struct fuse_req *req, struct page **pages, + unsigned npages) { memset(req, 0, sizeof(*req)); INIT_LIST_HEAD(&req->list); INIT_LIST_HEAD(&req->intr_entry); init_waitqueue_head(&req->waitq); atomic_set(&req->count, 1); + req->pages = pages; + req->max_pages = npages; } -struct fuse_req *fuse_request_alloc(void) +static struct fuse_req *__fuse_request_alloc(int npages, gfp_t flags) { - struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, GFP_KERNEL); - if (req) - fuse_request_init(req); + struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); + if (req) { + struct page **pages; + + if (npages <= FUSE_REQ_INLINE_PAGES) + pages = req->inline_pages; + else + pages = kmalloc(sizeof(struct page *) * npages, flags); + + if (!pages) { + kmem_cache_free(fuse_req_cachep, req); + return NULL; + } + + fuse_request_init(req, pages, npages); + } return req; } + +struct fuse_req *fuse_request_alloc(int npages) +{ + return __fuse_request_alloc(npages, GFP_KERNEL); +} EXPORT_SYMBOL_GPL(fuse_request_alloc); -struct fuse_req *fuse_request_alloc_nofs(void) +struct f…
creator: Maxim Patlasov
date: 2012-09-19T16:31:45-00:00

[PATCH 02/11] fuse: categorize fuse_get_req()
type: item
description: The patch categorizes all fuse_get_req() invocations into two categories: - fuse_get_req_nopages(fc) - when caller doesn't care about req->pages - fuse_get_req(fc, n) - when caller need n page pointers (n > 0) Adding fuse_get_req_nopages() helps to avoid numerous fuse_get_req(fc, 0) scattered over code. Now it's clear from the first glance when a caller need fuse_req with page pointers. The patch doesn't make any logic changes. In multi-page case, it silly allocates array of FUSE_MAX_PAGES_PER_REQ page pointers. This will be amended by future patches. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/cuse.c | 2 +- fs/fuse/dev.c | 11 ++++++----- fs/fuse/dir.c | 38 +++++++++++++++++++------------------- fs/fuse/file.c | 30 +++++++++++++++--------------- fs/fuse/fuse_i.h | 17 ++++++++++++++--- fs/fuse/inode.c | 2 +- 6 files changed, 56 insertions(+), 44 deletions(-) diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c index 3426521..2f1d5dd 100644 --- a/fs/fuse/cuse.c +++ b/fs/fuse/cuse.c @@ -411,7 +411,7 @@ static int cuse_send_init(struct cuse_conn *cc) BUILD_BUG_ON(CUSE_INIT_INFO_MAX > PAGE_SIZE); - req = fuse_get_req(fc); + req = fuse_get_req(fc, 1); if (IS_ERR(req)) { rc = PTR_ERR(req); goto err; diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 1b5fc73..3ad5570 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -117,7 +117,7 @@ static void fuse_req_init_context(struct fuse_req *req) req->in.h.pid = current->pid; } -struct fuse_req *fuse_get_req(struct fuse_conn *fc) +struct fuse_req *fuse_get_req(struct fuse_conn *fc, int npages) { struct fuse_req *req; sigset_t oldset; @@ -136,7 +136,7 @@ struct fuse_req *fuse_get_req(struct fuse_conn *fc) if (!fc->connected) goto out; - req = fuse_request_alloc(FUSE_MAX_PAGES_PER_REQ); + req = fuse_request_alloc(npages); err = -ENOMEM; if (!req) goto out; @@ -207,13 +207,14 @@ static void put_reserved_req(struct fuse_conn *fc, struct fuse_req *req) * filesystem should not have it's own file open. If deadlock is * intent…
creator: Maxim Patlasov
date: 2012-09-19T16:31:54-00:00

[PATCH 03/11] fuse: rework fuse_retrieve()
type: item
description: The patch reworks fuse_retrieve() to allocate only so many page pointers as needed. The core part of the patch is the following calculation: num_pages = (num + offset + PAGE_SIZE - 1) >> PAGE_SHIFT; (thanks Miklos for formula). All other changes are mostly shuffling lines. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/dev.c | 25 +++++++++++++++---------- 1 files changed, 15 insertions(+), 10 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 3ad5570..b241a7d 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1563,13 +1563,24 @@ static int fuse_retrieve(struct fuse_conn *fc, struct inode *inode, unsigned int num; unsigned int offset; size_t total_len = 0; + int num_pages; - req = fuse_get_req(fc, FUSE_MAX_PAGES_PER_REQ); + offset = outarg->offset & ~PAGE_CACHE_MASK; + file_size = i_size_read(inode); + + num = outarg->size; + if (outarg->offset > file_size) + num = 0; + else if (outarg->offset + num > file_size) + num = file_size - outarg->offset; + + num_pages = (num + offset + PAGE_SIZE - 1) >> PAGE_SHIFT; + num_pages = min(num_pages, FUSE_MAX_PAGES_PER_REQ); + + req = fuse_get_req(fc, num_pages); if (IS_ERR(req)) return PTR_ERR(req); - offset = outarg->offset & ~PAGE_CACHE_MASK; - req->in.h.opcode = FUSE_NOTIFY_REPLY; req->in.h.nodeid = outarg->nodeid; req->in.numargs = 2; @@ -1578,14 +1589,8 @@ static int fuse_retrieve(struct fuse_conn *fc, struct inode *inode, req->end = fuse_retrieve_end; index = outarg->offset >> PAGE_CACHE_SHIFT; - file_size = i_size_read(inode); - num = outarg->size; - if (outarg->offset > file_size) - num = 0; - else if (outarg->offset + num > file_size) - num = file_size - outarg->offset; - while (num && req->num_pages < FUSE_MAX_PAGES_PER_REQ) { + while (num && req->num_pages < num_pages) { struct page *page; unsigned int this_num;
creator: Maxim Patlasov
date: 2012-09-19T16:32:03-00:00

[PATCH 04/11] fuse: rework fuse_readpages()
type: item
description: The patch uses 'nr_pages' argument of fuse_readpages() as heuristics for the number of page pointers to allocate. This can be improved further by taking in consideration fc->max_read and gaps between page indices, but it's not clear whether it's worthy or not. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 214e13e..a618371 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -641,6 +641,7 @@ struct fuse_fill_data { struct fuse_req *req; struct file *file; struct inode *inode; + unsigned nr_pages; }; static int fuse_readpages_fill(void *_data, struct page *page) @@ -656,16 +657,25 @@ static int fuse_readpages_fill(void *_data, struct page *page) (req->num_pages == FUSE_MAX_PAGES_PER_REQ || (req->num_pages + 1) * PAGE_CACHE_SIZE > fc->max_read || req->pages[req->num_pages - 1]->index + 1 != page->index)) { + int nr_alloc = min_t(unsigned, data->nr_pages, + FUSE_MAX_PAGES_PER_REQ); fuse_send_readpages(req, data->file); - data->req = req = fuse_get_req(fc, FUSE_MAX_PAGES_PER_REQ); + data->req = req = fuse_get_req(fc, nr_alloc); if (IS_ERR(req)) { unlock_page(page); return PTR_ERR(req); } } + + if (WARN_ON(req->num_pages >= req->max_pages)) { + fuse_put_request(fc, req); + return -EIO; + } + page_cache_get(page); req->pages[req->num_pages] = page; req->num_pages++; + data->nr_pages--; return 0; } @@ -676,6 +686,7 @@ static int fuse_readpages(struct file *file, struct address_space *mapping, struct fuse_conn *fc = get_fuse_conn(inode); struct fuse_fill_data data; int err; + int nr_alloc = min_t(unsigned, nr_pages, FUSE_MAX_PAGES_PER_REQ); err = -EIO; if (is_bad_inode(inode)) @@ -683,7 +694,8 @@ static int fuse_readpages(struct file *file, struct address_space *mapping, data.file = file; data.inode = inode; - data.req = fuse_get_req(fc, FUSE_MAX_PAGES_PER_REQ); + data.req = fuse_get_req(fc, nr_alloc); + data.nr_pages = nr_pag…
creator: Maxim Patlasov
date: 2012-09-19T16:32:16-00:00

[PATCH 05/11] fuse: rework fuse_perform_write()
type: item
description: The patch allocates as many page pointers in fuse_req as needed to cover interval [pos .. pos+len-1]. Inline helper fuse_wr_pages() is introduced to hide this cumbersome arithmetic. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index a618371..d72d638 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -881,11 +881,19 @@ static ssize_t fuse_fill_write_pages(struct fuse_req *req, if (!fc->big_writes) break; } while (iov_iter_count(ii) && count < fc->max_write && - req->num_pages < FUSE_MAX_PAGES_PER_REQ && offset == 0); + req->num_pages < req->max_pages && offset == 0); return count > 0 ? count : err; } +static inline unsigned fuse_wr_pages(loff_t pos, size_t len) +{ + return min_t(unsigned, + ((pos + len - 1) >> PAGE_CACHE_SHIFT) - + (pos >> PAGE_CACHE_SHIFT) + 1, + FUSE_MAX_PAGES_PER_REQ); +} + static ssize_t fuse_perform_write(struct file *file, struct address_space *mapping, struct iov_iter *ii, loff_t pos) @@ -901,8 +909,9 @@ static ssize_t fuse_perform_write(struct file *file, do { struct fuse_req *req; ssize_t count; + unsigned nr_pages = fuse_wr_pages(pos, iov_iter_count(ii)); - req = fuse_get_req(fc, FUSE_MAX_PAGES_PER_REQ); + req = fuse_get_req(fc, nr_pages); if (IS_ERR(req)) { err = PTR_ERR(req); break;
creator: Maxim Patlasov
date: 2012-09-19T16:32:25-00:00

[PATCH 06/11] fuse: rework fuse_do_ioctl()
type: item
description: fuse_do_ioctl() already calculates the number of pages it's going to use. It is stored in 'num_pages' variable. So the patch simply uses it for allocating fuse_req. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index d72d638..08899a6 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1893,7 +1893,7 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg, num_pages++; } - req = fuse_get_req(fc, FUSE_MAX_PAGES_PER_REQ); + req = fuse_get_req(fc, num_pages); if (IS_ERR(req)) { err = PTR_ERR(req); req = NULL;
creator: Maxim Patlasov
date: 2012-09-19T16:32:37-00:00

[PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req
type: item
description: The ability to save page pointers along with lengths and offsets in fuse_req will be useful to cover several iovec-s with a single fuse_req. Per-request page_offset is removed because anybody who need it can use req->page_descs[0].offset instead. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/dev.c | 27 +++++++++++++++++++-------- fs/fuse/file.c | 10 +++++----- fs/fuse/fuse_i.h | 15 ++++++++++++--- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index b241a7d..ea63d39 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -35,14 +35,16 @@ static struct fuse_conn *fuse_get_conn(struct file *file) } static void fuse_request_init(struct fuse_req *req, struct page **pages, - unsigned npages) + struct page_desc *page_descs, unsigned npages) { memset(req, 0, sizeof(*req)); + memset(page_descs, 0, sizeof(*page_descs) * npages); INIT_LIST_HEAD(&req->list); INIT_LIST_HEAD(&req->intr_entry); init_waitqueue_head(&req->waitq); atomic_set(&req->count, 1); req->pages = pages; + req->page_descs = page_descs; req->max_pages = npages; } @@ -51,18 +53,25 @@ static struct fuse_req *__fuse_request_alloc(int npages, gfp_t flags) struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); if (req) { struct page **pages; + struct page_desc *page_descs; - if (npages <= FUSE_REQ_INLINE_PAGES) + if (npages <= FUSE_REQ_INLINE_PAGES) { pages = req->inline_pages; - else + page_descs = req->inline_page_descs; + } else { pages = kmalloc(sizeof(struct page *) * npages, flags); + page_descs = kmalloc(sizeof(struct page_desc) * npages, + flags); + } - if (!pages) { + if (!pages || !page_descs) { + kfree(pages); + kfree(page_descs); kmem_cache_free(fuse_req_cachep, req); return NULL; } - fuse_request_init(req, pages, npages); + fuse_request_init(req, pages, page_descs, npages); } return req; } @@ -82,6 +91,8 @@ void fuse_request_free(struct fuse_req *req) { if (req->pages != req->inline_pages) kfree(req->pages); + if (req->page_descs …
creator: Maxim Patlasov
date: 2012-09-19T16:32:55-00:00

[PATCH 08/11] fuse: use req-&gt;page_descs[] for argpages cases
type: item
description: Previously, anyone who set flag 'argpages' only filled req->pages[] and set per-request page_offset. This patch re-works all cases where argpages=1 to fill req->page_descs[] properly. Having req->page_descs[] filled properly allows to re-work fuse_copy_pages() to copy page fragments described by req->page_descs[]. This will be useful for next patches optimizing direct_IO. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/cuse.c | 1 + fs/fuse/dev.c | 7 +++---- fs/fuse/dir.c | 1 + fs/fuse/file.c | 20 ++++++++++++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c index 2f1d5dd..38efe69 100644 --- a/fs/fuse/cuse.c +++ b/fs/fuse/cuse.c @@ -441,6 +441,7 @@ static int cuse_send_init(struct cuse_conn *cc) req->out.argvar = 1; req->out.argpages = 1; req->pages[0] = page; + req->page_descs[0].length = req->out.args[1].size; req->num_pages = 1; req->end = cuse_process_init_reply; fuse_request_send_background(fc, req); diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index ea63d39..21b6272 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -888,11 +888,11 @@ static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes, { unsigned i; struct fuse_req *req = cs->req; - unsigned offset = req->page_descs[0].offset; - unsigned count = min(nbytes, (unsigned) PAGE_SIZE - offset); for (i = 0; i < req->num_pages && (nbytes || zeroing); i++) { int err; + unsigned offset = req->page_descs[i].offset; + unsigned count = min(nbytes, req->page_descs[i].length); err = fuse_copy_page(cs, &req->pages[i], offset, count, zeroing); @@ -900,8 +900,6 @@ static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes, return err; nbytes -= count; - count = min(nbytes, (unsigned) PAGE_SIZE); - offset = 0; } return 0; } @@ -1611,6 +1609,7 @@ static int fuse_retrieve(struct fuse_conn *fc, struct inode *inode, this_num = min_t(unsigned, num, PAGE_CACHE_SIZE - offset); req->pages[req->num_pages] = page; + req->page_descs[req->n…
creator: Maxim Patlasov
date: 2012-09-19T16:33:03-00:00

[PATCH 09/11] fuse: pass iov[] to fuse_get_user_pages()
type: item
description: The patch makes preliminary work for the next patch optimizing scatter-gather direct IO. The idea is to allow fuse_get_user_pages() to pack as many iov-s to each fuse request as possible. So, here we only rework all related call-paths to carry iov[] from fuse_direct_IO() to fuse_get_user_pages(). Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 108 +++++++++++++++++++++++++++++--------------------------- 1 files changed, 55 insertions(+), 53 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 8a1f833..db9efb5 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1056,11 +1056,15 @@ static inline void fuse_page_descs_length_init(struct fuse_req *req) req->page_descs[i].offset; } -static int fuse_get_user_pages(struct fuse_req *req, const char __user *buf, +static int fuse_get_user_pages(struct fuse_req *req, + const struct iovec **iov_pp, + unsigned long *nr_segs_p, + size_t *iov_offset_p, size_t *nbytesp, int write) { size_t nbytes = *nbytesp; - unsigned long user_addr = (unsigned long) buf; + size_t frag_size = min_t(size_t, nbytes, (*iov_pp)->iov_len - *iov_offset_p); + unsigned long user_addr = (unsigned long)(*iov_pp)->iov_base + *iov_offset_p; unsigned offset = user_addr & ~PAGE_MASK; int npages; @@ -1071,10 +1075,13 @@ static int fuse_get_user_pages(struct fuse_req *req, const char __user *buf, else req->out.args[0].value = (void *) user_addr; + (*iov_pp)++; + (*nr_segs_p)--; + *nbytesp = frag_size; return 0; } - nbytes = min_t(size_t, nbytes, FUSE_MAX_PAGES_PER_REQ << PAGE_SHIFT); + nbytes = min_t(size_t, frag_size, FUSE_MAX_PAGES_PER_REQ << PAGE_SHIFT); npages = (nbytes + offset + PAGE_SIZE - 1) >> PAGE_SHIFT; npages = clamp(npages, 1, FUSE_MAX_PAGES_PER_REQ); npages = get_user_pages_fast(user_addr, npages, !write, req->pages); @@ -1092,17 +1099,26 @@ static int fuse_get_user_pages(struct fuse_req *req, const char __user *buf, nbytes = (req->num_pages << PAGE_SHIFT) - req->page_descs[0].offset; - if (*nbytesp < nbytes) …
creator: Maxim Patlasov
date: 2012-09-19T16:33:12-00:00

[PATCH 10/11] fuse: optimize fuse_get_user_pages()
type: item
description: Let fuse_get_user_pages() pack as many iov-s to a single fuse_req as possible. This is very beneficial in case of iov[] consisting of many iov-s of relatively small sizes (e.g. PAGE_SIZE). Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 94 +++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 58 insertions(+), 36 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index db9efb5..5b0fa5d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1047,13 +1047,24 @@ static void fuse_release_user_pages(struct fuse_req *req, int write) } } -static inline void fuse_page_descs_length_init(struct fuse_req *req) +static inline void fuse_page_descs_length_init(struct fuse_req *req, + unsigned i, int n) { - int i; + while (n-- > 0) + req->page_descs[i + n].length = PAGE_SIZE - + req->page_descs[i + n].offset; +} - for (i = 0; i < req->num_pages; i++) - req->page_descs[i].length = PAGE_SIZE - - req->page_descs[i].offset; +static inline unsigned long fuse_get_ua(const struct iovec *iov, + size_t iov_offset) +{ + return (unsigned long)iov->iov_base + iov_offset; +} + +static inline size_t fuse_get_fr_sz(const struct iovec *iov, size_t iov_offset, + size_t max_size) +{ + return min_t(size_t, iov->iov_len - iov_offset, max_size); } static int fuse_get_user_pages(struct fuse_req *req, @@ -1062,14 +1073,12 @@ static int fuse_get_user_pages(struct fuse_req *req, size_t *iov_offset_p, size_t *nbytesp, int write) { - size_t nbytes = *nbytesp; - size_t frag_size = min_t(size_t, nbytes, (*iov_pp)->iov_len - *iov_offset_p); - unsigned long user_addr = (unsigned long)(*iov_pp)->iov_base + *iov_offset_p; - unsigned offset = user_addr & ~PAGE_MASK; - int npages; + size_t nbytes = 0; /* # bytes already packed in req */ /* Special case for kernel I/O: can copy directly into the buffer */ if (segment_eq(get_fs(), KERNEL_DS)) { + unsigned long user_addr = fuse_get_ua(*iov_pp, *iov_offset_p); + if (write) req->in.args[1].value = (void *) user_addr;…
creator: Maxim Patlasov
date: 2012-09-19T16:33:22-00:00

[PATCH 11/11] fuse: optimize __fuse_direct_io()
type: item
description: __fuse_direct_io() allocates fuse-requests by calling fuse_get_req(fc, n). The patch calculates 'n' based on iov[] array. This is useful because allocating FUSE_MAX_PAGES_PER_REQ page pointers and descriptors for each fuse request would be waste of memory in case of iov-s of smaller size. Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 27 +++++++++++++++++++++++---- 1 files changed, 23 insertions(+), 4 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 5b0fa5d..5ecab19 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1090,14 +1090,14 @@ static int fuse_get_user_pages(struct fuse_req *req, return 0; } - while (nbytes < *nbytesp && req->num_pages < FUSE_MAX_PAGES_PER_REQ) { + while (nbytes < *nbytesp && req->num_pages < req->max_pages) { int npages; unsigned long user_addr = fuse_get_ua(*iov_pp, *iov_offset_p); unsigned offset = user_addr & ~PAGE_MASK; size_t frag_size = fuse_get_fr_sz(*iov_pp, *iov_offset_p, *nbytesp - nbytes); - int n = FUSE_MAX_PAGES_PER_REQ - req->num_pages; + int n = req->max_pages - req->num_pages; frag_size = min_t(size_t, frag_size, n << PAGE_SHIFT); npages = (frag_size + offset + PAGE_SIZE - 1) >> PAGE_SHIFT; @@ -1138,6 +1138,24 @@ static int fuse_get_user_pages(struct fuse_req *req, return 0; } +static inline int fuse_iov_npages(const struct iovec *iov, + unsigned long nr_segs, size_t iov_offset) +{ + int npages = 0; + + while (nr_segs-- > 0 && npages < FUSE_MAX_PAGES_PER_REQ) { + unsigned long user_addr = fuse_get_ua(iov, iov_offset); + unsigned offset = user_addr & ~PAGE_MASK; + size_t frag_size = iov->iov_len - iov_offset; + + npages += (frag_size + offset + PAGE_SIZE - 1) >> PAGE_SHIFT; + iov++; + iov_offset = 0; + } + + return min(npages, FUSE_MAX_PAGES_PER_REQ); +} + static ssize_t __fuse_direct_io(struct file *file, const struct iovec *iov, unsigned long nr_segs, size_t count, loff_t *ppos, int write) @@ -1150,7 +1168,7 @@ static ssize_t __fuse_direct_io(struct file *file, const struct iov…
creator: Maxim Patlasov
date: 2012-09-19T16:33:31-00:00

[PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req (v2)
type: item
description: The ability to save page pointers along with lengths and offsets in fuse_req will be useful to cover several iovec-s with a single fuse_req. Per-request page_offset is removed because anybody who need it can use req->page_descs[0].offset instead. Changed in v2: - replaced structure page_desc with fuse_page_desc Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/dev.c | 26 +++++++++++++++++++------- fs/fuse/file.c | 10 +++++----- fs/fuse/fuse_i.h | 15 ++++++++++++--- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index b241a7d..72ad962 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -35,14 +35,17 @@ static struct fuse_conn *fuse_get_conn(struct file *file) } static void fuse_request_init(struct fuse_req *req, struct page **pages, + struct fuse_page_desc *page_descs, unsigned npages) { memset(req, 0, sizeof(*req)); + memset(page_descs, 0, sizeof(*page_descs) * npages); INIT_LIST_HEAD(&req->list); INIT_LIST_HEAD(&req->intr_entry); init_waitqueue_head(&req->waitq); atomic_set(&req->count, 1); req->pages = pages; + req->page_descs = page_descs; req->max_pages = npages; } @@ -51,18 +54,25 @@ static struct fuse_req *__fuse_request_alloc(int npages, gfp_t flags) struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); if (req) { struct page **pages; + struct fuse_page_desc *page_descs; - if (npages <= FUSE_REQ_INLINE_PAGES) + if (npages <= FUSE_REQ_INLINE_PAGES) { pages = req->inline_pages; - else + page_descs = req->inline_page_descs; + } else { pages = kmalloc(sizeof(struct page *) * npages, flags); + page_descs = kmalloc(sizeof(struct fuse_page_desc) * + npages, flags); + } - if (!pages) { + if (!pages || !page_descs) { + kfree(pages); + kfree(page_descs); kmem_cache_free(fuse_req_cachep, req); return NULL; } - fuse_request_init(req, pages, npages); + fuse_request_init(req, pages, page_descs, npages); } return req; } @@ -82,6 +92,8 @@ void fuse_request_free(struct fuse_req *req) { if (req->pages !=…
creator: Maxim Patlasov
date: 2012-10-01T11:20:00-00:00

[PATCH 10/11] fuse: optimize fuse_get_user_pages() - v2
type: item
description: Let fuse_get_user_pages() pack as many iov-s to a single fuse_req as possible. This is very beneficial in case of iov[] consisting of many iov-s of relatively small sizes (e.g. PAGE_SIZE). Changed in v2: - renamed local vars in fuse_page_descs_length_init() to be more readable Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> --- fs/fuse/file.c | 94 +++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 58 insertions(+), 36 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index db9efb5..4d30697 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1047,13 +1047,24 @@ static void fuse_release_user_pages(struct fuse_req *req, int write) } } -static inline void fuse_page_descs_length_init(struct fuse_req *req) +static inline void fuse_page_descs_length_init(struct fuse_req *req, + unsigned index, int nr_pages) { - int i; + while (nr_pages-- > 0) + req->page_descs[index + nr_pages].length = PAGE_SIZE - + req->page_descs[index + nr_pages].offset; +} - for (i = 0; i < req->num_pages; i++) - req->page_descs[i].length = PAGE_SIZE - - req->page_descs[i].offset; +static inline unsigned long fuse_get_ua(const struct iovec *iov, + size_t iov_offset) +{ + return (unsigned long)iov->iov_base + iov_offset; +} + +static inline size_t fuse_get_fr_sz(const struct iovec *iov, size_t iov_offset, + size_t max_size) +{ + return min_t(size_t, iov->iov_len - iov_offset, max_size); } static int fuse_get_user_pages(struct fuse_req *req, @@ -1062,14 +1073,12 @@ static int fuse_get_user_pages(struct fuse_req *req, size_t *iov_offset_p, size_t *nbytesp, int write) { - size_t nbytes = *nbytesp; - size_t frag_size = min_t(size_t, nbytes, (*iov_pp)->iov_len - *iov_offset_p); - unsigned long user_addr = (unsigned long)(*iov_pp)->iov_base + *iov_offset_p; - unsigned offset = user_addr & ~PAGE_MASK; - int npages; + size_t nbytes = 0; /* # bytes already packed in req */ /* Special case for kernel I/O: can copy directly into the buffer */ if (segment_eq(get_fs(), KERNEL…
creator: Maxim Patlasov
date: 2012-10-15T12:03:51-00:00

Re: [PATCH v2 00/11] fuse: optimize scatter-gather direct IO
type: item
description: Hi Miklos, 09/19/2012 08:31 PM, Maxim Patlasov пишет: > Hi, > > Existing fuse implementation processes scatter-gather direct IO in suboptimal > way: fuse_direct_IO passes iovec[] to fuse_loop_dio and the latter calls > fuse_direct_read/write for each iovec from iovec[] array. Thus we have as many > submitted fuse-requests as the number of elements in iovec[] array. This is > pure waste of resources and affects performance negatively especially for the > case of many small chunks (e.g. page-size) packed in one iovec[] array. > > The patch-set amends situation in a natural way: let's simply pack as > many iovec[] segments to every fuse-request as possible. > > To estimate performance improvement I used slightly modified fusexmp over > tmpfs (clearing O_DIRECT bit from fi->flags in xmp_open). The test opened > a file with O_DIRECT, then called readv/writev in a loop. An iovec[] for > readv/writev consisted of 32 segments of 4K each. The throughput on some > commodity (rather feeble) server was (in MB/sec): > > original / patched > writev: ~107 / ~480 > readv: ~114 / ~569 > > We're exploring possibility to use fuse for our own distributed storage > implementation and big iovec[] arrays of many page-size chunks is typical > use-case for device virtualization thread performing i/o on behalf of > virtual-machine it serves. > > Changed in v2: > - inline array of page pointers req->pages[] is replaced with dynamically > allocated one; the number of elements is calculated a bit more > intelligently than being equal to FUSE_MAX_PAGES_PER_REQ; this is done > for the sake of memory economy. > - a dynamically allocated array of so-called 'page descriptors' - an offset > in page plus the length of fragment - is added to fuse_req; this is done > to simplify processing fuse requests covering several iov-s. > > Thanks, > Maxim > > --- > > Maxim Patlasov (11): > fuse: general infrastructure for pages[] of variable size > fuse: categorize fuse_get_req() > fuse: rework fuse_retrieve() >…
creator: Maxim Patlasov
date: 2012-10-16T17:32:17-00:00

Re: [PATCH 01/11] fuse: general infrastructure for pages[] of variable size
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch removes inline array of FUSE_MAX_PAGES_PER_REQ page pointers from > fuse_req. Instead of that, req->pages may now point either to small inline > array or to an array allocated dynamically. > > This essentially means that all callers of fuse_request_alloc[_nofs] should > pass the number of pages needed explicitly. > > The patch doesn't make any logic changes. > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/dev.c | 46 +++++++++++++++++++++++++++++++++------------- > fs/fuse/file.c | 4 ++-- > fs/fuse/fuse_i.h | 15 ++++++++++++--- > fs/fuse/inode.c | 4 ++-- > 4 files changed, 49 insertions(+), 20 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 7df2b5e..1b5fc73 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -34,34 +34,54 @@ static struct fuse_conn *fuse_get_conn(struct file *file) > return file->private_data; > } > > -static void fuse_request_init(struct fuse_req *req) > +static void fuse_request_init(struct fuse_req *req, struct page **pages, > + unsigned npages) > { > memset(req, 0, sizeof(*req)); > INIT_LIST_HEAD(&req->list); > INIT_LIST_HEAD(&req->intr_entry); > init_waitqueue_head(&req->waitq); > atomic_set(&req->count, 1); > + req->pages = pages; > + req->max_pages = npages; > } > > -struct fuse_req *fuse_request_alloc(void) > +static struct fuse_req *__fuse_request_alloc(int npages, gfp_t flags) I don't see any reason for npages to be a signed integer. > { > - struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, GFP_KERNEL); > - if (req) > - fuse_request_init(req); > + struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); > + if (req) { > + struct page **pages; > + > + if (npages <= FUSE_REQ_INLINE_PAGES) > + pages = req->inline_pages; > + else > + pages = kmalloc(sizeof(struct page *) * npages, flags); > + > + if (!pages) { > + kmem_cache_free(fuse_req_cachep, req); > + return NULL; > + } > + > + fuse_request_init(req, pages, npages…
creator: Miklos Szeredi
date: 2012-10-24T16:09:22-00:00

Re: [PATCH 02/11] fuse: categorize fuse_get_req()
type: item
description: Hi, Sorry about the long delay in responding. See comments below. Thanks, Miklos Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch categorizes all fuse_get_req() invocations into two categories: > - fuse_get_req_nopages(fc) - when caller doesn't care about req->pages > - fuse_get_req(fc, n) - when caller need n page pointers (n > 0) > > Adding fuse_get_req_nopages() helps to avoid numerous fuse_get_req(fc, 0) > scattered over code. Now it's clear from the first glance when a caller need > fuse_req with page pointers. > > The patch doesn't make any logic changes. In multi-page case, it silly > allocates array of FUSE_MAX_PAGES_PER_REQ page pointers. This will be amended > by future patches. > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/cuse.c | 2 +- > fs/fuse/dev.c | 11 ++++++----- > fs/fuse/dir.c | 38 +++++++++++++++++++------------------- > fs/fuse/file.c | 30 +++++++++++++++--------------- > fs/fuse/fuse_i.h | 17 ++++++++++++++--- > fs/fuse/inode.c | 2 +- > 6 files changed, 56 insertions(+), 44 deletions(-) > > diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c > index 3426521..2f1d5dd 100644 > --- a/fs/fuse/cuse.c > +++ b/fs/fuse/cuse.c > @@ -411,7 +411,7 @@ static int cuse_send_init(struct cuse_conn *cc) > > BUILD_BUG_ON(CUSE_INIT_INFO_MAX > PAGE_SIZE); > > - req = fuse_get_req(fc); > + req = fuse_get_req(fc, 1); > if (IS_ERR(req)) { > rc = PTR_ERR(req); > goto err; > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 1b5fc73..3ad5570 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -117,7 +117,7 @@ static void fuse_req_init_context(struct fuse_req *req) > req->in.h.pid = current->pid; > } > > -struct fuse_req *fuse_get_req(struct fuse_conn *fc) > +struct fuse_req *fuse_get_req(struct fuse_conn *fc, int npages) Again, npages should be unsigned, AFAICS. > { > struct fuse_req *req; > sigset_t oldset; > @@ -136,7 +136,7 @@ struct fuse_req *fuse_get_req(struct fuse_conn *fc) > if (!fc->connected) > goto out; > > - req = fuse…
creator: Miklos Szeredi
date: 2012-10-24T16:26:45-00:00

Re: [PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req (v2)
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The ability to save page pointers along with lengths and offsets in fuse_req > will be useful to cover several iovec-s with a single fuse_req. > > Per-request page_offset is removed because anybody who need it can use > req->page_descs[0].offset instead. > > Changed in v2: > - replaced structure page_desc with fuse_page_desc > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/dev.c | 26 +++++++++++++++++++------- > fs/fuse/file.c | 10 +++++----- > fs/fuse/fuse_i.h | 15 ++++++++++++--- > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index b241a7d..72ad962 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -35,14 +35,17 @@ static struct fuse_conn *fuse_get_conn(struct file *file) > } > > static void fuse_request_init(struct fuse_req *req, struct page **pages, > + struct fuse_page_desc *page_descs, > unsigned npages) > { > memset(req, 0, sizeof(*req)); > + memset(page_descs, 0, sizeof(*page_descs) * npages); Makes me wonder: why aren't we zeroing out the page array too? > INIT_LIST_HEAD(&req->list); > INIT_LIST_HEAD(&req->intr_entry); > init_waitqueue_head(&req->waitq); > atomic_set(&req->count, 1); > req->pages = pages; > + req->page_descs = page_descs; > req->max_pages = npages; > } > > @@ -51,18 +54,25 @@ static struct fuse_req *__fuse_request_alloc(int npages, gfp_t flags) > struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); > if (req) { > struct page **pages; > + struct fuse_page_desc *page_descs; > > - if (npages <= FUSE_REQ_INLINE_PAGES) > + if (npages <= FUSE_REQ_INLINE_PAGES) { > pages = req->inline_pages; > - else > + page_descs = req->inline_page_descs; > + } else { > pages = kmalloc(sizeof(struct page *) * npages, flags); > + page_descs = kmalloc(sizeof(struct fuse_page_desc) * > + npages, flags); > + } > > - if (!pages) { > + if (!pages || !page_descs) { > + kfree(pages); > + kfree(page_descs); > kmem_cache_fre…
creator: Miklos Szeredi
date: 2012-10-25T13:24:17-00:00

Re: [PATCH 07/11] fuse: add per-page descriptor &lt;offset, length&gt; to fuse_req (v2)
type: item
description: Hi, 10/25/2012 05:24 PM, Miklos Szeredi пишет: > Maxim Patlasov <mpatlasov@parallels.com> writes: > >> The ability to save page pointers along with lengths and offsets in fuse_req >> will be useful to cover several iovec-s with a single fuse_req. >> >> Per-request page_offset is removed because anybody who need it can use >> req->page_descs[0].offset instead. >> >> Changed in v2: >> - replaced structure page_desc with fuse_page_desc >> >> Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> >> --- >> fs/fuse/dev.c | 26 +++++++++++++++++++------- >> fs/fuse/file.c | 10 +++++----- >> fs/fuse/fuse_i.h | 15 ++++++++++++--- >> 3 files changed, 36 insertions(+), 15 deletions(-) >> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >> index b241a7d..72ad962 100644 >> --- a/fs/fuse/dev.c >> +++ b/fs/fuse/dev.c >> @@ -35,14 +35,17 @@ static struct fuse_conn *fuse_get_conn(struct file *file) >> } >> >> static void fuse_request_init(struct fuse_req *req, struct page **pages, >> + struct fuse_page_desc *page_descs, >> unsigned npages) >> { >> memset(req, 0, sizeof(*req)); >> + memset(page_descs, 0, sizeof(*page_descs) * npages); > Makes me wonder: why aren't we zeroing out the page array too? Good catch, thnx! This is a legacy since time when I attempted to use <page, length, offset> as fuse_page_desc. Now, when we have both page_descs[] and pages[], it's natural to zero both. > > @@ -82,6 +92,8 @@ void fuse_request_free(struct fuse_req *req) > { > if (req->pages != req->inline_pages) > kfree(req->pages); > + if (req->page_descs != req->inline_page_descs) > + kfree(req->page_descs); > You allocate them together, you can free them together. Just add a > BUG_ON() if you feel paranoid. OK. Thanks, Maxim
creator: Maxim Patlasov
date: 2012-10-25T13:39:21-00:00

Re: [PATCH 08/11] fuse: use req-&gt;page_descs[] for argpages cases
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > Previously, anyone who set flag 'argpages' only filled req->pages[] and set > per-request page_offset. This patch re-works all cases where argpages=1 to > fill req->page_descs[] properly. > > Having req->page_descs[] filled properly allows to re-work fuse_copy_pages() > to copy page fragments described by req->page_descs[]. This will be useful > for next patches optimizing direct_IO. > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/cuse.c | 1 + > fs/fuse/dev.c | 7 +++---- > fs/fuse/dir.c | 1 + > fs/fuse/file.c | 20 ++++++++++++++++++++ > 4 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c > index 2f1d5dd..38efe69 100644 > --- a/fs/fuse/cuse.c > +++ b/fs/fuse/cuse.c > @@ -441,6 +441,7 @@ static int cuse_send_init(struct cuse_conn *cc) > req->out.argvar = 1; > req->out.argpages = 1; > req->pages[0] = page; > + req->page_descs[0].length = req->out.args[1].size; > req->num_pages = 1; > req->end = cuse_process_init_reply; > fuse_request_send_background(fc, req); > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index ea63d39..21b6272 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -888,11 +888,11 @@ static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes, > { > unsigned i; > struct fuse_req *req = cs->req; > - unsigned offset = req->page_descs[0].offset; > - unsigned count = min(nbytes, (unsigned) PAGE_SIZE - offset); > > for (i = 0; i < req->num_pages && (nbytes || zeroing); i++) { > int err; > + unsigned offset = req->page_descs[i].offset; > + unsigned count = min(nbytes, req->page_descs[i].length); Wouldn't it be cleaner if callers calculated the last page's .length value from the total number of bytes? So this would just be unsigned count = req->page_descs[i].length; And at the end of the function we can assert that nbytes went to exactly zero with a WARN_ON(). But this is a change that needs careful testing, so maybe we're …
creator: Miklos Szeredi
date: 2012-10-25T14:05:22-00:00

Re: [PATCH 09/11] fuse: pass iov[] to fuse_get_user_pages()
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch makes preliminary work for the next patch optimizing scatter-gather > direct IO. The idea is to allow fuse_get_user_pages() to pack as many iov-s > to each fuse request as possible. So, here we only rework all related > call-paths to carry iov[] from fuse_direct_IO() to fuse_get_user_pages(). > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/file.c | 108 +++++++++++++++++++++++++++++--------------------------- > 1 files changed, 55 insertions(+), 53 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 8a1f833..db9efb5 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -1056,11 +1056,15 @@ static inline void fuse_page_descs_length_init(struct fuse_req *req) > req->page_descs[i].offset; > } > > -static int fuse_get_user_pages(struct fuse_req *req, const char __user *buf, > +static int fuse_get_user_pages(struct fuse_req *req, > + const struct iovec **iov_pp, > + unsigned long *nr_segs_p, > + size_t *iov_offset_p, Aren't these arguments exactly what 'struct iov_iter' is supposed to represent? Would be a nice cleanup, and in addition we'd get well tested helper functions that iterate over the iovec. > size_t *nbytesp, int write) > { > size_t nbytes = *nbytesp; > - unsigned long user_addr = (unsigned long) buf; > + size_t frag_size = min_t(size_t, nbytes, (*iov_pp)->iov_len - *iov_offset_p); > + unsigned long user_addr = (unsigned long)(*iov_pp)->iov_base + *iov_offset_p; > unsigned offset = user_addr & ~PAGE_MASK; > int npages; > > @@ -1071,10 +1075,13 @@ static int fuse_get_user_pages(struct fuse_req *req, const char __user *buf, > else > req->out.args[0].value = (void *) user_addr; > > + (*iov_pp)++; > + (*nr_segs_p)--; > + *nbytesp = frag_size; > return 0; > } > > - nbytes = min_t(size_t, nbytes, FUSE_MAX_PAGES_PER_REQ << PAGE_SHIFT); > + nbytes = min_t(size_t, frag_size, FUSE_MAX_PAGES_PER_REQ << PAGE_SHIFT); > npages = (nbytes + offset + PAGE_SIZE - 1) >> …
creator: Miklos Szeredi
date: 2012-10-25T14:29:27-00:00

Re: [PATCH 10/11] fuse: optimize fuse_get_user_pages() - v2
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > Let fuse_get_user_pages() pack as many iov-s to a single fuse_req as > possible. This is very beneficial in case of iov[] consisting of many > iov-s of relatively small sizes (e.g. PAGE_SIZE). > > Changed in v2: > - renamed local vars in fuse_page_descs_length_init() to be more readable > > Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> > --- > fs/fuse/file.c | 94 +++++++++++++++++++++++++++++++++++--------------------- > 1 files changed, 58 insertions(+), 36 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index db9efb5..4d30697 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -1047,13 +1047,24 @@ static void fuse_release_user_pages(struct fuse_req *req, int write) > } > } > > -static inline void fuse_page_descs_length_init(struct fuse_req *req) > +static inline void fuse_page_descs_length_init(struct fuse_req *req, > + unsigned index, int nr_pages) > { > - int i; > + while (nr_pages-- > 0) Please avoid such constructs. They are hard to read and easy to mess up. Use an auxiliary local variable, just like you used to. > + req->page_descs[index + nr_pages].length = PAGE_SIZE - > + req->page_descs[index + nr_pages].offset; > +} > > - for (i = 0; i < req->num_pages; i++) > - req->page_descs[i].length = PAGE_SIZE - > - req->page_descs[i].offset; > +static inline unsigned long fuse_get_ua(const struct iovec *iov, fuse_get_user_addr() > + size_t iov_offset) > +{ > + return (unsigned long)iov->iov_base + iov_offset; > +} > + > +static inline size_t fuse_get_fr_sz(const struct iovec *iov, size_t > iov_offset, iov_iter_single_seg_count() is the function you are reimplementing here, I guess. I haven't reviewed the as I think it will automatically get cleaner if you switch to iov_iter. > + size_t max_size) > +{ > + return min_t(size_t, iov->iov_len - iov_offset, max_size); > } > > static int fuse_get_user_pages(struct fuse_req *req, > @@ -1062,14 +1073,12 @@ static int fuse_get_user_pages(struc…
creator: Miklos Szeredi
date: 2012-10-25T14:50:45-00:00

Re: [PATCH 08/11] fuse: use req-&gt;page_descs[] for argpages cases
type: item
description: Miklos, 10/25/2012 06:05 PM, Miklos Szeredi пишет: > Maxim Patlasov <mpatlasov@parallels.com> writes: > >> @@ -888,11 +888,11 @@ static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes, >> { >> unsigned i; >> struct fuse_req *req = cs->req; >> - unsigned offset = req->page_descs[0].offset; >> - unsigned count = min(nbytes, (unsigned) PAGE_SIZE - offset); >> >> for (i = 0; i < req->num_pages && (nbytes || zeroing); i++) { >> int err; >> + unsigned offset = req->page_descs[i].offset; >> + unsigned count = min(nbytes, req->page_descs[i].length); > Wouldn't it be cleaner if callers calculated the last page's .length > value from the total number of bytes? So this would just be > > unsigned count = req->page_descs[i].length; > > And at the end of the function we can assert that nbytes went to exactly > zero with a WARN_ON(). > > But this is a change that needs careful testing, so maybe we're better > off having that as a separate incremental patch later... It cannot be as simple as 'unsigned count = req->page_descs[i].length' because in case of short reads 'nbytes' (coming from userspace) can be unpredictably small. Modulo you share my opinion that a caller of fuse_copy_pages() shouldn't modify req->page_descs[i].length. As for WARN_ON(), we could probably guarantee that 'nbytes' <= capacity(req->pages[]) in WRITEs, but in READs, 'nbytes' comes from userspace and I'm not sure it's OK to clutter logs due to misbehaved userspace fuse (if we get 'nbytes' unexpectedly large). Thanks, Maxim
creator: Maxim Patlasov
date: 2012-10-25T15:38:30-00:00