OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
items: [PATCH 1/6] fuse: general infrastructure for pages[] of variable size
items: [PATCH 2/6] fuse: categorize fuse_get_req()
items: [PATCH 3/6] fuse: rework fuse_retrieve()
items: [PATCH 4/6] fuse: rework fuse_readpages()
items: [PATCH 5/6] fuse: rework fuse_perform_write()
items: [PATCH 6/6] fuse: rework fuse_do_ioctl()
items: Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
items: Re: [PATCH 1/6] fuse: general infrastructure for pages[] of variable size
items: Re: [PATCH 2/6] fuse: categorize fuse_get_req()
items: Re: [PATCH 3/6] fuse: rework fuse_retrieve()
items: Re: [PATCH 4/6] fuse: rework fuse_readpages()
items: Re: [PATCH 5/6] fuse: rework fuse_perform_write()

[PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: Hi, Currently, any fuse request always includes inline pages[] array of FUSE_MAX_PAGES_PER_REQ elements. This is the waste of memory because in many cases smaller size would suffice. The patch-set tries to allocate only as many elements of pages[] array as actaully needed. This will be even more useful in the future because of: 1. Mitsuo's patches making maximum read/write request size tunable. 2. My patches optimizing scatter-gather direct IO. To make them simplier I'll need to substitute array of 'struct page *' with array of 'struct bio_vec'. It would make memory overhead worse if implemented w/o this patch-set. Thanks, Maxim --- Maxim Patlasov (6): 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() fs/fuse/cuse.c | 2 +- fs/fuse/dev.c | 70 +++++++++++++++++++++++++++++++++++++----------------- fs/fuse/dir.c | 38 +++++++++++++++-------------- fs/fuse/file.c | 53 +++++++++++++++++++++++++---------------- fs/fuse/fuse_i.h | 45 ++++++++++++++++++++++++++++++----- fs/fuse/inode.c | 6 ++--- 6 files changed, 143 insertions(+), 71 deletions(-) -- Signature
creator: Maxim Patlasov
date: 2012-09-07T17:40:37-00:00

[PATCH 1/6] 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. --- fs/fuse/dev.c | 40 ++++++++++++++++++++++++++++++---------- fs/fuse/file.c | 4 ++-- fs/fuse/fuse_i.h | 9 ++++++--- fs/fuse/inode.c | 4 ++-- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 7df2b5e..c0283a1 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -36,32 +36,52 @@ static struct fuse_conn *fuse_get_conn(struct file *file) static void fuse_request_init(struct fuse_req *req) { + struct page **pages = req->pages; + 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; } -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) + struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); + if (req) { + if (npages <= 1) + req->pages = req->inline_pages; + else + req->pages = kmalloc(sizeof(struct page *) * npages, + flags); + + if (!req->pages) { + kmem_cache_free(fuse_req_cachep, req); + return NULL; + } + fuse_request_init(req); + } 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 fuse_req *fuse_request_alloc_nofs(int npages) { - struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, GFP_NOFS); - if (req) - fuse_request_init(req); - return req; + return __fuse_request_alloc(npages, GFP_NOFS); } void fuse_request_free(struct f…
creator: Maxim Patlasov
date: 2012-09-07T17:41:07-00:00

[PATCH 2/6] fuse: categorize fuse_get_req()
type: item
description: The patch categorizes all fuse_get_req() invocations into three categories: - fuse_get_req_nopages() - when caller doesn't care about req->pages - fuse_get_req_onepage() - when caller need exactly one page - fuse_get_req_multipage() - in other cases. The decision to split one fuse_get_req() into three different functions was made because having fuse_get_req(fc, n) scattered over code would be too error prone. After splitting, it's clear from the first glance when a caller needs multi-page fuse_req. 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. --- fs/fuse/cuse.c | 2 +- fs/fuse/dev.c | 10 +++++----- fs/fuse/dir.c | 38 +++++++++++++++++++------------------- fs/fuse/file.c | 30 +++++++++++++++--------------- fs/fuse/fuse_i.h | 36 +++++++++++++++++++++++++++++++++--- fs/fuse/inode.c | 2 +- 6 files changed, 74 insertions(+), 44 deletions(-) diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c index 3426521..672d86a 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_onepage(fc); if (IS_ERR(req)) { rc = PTR_ERR(req); goto err; diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index c0283a1..6e38af5 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,13 @@ static void put_reserved_req(struct fuse_conn *fc, struct fuse_r…
creator: Maxim Patlasov
date: 2012-09-07T17:41:19-00:00

[PATCH 3/6] 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 moslty shuffling lines. The patch also contains one-line fix suggested by Miklos: the loop iterating over pages from inode mapping should interpret 'offset' as offset in the very first page only. --- fs/fuse/dev.c | 26 ++++++++++++++++---------- 1 files changed, 16 insertions(+), 10 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 6e38af5..cf69df3 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1562,13 +1562,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_multipage(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_multipage(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; @@ -1577,14 +1588,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; @@ -1596,6 +1601,7 @@ static int fuse_ret…
creator: Maxim Patlasov
date: 2012-09-07T17:41:32-00:00

[PATCH 4/6] fuse: rework fuse_readpages()
type: item
description: The patch uses 'nr_pages' argument of fuse_readpages() as the heuristics for 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 wheteher it's worthy or not. --- fs/fuse/file.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index c811f3d..9a6dcc6 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,8 +657,10 @@ 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_multipage(fc, FUSE_MAX_PAGES_PER_REQ); + data->req = req = fuse_get_req_multipage(fc, nr_alloc); if (IS_ERR(req)) { unlock_page(page); return PTR_ERR(req); @@ -666,6 +669,7 @@ static int fuse_readpages_fill(void *_data, struct page *page) page_cache_get(page); req->pages[req->num_pages] = page; req->num_pages++; + data->nr_pages--; return 0; } @@ -676,6 +680,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 +688,8 @@ static int fuse_readpages(struct file *file, struct address_space *mapping, data.file = file; data.inode = inode; - data.req = fuse_get_req_multipage(fc, FUSE_MAX_PAGES_PER_REQ); + data.req = fuse_get_req_multipage(fc, nr_alloc); + data.nr_pages = nr_pages; err = PTR_ERR(data.req); if (IS_ERR(data.re…
creator: Maxim Patlasov
date: 2012-09-07T17:41:40-00:00

[PATCH 5/6] 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]. FUSE_WR_PAGES macro is introduced to hide this cumbersome arithmetics. --- fs/fuse/file.c | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 9a6dcc6..84cc83c 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -815,7 +815,8 @@ static size_t fuse_send_write_pages(struct fuse_req *req, struct file *file, static ssize_t fuse_fill_write_pages(struct fuse_req *req, struct address_space *mapping, - struct iov_iter *ii, loff_t pos) + struct iov_iter *ii, loff_t pos, + unsigned nr_pages) { struct fuse_conn *fc = get_fuse_conn(mapping->host); unsigned offset = pos & (PAGE_CACHE_SIZE - 1); @@ -875,11 +876,16 @@ 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 < nr_pages && offset == 0); return count > 0 ? count : err; } +#define FUSE_WR_PAGES(pos, len) 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) @@ -895,14 +901,15 @@ 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_multipage(fc, FUSE_MAX_PAGES_PER_REQ); + req = fuse_get_req_multipage(fc, nr_pages); if (IS_ERR(req)) { err = PTR_ERR(req); break; } - count = fuse_fill_write_pages(req, mapping, ii, pos); + count = fuse_fill_write_pages(req, mapping, ii, pos, nr_pages); if (count <= 0) { err = count; } else {
creator: Maxim Patlasov
date: 2012-09-07T17:41:49-00:00

[PATCH 6/6] 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. --- 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 84cc83c..abb070b 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1885,7 +1885,7 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg, num_pages++; } - req = fuse_get_req_multipage(fc, FUSE_MAX_PAGES_PER_REQ); + req = fuse_get_req_multipage(fc, num_pages); if (IS_ERR(req)) { err = PTR_ERR(req); req = NULL;
creator: Maxim Patlasov
date: 2012-09-07T17:41:59-00:00

Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: Hi, I noticed that 'Signed-off-by' is missed in my patches. Very sorry for inconvenience. I'll be more careful next time. Thanks, Maxim 09/07/2012 09:40 PM, Maxim Patlasov пишет: > Hi, > > Currently, any fuse request always includes inline pages[] array of > FUSE_MAX_PAGES_PER_REQ elements. This is the waste of memory because > in many cases smaller size would suffice. > ...
creator: Maxim Patlasov
date: 2012-09-10T10:18:41-00:00

Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: Hi Miklos, So far as no objections appeared, I'll go ahead and replace fuse req->page with req->pagevec. It will point to an array of structs: struct page_vec { struct page *pv_page; unsigned int pv_len; unsigned int pv_offset; }; instead of 'struct page *' as it used to be. It seems to be what you suggested in one of your comments. Are you OK about it? Thanks, Maxim 09/07/2012 09:40 PM, Maxim Patlasov пишет: > Hi, > > Currently, any fuse request always includes inline pages[] array of > FUSE_MAX_PAGES_PER_REQ elements. This is the waste of memory because > in many cases smaller size would suffice. > > The patch-set tries to allocate only as many elements of pages[] array as > actaully needed. This will be even more useful in the future because of: > > 1. Mitsuo's patches making maximum read/write request size tunable. > 2. My patches optimizing scatter-gather direct IO. To make them simplier I'll > need to substitute array of 'struct page *' with array of 'struct bio_vec'. > It would make memory overhead worse if implemented w/o this patch-set. > > Thanks, > Maxim >
creator: Maxim Patlasov
date: 2012-09-12T16:07:18-00:00

Re: [PATCH 1/6] 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. See comments inline. > --- > fs/fuse/dev.c | 40 ++++++++++++++++++++++++++++++---------- > fs/fuse/file.c | 4 ++-- > fs/fuse/fuse_i.h | 9 ++++++--- > fs/fuse/inode.c | 4 ++-- > 4 files changed, 40 insertions(+), 17 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 7df2b5e..c0283a1 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -36,32 +36,52 @@ static struct fuse_conn *fuse_get_conn(struct file *file) > > static void fuse_request_init(struct fuse_req *req) > { > + struct page **pages = req->pages; > + Rather than this, make fuse_request_init() take 'pages' as a second argument. > 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; > } > > -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) > + struct fuse_req *req = kmem_cache_alloc(fuse_req_cachep, flags); > + if (req) { > + if (npages <= 1) Instead of '1' use a constant (e.g. FUSE_REQ_INLINE_PAGES) > + req->pages = req->inline_pages; > + else > + req->pages = kmalloc(sizeof(struct page *) * npages, > + flags); > + > + if (!req->pages) { > + kmem_cache_free(fuse_req_cachep, req); > + return NULL; > + } > + > fuse_request_init(req); > + } > return req; > } > + > +struct fuse_req *fuse_request_alloc(int npages) > +{ > + return __fuse_request_alloc(npages, GFP_KERNEL); > +} > EXPORT_SYMBOL_G…
creator: Miklos Szeredi
date: 2012-09-12T16:09:01-00:00

Re: [PATCH 2/6] fuse: categorize fuse_get_req()
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch categorizes all fuse_get_req() invocations into three categories: > - fuse_get_req_nopages() - when caller doesn't care about req->pages > - fuse_get_req_onepage() - when caller need exactly one page > - fuse_get_req_multipage() - in other cases. I think you are overcomplicating this. Just have two functions: fuse_get_req_nopages() and fuse_get_req(). I don't think the _onepage variant deserves a separate helper. Thanks, Miklos
creator: Miklos Szeredi
date: 2012-09-12T16:20:15-00:00

Re: [PATCH 3/6] fuse: rework fuse_retrieve()
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > 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 moslty shuffling lines. > > The patch also contains one-line fix suggested by Miklos: the loop iterating > over pages from inode mapping should interpret 'offset' as offset in the very > first page only. That fix is already in -linus, so let's leave it out of this patch. Thanks, Miklos > --- > fs/fuse/dev.c | 26 ++++++++++++++++---------- > 1 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 6e38af5..cf69df3 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -1562,13 +1562,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_multipage(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_multipage(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; > @@ -1577,14 +1588,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 +…
creator: Miklos Szeredi
date: 2012-09-12T16:23:55-00:00

Re: [PATCH 4/6] fuse: rework fuse_readpages()
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch uses 'nr_pages' argument of fuse_readpages() as the heuristics for > 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 wheteher it's worthy or not. > --- > fs/fuse/file.c | 10 ++++++++-- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index c811f3d..9a6dcc6 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,8 +657,10 @@ 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_multipage(fc, FUSE_MAX_PAGES_PER_REQ); > + data->req = req = fuse_get_req_multipage(fc, nr_alloc); > if (IS_ERR(req)) { > unlock_page(page); > return PTR_ERR(req); > @@ -666,6 +669,7 @@ static int fuse_readpages_fill(void *_data, struct page *page) > page_cache_get(page); Okay, this is where things get hairy and where we should do something like: if (WARN_ON(req->num_pages >= req->max_pages)) return -EIO; > req->pages[req->num_pages] = page; > req->num_pages++; > + data->nr_pages--; > return 0; > } > > @@ -676,6 +680,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 +688,8 @@ static int fuse_…
creator: Miklos Szeredi
date: 2012-09-12T16:41:40-00:00

Re: [PATCH 5/6] fuse: rework fuse_perform_write()
type: item
description: Maxim Patlasov <mpatlasov@parallels.com> writes: > The patch allocates as many page pointers in fuse_req as needed to cover > interval [pos .. pos+len-1]. FUSE_WR_PAGES macro is introduced to hide this > cumbersome arithmetics. Please don't ever use a macro for something which you can use an inline function. > --- > fs/fuse/file.c | 15 +++++++++++---- > 1 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 9a6dcc6..84cc83c 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -815,7 +815,8 @@ static size_t fuse_send_write_pages(struct fuse_req *req, struct file *file, > > static ssize_t fuse_fill_write_pages(struct fuse_req *req, > struct address_space *mapping, > - struct iov_iter *ii, loff_t pos) > + struct iov_iter *ii, loff_t pos, > + unsigned nr_pages) > { > struct fuse_conn *fc = get_fuse_conn(mapping->host); > unsigned offset = pos & (PAGE_CACHE_SIZE - 1); > @@ -875,11 +876,16 @@ 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 < nr_pages && offset == 0); > > return count > 0 ? count : err; > } > > +#define FUSE_WR_PAGES(pos, len) 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) > @@ -895,14 +901,15 @@ 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_multipage(fc, FUSE_MAX_PAGES_PER_REQ); > + req = fuse_get_req_multipage(fc, nr_pages); > if (IS_ERR(req)) { > err = PTR_ERR(req); > break; > } > > - count = fuse_fill_write_pages(req, mapping, ii, pos); > + count = fuse_fill_write_pages(re…
creator: Miklos Szeredi
date: 2012-09-12T16:43:54-00:00

Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: "Maxim V. Patlasov" <mpatlasov@parallels.com> writes: > Hi Miklos, > > So far as no objections appeared, I'll go ahead and replace fuse req->page with > req->pagevec. It will point to an array of structs: > > struct page_vec { > struct page *pv_page; > unsigned int pv_len; > unsigned int pv_offset; > }; > > instead of 'struct page *' as it used to be. It seems to be what you suggested > in one of your comments. Are you OK about it? Yes, that's exactly what I was thinking. Thanks, Miklos
creator: Miklos Szeredi
date: 2012-09-12T16:49:01-00:00

Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: Hi Miklos, 09/12/2012 08:49 PM, Miklos Szeredi пишет: > "Maxim V. Patlasov" <mpatlasov@parallels.com> writes: > >> Hi Miklos, >> >> So far as no objections appeared, I'll go ahead and replace fuse req->page with >> req->pagevec. It will point to an array of structs: >> >> struct page_vec { >> struct page *pv_page; >> unsigned int pv_len; >> unsigned int pv_offset; >> }; >> >> instead of 'struct page *' as it used to be. It seems to be what you suggested >> in one of your comments. Are you OK about it? > Yes, that's exactly what I was thinking. I've encountered a problem while trying to follow this approach. fuse_get_user_pages() passes 'req->pages' to get_user_pages_fast(). get_user_pages_fast() and friends are not ready to get a pointer to array of page_vec-s from fuse. I can see five ways to solve the problem: 1. Re-work get_user_pages_fast() and friends adding ability to fill page_vec array. Too much work. Very ugly. I strongly dislike this way. 2. Allocate a temporary array of page pointers in fuse_get_user_pages() to use as argument to get_user_pages_fast(). Ugly and may have performance impact. I dislike this way too. 3. Call get_user_pages_fast() for each page (i.e. pass npages == 1 to it). Easy to implement but may have performance impact. I'd refrain from it. 4. Keep req->pages 'as is', but add req->page_descs pointing to an array of <offset, len> structures. Looks clumsy, straightforward, but quite doable. 5. Use a hack in fuse_get_user_pages(): temporarily cast req->pagevecs to 'struct page **pages', pass it get_user_pages_fast(), then transform the content of req->pagevecs[] to have page pointers stored in proper places (like 'for (i=...) pagevecs[i].pv_page = pages[i];'). What do you think? Btw, thanks a lot for careful review of patch-set. I agree with your comments. Next version will have those findings fixed. Thanks, Maxim
creator: Maxim Patlasov
date: 2012-09-14T13:18:17-00:00

Re: [PATCH 0/6] fuse: allocate req-&gt;pages[] dynamically
type: item
description: "Maxim V. Patlasov" <mpatlasov@parallels.com> writes: > Hi Miklos, > > 09/12/2012 08:49 PM, Miklos Szeredi пишет: >> "Maxim V. Patlasov" <mpatlasov@parallels.com> writes: >> >>> Hi Miklos, >>> >>> So far as no objections appeared, I'll go ahead and replace fuse req->page with >>> req->pagevec. It will point to an array of structs: >>> >>> struct page_vec { >>> struct page *pv_page; >>> unsigned int pv_len; >>> unsigned int pv_offset; >>> }; >>> >>> instead of 'struct page *' as it used to be. It seems to be what you suggested >>> in one of your comments. Are you OK about it? >> Yes, that's exactly what I was thinking. > > I've encountered a problem while trying to follow this > approach. fuse_get_user_pages() passes 'req->pages' to > get_user_pages_fast(). get_user_pages_fast() and friends are not ready to get a > pointer to array of page_vec-s from fuse. I can see five ways to solve the > problem: > > 1. Re-work get_user_pages_fast() and friends adding ability to fill page_vec > array. Too much work. Very ugly. I strongly dislike this way. > > 2. Allocate a temporary array of page pointers in fuse_get_user_pages() to use > as argument to get_user_pages_fast(). Ugly and may have performance impact. I > dislike this way too. > > 3. Call get_user_pages_fast() for each page (i.e. pass npages == 1 to it). Easy > to implement but may have performance impact. I'd refrain from it. > > 4. Keep req->pages 'as is', but add req->page_descs pointing to an array of > <offset, len> structures. Looks clumsy, straightforward, but quite > doable. > > 5. Use a hack in fuse_get_user_pages(): temporarily cast req->pagevecs to > struct page **pages', pass it get_user_pages_fast(), then transform the content > of req->pagevecs[] to have page pointers stored in proper places (like 'for > (i=...) pagevecs[i].pv_page = pages[i];'). > > What do you think? I'd go for number 4. Thanks, Miklos
creator: Miklos Szeredi
date: 2012-09-14T14:39:44-00:00