Menu
▾
▴
fuse-devel
|
From: Maxim P. <mpa...@pa...> - 2012-11-16 17:04:51
|
Hi,
This is the second iteration of Pavel Emelyanov's patch-set implementing
write-back policy for FUSE page cache. Initial patch-set description was
the following:
One of the problems with the existing FUSE implementation is that it uses the
write-through cache policy which results in performance problems on certain
workloads. E.g. when copying a big file into a FUSE file the cp pushes every
128k to the userspace synchronously. This becomes a problem when the userspace
back-end uses networking for storing the data.
A good solution of this is switching the FUSE page cache into a write-back policy.
With this file data are pushed to the userspace with big chunks (depending on the
dirty memory limits, but this is much more than 128k) which lets the FUSE daemons
handle the size updates in a more efficient manner.
The writeback feature is per-connection and is explicitly configurable at the
init stage (is it worth making it CAP_SOMETHING protected?) When the writeback is
turned ON:
* still copy writeback pages to temporary buffer when sending a writeback request
and finish the page writeback immediately
* make kernel maintain the inode's i_size to avoid frequent i_size synchronization
with the user space
* take NR_WRITEBACK_TEMP into account when makeing balance_dirty_pages decision.
This protects us from having too many dirty pages on FUSE
The provided patchset survives the fsx test. Performance measurements are not yet
all finished, but the mentioned copying of a huge file becomes noticeably faster
even on machines with few RAM and doesn't make the system stuck (the dirty pages
balancer does its work OK). Applies on top of v3.5-rc4.
We are currently exploring this with our own distributed storage implementation
which is heavily oriented on storing big blobs of data with extremely rare meta-data
updates (virtual machines' and containers' disk images). With the existing cache
policy a typical usage scenario -- copying a big VM disk into a cloud -- takes way
too much time to proceed, much longer than if it was simply scp-ed over the same
network. The write-back policy (as I mentioned) noticeably improves this scenario.
Kirill (in Cc) can share more details about the performance and the storage concepts
details if required.
Changed in v2:
- numerous bugfixes:
- fuse_write_begin and fuse_writepages_fill and fuse_writepage_locked must wait
on page writeback because page writeback can extend beyond the lifetime of
the page-cache page
- fuse_send_writepages can end_page_writeback on original page only after adding
request to fi->writepages list; otherwise another writeback may happen inside
the gap between end_page_writeback and adding to the list
- fuse_direct_io must wait on page writeback; otherwise data corruption is possible
due to reordering requests
- fuse_flush must flush dirty memory and wait for all writeback on given inode
before sending FUSE_FLUSH to userspace; otherwise FUSE_FLUSH is not reliable
- fuse_file_fallocate must hold i_mutex around FUSE_FALLOCATE and i_size update;
otherwise a race with a writer extending i_size is possible
- fix handling errors in fuse_writepages and fuse_send_writepages
- handle i_mtime intelligently if writeback cache is on (see patch #7 (update i_mtime
on buffered writes) for details.
- put enabling writeback cache under fusermount control; (see mount option
'allow_wbcache' introduced by patch #13 (turn writeback cache on))
- rebased on v3.7-rc5
Thanks,
Maxim
---
Maxim Patlasov (14):
fuse: Linking file to inode helper
fuse: Getting file for writeback helper
fuse: Prepare to handle short reads
fuse: Prepare to handle multiple pages in writeback
fuse: Connection bit for enabling writeback
fuse: Trust kernel i_size only
fuse: Update i_mtime on buffered writes
fuse: Flush files on wb close
fuse: Implement writepages and write_begin/write_end callbacks
fuse: fuse_writepage_locked() should wait on writeback
fuse: fuse_flush() should wait on writeback
fuse: Fix O_DIRECT operations vs cached writeback misorder
fuse: Turn writeback cache on
mm: Account for WRITEBACK_TEMP in balance_dirty_pages
fs/fuse/dir.c | 51 ++++
fs/fuse/file.c | 523 +++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 20 ++
fs/fuse/inode.c | 98 ++++++++
include/uapi/linux/fuse.h | 1
mm/page-writeback.c | 3
6 files changed, 638 insertions(+), 58 deletions(-)
--
Signature
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:20:51
|
Hi,
This is the second iteration of Pavel Emelyanov's patch-set implementing
write-back policy for FUSE page cache. Initial patch-set description was
the following:
One of the problems with the existing FUSE implementation is that it uses the
write-through cache policy which results in performance problems on certain
workloads. E.g. when copying a big file into a FUSE file the cp pushes every
128k to the userspace synchronously. This becomes a problem when the userspace
back-end uses networking for storing the data.
A good solution of this is switching the FUSE page cache into a write-back policy.
With this file data are pushed to the userspace with big chunks (depending on the
dirty memory limits, but this is much more than 128k) which lets the FUSE daemons
handle the size updates in a more efficient manner.
The writeback feature is per-connection and is explicitly configurable at the
init stage (is it worth making it CAP_SOMETHING protected?) When the writeback is
turned ON:
* still copy writeback pages to temporary buffer when sending a writeback request
and finish the page writeback immediately
* make kernel maintain the inode's i_size to avoid frequent i_size synchronization
with the user space
* take NR_WRITEBACK_TEMP into account when makeing balance_dirty_pages decision.
This protects us from having too many dirty pages on FUSE
The provided patchset survives the fsx test. Performance measurements are not yet
all finished, but the mentioned copying of a huge file becomes noticeably faster
even on machines with few RAM and doesn't make the system stuck (the dirty pages
balancer does its work OK). Applies on top of v3.5-rc4.
We are currently exploring this with our own distributed storage implementation
which is heavily oriented on storing big blobs of data with extremely rare meta-data
updates (virtual machines' and containers' disk images). With the existing cache
policy a typical usage scenario -- copying a big VM disk into a cloud -- takes way
too much time to proceed, much longer than if it was simply scp-ed over the same
network. The write-back policy (as I mentioned) noticeably improves this scenario.
Kirill (in Cc) can share more details about the performance and the storage concepts
details if required.
Changed in v2:
- numerous bugfixes:
- fuse_write_begin and fuse_writepages_fill and fuse_writepage_locked must wait
on page writeback because page writeback can extend beyond the lifetime of
the page-cache page
- fuse_send_writepages can end_page_writeback on original page only after adding
request to fi->writepages list; otherwise another writeback may happen inside
the gap between end_page_writeback and adding to the list
- fuse_direct_io must wait on page writeback; otherwise data corruption is possible
due to reordering requests
- fuse_flush must flush dirty memory and wait for all writeback on given inode
before sending FUSE_FLUSH to userspace; otherwise FUSE_FLUSH is not reliable
- fuse_file_fallocate must hold i_mutex around FUSE_FALLOCATE and i_size update;
otherwise a race with a writer extending i_size is possible
- fix handling errors in fuse_writepages and fuse_send_writepages
- handle i_mtime intelligently if writeback cache is on (see patch #7 (update i_mtime
on buffered writes) for details.
- put enabling writeback cache under fusermount control; (see mount option
'allow_wbcache' introduced by patch #13 (turn writeback cache on))
- rebased on v3.7-rc5
Changed in v3:
- rebased on for-next branch of the fuse tree
Thanks,
Maxim
---
Maxim V. Patlasov (14):
fuse: Linking file to inode helper
fuse: Getting file for writeback helper
fuse: Prepare to handle short reads
fuse: Prepare to handle multiple pages in writeback
fuse: Connection bit for enabling writeback
fuse: Trust kernel i_size only - v2
fuse: Update i_mtime on buffered writes
fuse: Flush files on wb close
fuse: Implement writepages and write_begin/write_end callbacks - v2
fuse: fuse_writepage_locked() should wait on writeback
fuse: fuse_flush() should wait on writeback
fuse: Fix O_DIRECT operations vs cached writeback misorder - v2
fuse: Turn writeback cache on
mm: Account for WRITEBACK_TEMP in balance_dirty_pages
fs/fuse/cuse.c | 5
fs/fuse/dir.c | 51 +++-
fs/fuse/file.c | 567 +++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 33 ++-
fs/fuse/inode.c | 98 ++++++++
include/uapi/linux/fuse.h | 2
mm/page-writeback.c | 3
7 files changed, 696 insertions(+), 63 deletions(-)
--
Signature
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:21:08
|
When writeback is ON every writeable file should be in per-inode write list,
not only mmap-ed ones. Thus introduce a helper for this linkage.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
Signed-off-by: Pavel Emelyanov <xe...@op...>
---
fs/fuse/file.c | 33 +++++++++++++++++++--------------
1 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 28bc9c6..9bdc0fa 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -167,6 +167,22 @@ int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
}
EXPORT_SYMBOL_GPL(fuse_do_open);
+static void fuse_link_write_file(struct file *file)
+{
+ struct inode *inode = file->f_dentry->d_inode;
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ struct fuse_file *ff = file->private_data;
+ /*
+ * file may be written through mmap, so chain it onto the
+ * inodes's write_file list
+ */
+ spin_lock(&fc->lock);
+ if (list_empty(&ff->write_entry))
+ list_add(&ff->write_entry, &fi->write_files);
+ spin_unlock(&fc->lock);
+}
+
void fuse_finish_open(struct inode *inode, struct file *file)
{
struct fuse_file *ff = file->private_data;
@@ -1484,20 +1500,9 @@ static const struct vm_operations_struct fuse_file_vm_ops = {
static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
{
- if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE)) {
- struct inode *inode = file->f_dentry->d_inode;
- struct fuse_conn *fc = get_fuse_conn(inode);
- struct fuse_inode *fi = get_fuse_inode(inode);
- struct fuse_file *ff = file->private_data;
- /*
- * file may be written through mmap, so chain it onto the
- * inodes's write_file list
- */
- spin_lock(&fc->lock);
- if (list_empty(&ff->write_entry))
- list_add(&ff->write_entry, &fi->write_files);
- spin_unlock(&fc->lock);
- }
+ if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
+ fuse_link_write_file(file);
+
file_accessed(file);
vma->vm_ops = &fuse_file_vm_ops;
return 0;
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:21:24
|
There will be a .writepageS callback implementation which will need to
get a fuse_file out of a fuse_inode, thus make a helper for this.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
Signed-off-by: Pavel Emelyanov <xe...@op...>
---
fs/fuse/file.c | 24 ++++++++++++++++--------
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 9bdc0fa..ad89b21 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1375,6 +1375,20 @@ static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_req *req)
fuse_writepage_free(fc, req);
}
+static struct fuse_file *fuse_write_file(struct fuse_conn *fc,
+ struct fuse_inode *fi)
+{
+ struct fuse_file *ff;
+
+ spin_lock(&fc->lock);
+ BUG_ON(list_empty(&fi->write_files));
+ ff = list_entry(fi->write_files.next, struct fuse_file, write_entry);
+ fuse_file_get(ff);
+ spin_unlock(&fc->lock);
+
+ return ff;
+}
+
static int fuse_writepage_locked(struct page *page)
{
struct address_space *mapping = page->mapping;
@@ -1382,7 +1396,6 @@ static int fuse_writepage_locked(struct page *page)
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_inode *fi = get_fuse_inode(inode);
struct fuse_req *req;
- struct fuse_file *ff;
struct page *tmp_page;
set_page_writeback(page);
@@ -1395,13 +1408,8 @@ static int fuse_writepage_locked(struct page *page)
if (!tmp_page)
goto err_free;
- spin_lock(&fc->lock);
- BUG_ON(list_empty(&fi->write_files));
- ff = list_entry(fi->write_files.next, struct fuse_file, write_entry);
- req->ff = fuse_file_get(ff);
- spin_unlock(&fc->lock);
-
- fuse_write_fill(req, ff, page_offset(page), 0);
+ req->ff = fuse_write_file(fc, fi);
+ fuse_write_fill(req, req->ff, page_offset(page), 0);
copy_highpage(tmp_page, page);
req->misc.write.in.write_flags |= FUSE_WRITE_CACHE;
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:21:48
|
A helper which gets called when read reports less bytes than was requested.
See patch #6 (trust kernel i_size only) for details.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
Signed-off-by: Pavel Emelyanov <xe...@op...>
---
fs/fuse/file.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index ad89b21..1d76283 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -538,6 +538,15 @@ static void fuse_read_update_size(struct inode *inode, loff_t size,
spin_unlock(&fc->lock);
}
+static void fuse_short_read(struct fuse_req *req, struct inode *inode,
+ u64 attr_ver)
+{
+ size_t num_read = req->out.args[0].size;
+
+ loff_t pos = page_offset(req->pages[0]) + num_read;
+ fuse_read_update_size(inode, pos, attr_ver);
+}
+
static int fuse_readpage(struct file *file, struct page *page)
{
struct inode *inode = page->mapping->host;
@@ -574,18 +583,18 @@ static int fuse_readpage(struct file *file, struct page *page)
req->page_descs[0].length = count;
num_read = fuse_send_read(req, file, pos, count, NULL);
err = req->out.h.error;
- fuse_put_request(fc, req);
if (!err) {
/*
* Short read means EOF. If file size is larger, truncate it
*/
if (num_read < count)
- fuse_read_update_size(inode, pos + num_read, attr_ver);
+ fuse_short_read(req, inode, attr_ver);
SetPageUptodate(page);
}
+ fuse_put_request(fc, req);
fuse_invalidate_attr(inode); /* atime changed */
out:
unlock_page(page);
@@ -608,13 +617,9 @@ static void fuse_readpages_end(struct fuse_conn *fc, struct fuse_req *req)
/*
* Short read means EOF. If file size is larger, truncate it
*/
- if (!req->out.h.error && num_read < count) {
- loff_t pos;
+ if (!req->out.h.error && num_read < count)
+ fuse_short_read(req, inode, req->misc.read.attr_ver);
- pos = page_offset(req->pages[0]) + num_read;
- fuse_read_update_size(inode, pos,
- req->misc.read.attr_ver);
- }
fuse_invalidate_attr(inode); /* atime changed */
}
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:22:06
|
The .writepages callback will issue writeback requests with more than one
page aboard. Make existing end/check code be aware of this.
Original patch by: Pavel Emelyanov <xe...@op...>
Signed-off-by: Maxim Patlasov <MPa...@pa...>
---
fs/fuse/file.c | 22 +++++++++++++++-------
1 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 1d76283..b28be33 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -345,7 +345,8 @@ static bool fuse_page_is_writeback(struct inode *inode, pgoff_t index)
BUG_ON(req->inode != inode);
curr_index = req->misc.write.in.offset >> PAGE_CACHE_SHIFT;
- if (curr_index == index) {
+ if (curr_index <= index &&
+ index < curr_index + req->num_pages) {
found = true;
break;
}
@@ -1295,7 +1296,10 @@ static ssize_t fuse_direct_write(struct file *file, const char __user *buf,
static void fuse_writepage_free(struct fuse_conn *fc, struct fuse_req *req)
{
- __free_page(req->pages[0]);
+ int i;
+
+ for (i = 0; i < req->num_pages; i++)
+ __free_page(req->pages[i]);
fuse_file_put(req->ff, false);
}
@@ -1304,10 +1308,13 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
struct inode *inode = req->inode;
struct fuse_inode *fi = get_fuse_inode(inode);
struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
+ int i;
list_del(&req->writepages_entry);
- dec_bdi_stat(bdi, BDI_WRITEBACK);
- dec_zone_page_state(req->pages[0], NR_WRITEBACK_TEMP);
+ for (i = 0; i < req->num_pages; i++) {
+ dec_bdi_stat(bdi, BDI_WRITEBACK);
+ dec_zone_page_state(req->pages[i], NR_WRITEBACK_TEMP);
+ }
bdi_writeout_inc(bdi);
wake_up(&fi->page_waitq);
}
@@ -1320,14 +1327,15 @@ __acquires(fc->lock)
struct fuse_inode *fi = get_fuse_inode(req->inode);
loff_t size = i_size_read(req->inode);
struct fuse_write_in *inarg = &req->misc.write.in;
+ __u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
if (!fc->connected)
goto out_free;
- if (inarg->offset + PAGE_CACHE_SIZE <= size) {
- inarg->size = PAGE_CACHE_SIZE;
+ if (inarg->offset + data_size <= size) {
+ inarg->size = data_size;
} else if (inarg->offset < size) {
- inarg->size = size & (PAGE_CACHE_SIZE - 1);
+ inarg->size = size - inarg->offset;
} else {
/* Got truncated off completely */
goto out_free;
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:22:21
|
Off (0) by default. Will be used in the next patches and will be turned
on at the very end.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
Signed-off-by: Pavel Emelyanov <xe...@op...>
---
fs/fuse/fuse_i.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 13befcd..65d76cd 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -446,6 +446,9 @@ struct fuse_conn {
/** Set if bdi is valid */
unsigned bdi_initialized:1;
+ /** write-back cache policy (default is write-through) */
+ unsigned writeback_cache:1;
+
/*
* The following bitfields are only for optimization purposes
* and hence races in setting them will not cause malfunction
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:22:57
|
Make fuse think that when writeback is on the inode's i_size is always
up-to-date and not update it with the value received from the userspace.
This is done because the page cache code may update i_size without letting
the FS know.
This assumption implies fixing the previously introduced short-read helper --
when a short read occurs the 'hole' is filled with zeroes.
fuse_file_fallocate() is also fixed because now we should keep i_size up to
date, so it must be updated if FUSE_FALLOCATE request succeeded.
Changed in v2:
- improved comment in fuse_short_read()
- fixed fuse_file_fallocate() for KEEP_SIZE mode
Original patch by: Pavel Emelyanov <xe...@op...>
Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
---
fs/fuse/dir.c | 9 ++++++---
fs/fuse/file.c | 43 +++++++++++++++++++++++++++++++++++++++++--
fs/fuse/inode.c | 6 ++++--
3 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ed8f8c5..ff8b603 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -827,7 +827,7 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
stat->mtime.tv_nsec = attr->mtimensec;
stat->ctime.tv_sec = attr->ctime;
stat->ctime.tv_nsec = attr->ctimensec;
- stat->size = attr->size;
+ stat->size = i_size_read(inode);
stat->blocks = attr->blocks;
if (attr->blksize != 0)
@@ -1541,6 +1541,7 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
struct fuse_setattr_in inarg;
struct fuse_attr_out outarg;
bool is_truncate = false;
+ bool is_wb = fc->writeback_cache;
loff_t oldsize;
int err;
@@ -1613,7 +1614,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
fuse_change_attributes_common(inode, &outarg.attr,
attr_timeout(&outarg));
oldsize = inode->i_size;
- i_size_write(inode, outarg.attr.size);
+ if (!is_wb || is_truncate || !S_ISREG(inode->i_mode))
+ i_size_write(inode, outarg.attr.size);
if (is_truncate) {
/* NOTE: this may release/reacquire fc->lock */
@@ -1625,7 +1627,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
* Only call invalidate_inode_pages2() after removing
* FUSE_NOWRITE, otherwise fuse_launder_page() would deadlock.
*/
- if (S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
+ if ((is_truncate || !is_wb) &&
+ S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
truncate_pagecache(inode, oldsize, outarg.attr.size);
invalidate_inode_pages2(inode->i_mapping);
}
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index b28be33..6b64e11 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/compat.h>
#include <linux/swap.h>
+#include <linux/falloc.h>
static const struct file_operations fuse_direct_io_file_operations;
@@ -543,9 +544,31 @@ static void fuse_short_read(struct fuse_req *req, struct inode *inode,
u64 attr_ver)
{
size_t num_read = req->out.args[0].size;
+ struct fuse_conn *fc = get_fuse_conn(inode);
+
+ if (fc->writeback_cache) {
+ /*
+ * A hole in a file. Some data after the hole are in page cache,
+ * but have not reached the client fs yet. So, the hole is not
+ * present there.
+ */
+ int i;
+ int start_idx = num_read >> PAGE_CACHE_SHIFT;
+ size_t off = num_read & (PAGE_CACHE_SIZE - 1);
- loff_t pos = page_offset(req->pages[0]) + num_read;
- fuse_read_update_size(inode, pos, attr_ver);
+ for (i = start_idx; i < req->num_pages; i++) {
+ struct page *page = req->pages[i];
+ void *mapaddr = kmap_atomic(page);
+
+ memset(mapaddr + off, 0, PAGE_CACHE_SIZE - off);
+
+ kunmap_atomic(mapaddr);
+ off = 0;
+ }
+ } else {
+ loff_t pos = page_offset(req->pages[0]) + num_read;
+ fuse_read_update_size(inode, pos, attr_ver);
+ }
}
static int fuse_readpage(struct file *file, struct page *page)
@@ -2285,6 +2308,8 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
.mode = mode
};
int err;
+ bool change_i_size = fc->writeback_cache &&
+ !(mode & FALLOC_FL_KEEP_SIZE);
if (fc->no_fallocate)
return -EOPNOTSUPP;
@@ -2293,6 +2318,11 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
if (IS_ERR(req))
return PTR_ERR(req);
+ if (change_i_size) {
+ struct inode *inode = file->f_mapping->host;
+ mutex_lock(&inode->i_mutex);
+ }
+
req->in.h.opcode = FUSE_FALLOCATE;
req->in.h.nodeid = ff->nodeid;
req->in.numargs = 1;
@@ -2306,6 +2336,15 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
}
fuse_put_request(fc, req);
+ if (change_i_size) {
+ struct inode *inode = file->f_mapping->host;
+
+ if (!err)
+ fuse_write_update_size(inode, offset + length);
+
+ mutex_unlock(&inode->i_mutex);
+ }
+
return err;
}
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 9d95a5a..7e07dbd 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -196,6 +196,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
{
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_inode *fi = get_fuse_inode(inode);
+ bool is_wb = fc->writeback_cache;
loff_t oldsize;
struct timespec old_mtime;
@@ -209,10 +210,11 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
fuse_change_attributes_common(inode, attr, attr_valid);
oldsize = inode->i_size;
- i_size_write(inode, attr->size);
+ if (!is_wb || !S_ISREG(inode->i_mode))
+ i_size_write(inode, attr->size);
spin_unlock(&fc->lock);
- if (S_ISREG(inode->i_mode)) {
+ if (!is_wb && S_ISREG(inode->i_mode)) {
bool inval = false;
if (oldsize != attr->size) {
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:24:20
|
If writeback cache is on, buffered write doesn't result in immediate mtime
update in userspace because the userspace will see modified data later, when
writeback happens. Consequently, mtime provided by userspace may be older than
actual time of buffered write.
The problem can be solved by generating mtime locally (will come in next
patches) and flushing it to userspace periodically. Here we introduce a flag to
keep the state of fuse_inode: the flag is ON if and only if locally generated
mtime (stored in inode->i_mtime) was not pushed to the userspace yet.
The patch also implements all bits related to flushing and clearing the flag.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
---
fs/fuse/dir.c | 42 +++++++++++++++++++++++++----
fs/fuse/file.c | 31 ++++++++++++++++++---
fs/fuse/fuse_i.h | 13 ++++++++-
fs/fuse/inode.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 154 insertions(+), 11 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ff8b603..969c60d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -177,6 +177,13 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
if (flags & LOOKUP_RCU)
return -ECHILD;
+ if (test_bit(FUSE_I_MTIME_UPDATED,
+ &get_fuse_inode(inode)->state)) {
+ err = fuse_flush_mtime(inode, 0);
+ if (err)
+ return 0;
+ }
+
fc = get_fuse_conn(inode);
req = fuse_get_req_nopages(fc);
if (IS_ERR(req))
@@ -839,7 +846,7 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
}
static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
- struct file *file)
+ struct file *file, int locked)
{
int err;
struct fuse_getattr_in inarg;
@@ -848,6 +855,12 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
struct fuse_req *req;
u64 attr_version;
+ if (test_bit(FUSE_I_MTIME_UPDATED, &get_fuse_inode(inode)->state)) {
+ err = fuse_flush_mtime(inode, locked);
+ if (err)
+ return err;
+ }
+
req = fuse_get_req_nopages(fc);
if (IS_ERR(req))
return PTR_ERR(req);
@@ -893,7 +906,7 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
}
int fuse_update_attributes(struct inode *inode, struct kstat *stat,
- struct file *file, bool *refreshed)
+ struct file *file, bool *refreshed, int locked)
{
struct fuse_inode *fi = get_fuse_inode(inode);
int err;
@@ -901,7 +914,7 @@ int fuse_update_attributes(struct inode *inode, struct kstat *stat,
if (fi->i_time < get_jiffies_64()) {
r = true;
- err = fuse_do_getattr(inode, stat, file);
+ err = fuse_do_getattr(inode, stat, file, locked);
} else {
r = false;
err = 0;
@@ -1055,7 +1068,7 @@ static int fuse_perm_getattr(struct inode *inode, int mask)
if (mask & MAY_NOT_BLOCK)
return -ECHILD;
- return fuse_do_getattr(inode, NULL, NULL);
+ return fuse_do_getattr(inode, NULL, NULL, 0);
}
/*
@@ -1524,6 +1537,12 @@ void fuse_release_nowrite(struct inode *inode)
spin_unlock(&fc->lock);
}
+static inline bool fuse_operation_updates_mtime_on_server(unsigned ivalid)
+{
+ return (ivalid & ATTR_SIZE) ||
+ ((ivalid & ATTR_MTIME) && update_mtime(ivalid));
+}
+
/*
* Set attributes, and at the same time refresh them.
*
@@ -1564,6 +1583,15 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
if (attr->ia_valid & ATTR_SIZE)
is_truncate = true;
+ if (!fuse_operation_updates_mtime_on_server(attr->ia_valid)) {
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ if (test_bit(FUSE_I_MTIME_UPDATED, &fi->state)) {
+ err = fuse_flush_mtime(inode, 1);
+ if (err)
+ return err;
+ }
+ }
+
req = fuse_get_req_nopages(fc);
if (IS_ERR(req))
return PTR_ERR(req);
@@ -1611,6 +1639,10 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
}
spin_lock(&fc->lock);
+ if (fuse_operation_updates_mtime_on_server(attr->ia_valid)) {
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ clear_bit(FUSE_I_MTIME_UPDATED, &fi->state);
+ }
fuse_change_attributes_common(inode, &outarg.attr,
attr_timeout(&outarg));
oldsize = inode->i_size;
@@ -1659,7 +1691,7 @@ static int fuse_getattr(struct vfsmount *mnt, struct dentry *entry,
if (!fuse_allow_task(fc, current))
return -EACCES;
- return fuse_update_attributes(inode, stat, NULL, NULL);
+ return fuse_update_attributes(inode, stat, NULL, NULL, 0);
}
static int fuse_setxattr(struct dentry *entry, const char *name,
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 6b64e11..4f8fa45 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -383,6 +383,13 @@ static int fuse_flush(struct file *file, fl_owner_t id)
if (is_bad_inode(inode))
return -EIO;
+ if (test_bit(FUSE_I_MTIME_UPDATED,
+ &get_fuse_inode(inode)->state)) {
+ err = fuse_flush_mtime(inode, 0);
+ if (err)
+ return err;
+ }
+
if (fc->no_flush)
return 0;
@@ -486,6 +493,15 @@ out:
static int fuse_fsync(struct file *file, loff_t start, loff_t end,
int datasync)
{
+ struct inode *inode = file->f_mapping->host;
+
+ if (test_bit(FUSE_I_MTIME_UPDATED,
+ &get_fuse_inode(inode)->state)) {
+ int err = fuse_flush_mtime(inode, 0);
+ if (err)
+ return err;
+ }
+
return fuse_fsync_common(file, start, end, datasync, 0);
}
@@ -772,7 +788,8 @@ static ssize_t fuse_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
if (fc->auto_inval_data ||
(pos + iov_length(iov, nr_segs) > i_size_read(inode))) {
int err;
- err = fuse_update_attributes(inode, NULL, iocb->ki_filp, NULL);
+ err = fuse_update_attributes(inode, NULL, iocb->ki_filp, NULL,
+ 0);
if (err)
return err;
}
@@ -1290,8 +1307,11 @@ static ssize_t __fuse_direct_write(struct file *file, const struct iovec *iov,
res = generic_write_checks(file, ppos, &count, 0);
if (!res) {
res = fuse_direct_io(file, iov, nr_segs, count, ppos, 1);
- if (res > 0)
+ if (res > 0) {
+ struct fuse_inode *fi = get_fuse_inode(inode);
fuse_write_update_size(inode, *ppos);
+ clear_bit(FUSE_I_MTIME_UPDATED, &fi->state);
+ }
}
fuse_invalidate_attr(inode);
@@ -1758,7 +1778,7 @@ static loff_t fuse_file_llseek(struct file *file, loff_t offset, int whence)
return generic_file_llseek(file, offset, whence);
mutex_lock(&inode->i_mutex);
- retval = fuse_update_attributes(inode, NULL, file, NULL);
+ retval = fuse_update_attributes(inode, NULL, file, NULL, 1);
if (!retval)
retval = generic_file_llseek(file, offset, whence);
mutex_unlock(&inode->i_mutex);
@@ -2339,8 +2359,11 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
if (change_i_size) {
struct inode *inode = file->f_mapping->host;
- if (!err)
+ if (!err) {
+ struct fuse_inode *fi = get_fuse_inode(inode);
fuse_write_update_size(inode, offset + length);
+ clear_bit(FUSE_I_MTIME_UPDATED, &fi->state);
+ }
mutex_unlock(&inode->i_mutex);
}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 65d76cd..fdeccc5 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -106,6 +106,15 @@ struct fuse_inode {
/** List of writepage requestst (pending or sent) */
struct list_head writepages;
+
+ /** Miscellaneous bits describing inode state */
+ unsigned long state;
+};
+
+/** FUSE inode state bits */
+enum {
+ /** i_mtime has been updated locally; a flush to userspace needed */
+ FUSE_I_MTIME_UPDATED,
};
struct fuse_conn;
@@ -784,7 +793,7 @@ int fuse_allow_task(struct fuse_conn *fc, struct task_struct *task);
u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id);
int fuse_update_attributes(struct inode *inode, struct kstat *stat,
- struct file *file, bool *refreshed);
+ struct file *file, bool *refreshed, int locked);
void fuse_flush_writepages(struct inode *inode);
@@ -826,4 +835,6 @@ int fuse_dev_release(struct inode *inode, struct file *file);
void fuse_write_update_size(struct inode *inode, loff_t pos);
+int fuse_flush_mtime(struct inode *inode, int locked);
+
#endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 7e07dbd..3687daf 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -201,7 +201,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
struct timespec old_mtime;
spin_lock(&fc->lock);
- if (attr_version != 0 && fi->attr_version > attr_version) {
+ if ((attr_version != 0 && fi->attr_version > attr_version) ||
+ test_bit(FUSE_I_MTIME_UPDATED, &fi->state)) {
spin_unlock(&fc->lock);
return;
}
@@ -257,6 +258,8 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
new_decode_dev(attr->rdev));
} else
BUG();
+
+ get_fuse_inode(inode)->state = 0;
}
int fuse_inode_eq(struct inode *inode, void *_nodeidp)
@@ -335,6 +338,80 @@ int fuse_reverse_inval_inode(struct super_block *sb, u64 nodeid,
return 0;
}
+/*
+ * Flush inode->i_mtime to the server and clear FUSE_I_MTIME_UPDATED flag
+ *
+ * Do nothing if anybody cleared FUSE_I_MTIME_UPDATED flag by the time we
+ * acquired i_mutex.
+ *
+ * Do not clear FUSE_I_MTIME_UPDATED flag after flush if anybody (buffered
+ * write) updated i_mtime by the time we acquired fc->lock.
+ */
+int fuse_flush_mtime(struct inode *inode, int locked)
+{
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_req *req;
+ struct fuse_setattr_in inarg;
+ struct fuse_attr_out outarg;
+ int err;
+
+ req = fuse_get_req_nopages(fc);
+ if (IS_ERR(req))
+ return PTR_ERR(req);
+
+ memset(&inarg, 0, sizeof(inarg));
+ memset(&outarg, 0, sizeof(outarg));
+
+ if (!locked)
+ mutex_lock(&inode->i_mutex);
+
+ /*
+ * This is crucial. We must re-check flag holding i_mutex. Otherwise
+ * it would be possible to overwrite fresh mtime on server (for
+ * example, updated as result of dio write) with our already outdated
+ * inode->i_mtime.
+ */
+ if (!test_bit(FUSE_I_MTIME_UPDATED, &fi->state)) {
+ mutex_unlock(&inode->i_mutex);
+ fuse_put_request(fc, req);
+ return 0;
+ }
+
+ inarg.valid |= FATTR_MTIME;
+ inarg.mtime = inode->i_mtime.tv_sec;
+ inarg.mtimensec = inode->i_mtime.tv_nsec;
+
+ req->in.h.opcode = FUSE_SETATTR;
+ req->in.h.nodeid = get_node_id(inode);
+ req->in.numargs = 1;
+ req->in.args[0].size = sizeof(inarg);
+ req->in.args[0].value = &inarg;
+ req->out.numargs = 1;
+ if (fc->minor < 9)
+ req->out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE;
+ else
+ req->out.args[0].size = sizeof(outarg);
+ req->out.args[0].value = &outarg;
+
+ fuse_request_send(fc, req);
+ err = req->out.h.error;
+ fuse_put_request(fc, req);
+
+ if (!err) {
+ spin_lock(&fc->lock);
+ if (inarg.mtime == inode->i_mtime.tv_sec &&
+ inarg.mtimensec == inode->i_mtime.tv_nsec)
+ clear_bit(FUSE_I_MTIME_UPDATED, &fi->state);
+ spin_unlock(&fc->lock);
+ }
+
+ if (!locked)
+ mutex_unlock(&inode->i_mutex);
+
+ return err;
+}
+
static void fuse_umount_begin(struct super_block *sb)
{
fuse_abort_conn(get_fuse_conn_super(sb));
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:24:36
|
Any write request requires a file handle to report to the userspace. Thus
when we close a file (and free the fuse_file with this info) we have to
flush all the outstanding writeback cache. Note, that simply calling the
filemap_write_and_wait() is not enough since fuse finishes page writeback
immediately and thus the -wait part of the mentioned call will be no-op.
Do real wait on per-inode writepages list.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
Signed-off-by: Pavel Emelyanov <xe...@op...>
---
fs/fuse/file.c | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 4f8fa45..496e74c 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -138,6 +138,12 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
}
}
+static void __fuse_file_put(struct fuse_file *ff)
+{
+ if (atomic_dec_and_test(&ff->count))
+ BUG();
+}
+
int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
bool isdir)
{
@@ -286,8 +292,23 @@ static int fuse_open(struct inode *inode, struct file *file)
return fuse_open_common(inode, file, false);
}
+static void fuse_flush_writeback(struct inode *inode, struct file *file)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_inode *fi = get_fuse_inode(inode);
+
+ filemap_write_and_wait(file->f_mapping);
+ wait_event(fi->page_waitq, list_empty_careful(&fi->writepages));
+ spin_unlock_wait(&fc->lock);
+}
+
static int fuse_release(struct inode *inode, struct file *file)
{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+
+ if (fc->writeback_cache)
+ fuse_flush_writeback(inode, file);
+
fuse_release_common(file, FUSE_RELEASE);
/* return value is ignored by VFS */
@@ -1343,7 +1364,8 @@ static void fuse_writepage_free(struct fuse_conn *fc, struct fuse_req *req)
for (i = 0; i < req->num_pages; i++)
__free_page(req->pages[i]);
- fuse_file_put(req->ff, false);
+ if (!fc->writeback_cache)
+ fuse_file_put(req->ff, false);
}
static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
@@ -1360,6 +1382,8 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
}
bdi_writeout_inc(bdi);
wake_up(&fi->page_waitq);
+ if (fc->writeback_cache)
+ __fuse_file_put(req->ff);
}
/* Called under fc->lock, may release and reacquire it */
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:25:40
|
The .writepages one is required to make each writeback request carry more than
one page on it.
Changed in v2:
- fixed fuse_prepare_write() to avoid reads beyond EOF
- fixed fuse_prepare_write() to zero uninitialized part of page
Original patch by: Pavel Emelyanov <xe...@op...>
Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
---
fs/fuse/file.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 281 insertions(+), 1 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 496e74c..3b4dc98 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -722,7 +722,10 @@ static void fuse_send_readpages(struct fuse_req *req, struct file *file)
struct fuse_fill_data {
struct fuse_req *req;
- struct file *file;
+ union {
+ struct file *file;
+ struct fuse_file *ff;
+ };
struct inode *inode;
unsigned nr_pages;
};
@@ -1530,6 +1533,280 @@ static int fuse_writepage(struct page *page, struct writeback_control *wbc)
return err;
}
+static int fuse_send_writepages(struct fuse_fill_data *data)
+{
+ int i, all_ok = 1;
+ struct fuse_req *req = data->req;
+ struct inode *inode = data->inode;
+ struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ loff_t off = -1;
+
+ if (!data->ff)
+ data->ff = fuse_write_file(fc, fi);
+
+ if (!data->ff) {
+ for (i = 0; i < req->num_pages; i++)
+ end_page_writeback(req->pages[i]);
+ return -EIO;
+ }
+
+ req->inode = inode;
+ req->misc.write.in.offset = page_offset(req->pages[0]);
+
+ spin_lock(&fc->lock);
+ list_add(&req->writepages_entry, &fi->writepages);
+ spin_unlock(&fc->lock);
+
+ for (i = 0; i < req->num_pages; i++) {
+ struct page *page = req->pages[i];
+ struct page *tmp_page;
+
+ tmp_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+ if (tmp_page) {
+ copy_highpage(tmp_page, page);
+ inc_bdi_stat(bdi, BDI_WRITEBACK);
+ inc_zone_page_state(tmp_page, NR_WRITEBACK_TEMP);
+ } else
+ all_ok = 0;
+ req->pages[i] = tmp_page;
+ if (i == 0)
+ off = page_offset(page);
+
+ end_page_writeback(page);
+ }
+
+ if (!all_ok) {
+ for (i = 0; i < req->num_pages; i++) {
+ struct page *page = req->pages[i];
+ if (page) {
+ dec_bdi_stat(bdi, BDI_WRITEBACK);
+ dec_zone_page_state(page, NR_WRITEBACK_TEMP);
+ __free_page(page);
+ req->pages[i] = NULL;
+ }
+ }
+
+ spin_lock(&fc->lock);
+ list_del(&req->writepages_entry);
+ wake_up(&fi->page_waitq);
+ spin_unlock(&fc->lock);
+ return -ENOMEM;
+ }
+
+ req->ff = fuse_file_get(data->ff);
+ fuse_write_fill(req, data->ff, off, 0);
+
+ req->misc.write.in.write_flags |= FUSE_WRITE_CACHE;
+ req->in.argpages = 1;
+ fuse_page_descs_length_init(req, 0, req->num_pages);
+ req->end = fuse_writepage_end;
+
+ spin_lock(&fc->lock);
+ list_add_tail(&req->list, &fi->queued_writes);
+ fuse_flush_writepages(data->inode);
+ spin_unlock(&fc->lock);
+
+ return 0;
+}
+
+static int fuse_writepages_fill(struct page *page,
+ struct writeback_control *wbc, void *_data)
+{
+ struct fuse_fill_data *data = _data;
+ struct fuse_req *req = data->req;
+ struct inode *inode = data->inode;
+ struct fuse_conn *fc = get_fuse_conn(inode);
+
+ if (fuse_page_is_writeback(inode, page->index)) {
+ if (wbc->sync_mode != WB_SYNC_ALL) {
+ redirty_page_for_writepage(wbc, page);
+ unlock_page(page);
+ return 0;
+ }
+ fuse_wait_on_page_writeback(inode, page->index);
+ }
+
+ if (req->num_pages &&
+ (req->num_pages == FUSE_MAX_PAGES_PER_REQ ||
+ (req->num_pages + 1) * PAGE_CACHE_SIZE > fc->max_write ||
+ req->pages[req->num_pages - 1]->index + 1 != page->index)) {
+ int err;
+
+ err = fuse_send_writepages(data);
+ if (err) {
+ unlock_page(page);
+ return err;
+ }
+
+ data->req = req =
+ fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
+ if (!req) {
+ unlock_page(page);
+ return -ENOMEM;
+ }
+ }
+
+ req->pages[req->num_pages] = page;
+ req->num_pages++;
+
+ if (test_set_page_writeback(page))
+ BUG();
+
+ unlock_page(page);
+
+ return 0;
+}
+
+static int fuse_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
+{
+ struct inode *inode = mapping->host;
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_fill_data data;
+ int err;
+
+ if (!fc->writeback_cache)
+ return generic_writepages(mapping, wbc);
+
+ err = -EIO;
+ if (is_bad_inode(inode))
+ goto out;
+
+ data.ff = NULL;
+ data.inode = inode;
+ data.req = fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
+ err = -ENOMEM;
+ if (!data.req)
+ goto out_put;
+
+ err = write_cache_pages(mapping, wbc, fuse_writepages_fill, &data);
+ if (data.req) {
+ if (!err && data.req->num_pages) {
+ err = fuse_send_writepages(&data);
+ if (err)
+ fuse_put_request(fc, data.req);
+ } else
+ fuse_put_request(fc, data.req);
+ }
+out_put:
+ if (data.ff)
+ fuse_file_put(data.ff, false);
+out:
+ return err;
+}
+
+/*
+ * Determine the number of bytes of data the page contains
+ */
+static inline unsigned fuse_page_length(struct page *page)
+{
+ loff_t i_size = i_size_read(page_file_mapping(page)->host);
+
+ if (i_size > 0) {
+ pgoff_t page_index = page_file_index(page);
+ pgoff_t end_index = (i_size - 1) >> PAGE_CACHE_SHIFT;
+ if (page_index < end_index)
+ return PAGE_CACHE_SIZE;
+ if (page_index == end_index)
+ return ((i_size - 1) & ~PAGE_CACHE_MASK) + 1;
+ }
+ return 0;
+}
+
+static int fuse_prepare_write(struct fuse_conn *fc, struct file *file,
+ struct page *page, loff_t pos, unsigned len)
+{
+ struct fuse_req *req;
+ unsigned num_read;
+ unsigned page_len;
+ int err;
+
+ if (PageUptodate(page) || (len == PAGE_CACHE_SIZE))
+ return 0;
+
+ page_len = fuse_page_length(page);
+ if (!page_len) {
+ zero_user(page, 0, PAGE_CACHE_SIZE);
+ return 0;
+ }
+
+ /*
+ * Page writeback can extend beyond the lifetime of the
+ * page-cache page, so make sure we read a properly synced
+ * page.
+ */
+ fuse_wait_on_page_writeback(page->mapping->host, page->index);
+
+ req = fuse_get_req(fc, 1);
+ err = PTR_ERR(req);
+ if (IS_ERR(req))
+ goto out;
+
+ req->out.page_zeroing = 1;
+ req->out.argpages = 1;
+ req->num_pages = 1;
+ req->pages[0] = page;
+ req->page_descs[0].offset = 0;
+ req->page_descs[0].length = PAGE_SIZE;
+ num_read = fuse_send_read(req, file, page_offset(page), page_len, NULL);
+ err = req->out.h.error;
+ fuse_put_request(fc, req);
+out:
+ if (err) {
+ unlock_page(page);
+ page_cache_release(page);
+ } else if (num_read != PAGE_CACHE_SIZE) {
+ zero_user_segment(page, num_read, PAGE_CACHE_SIZE);
+ }
+ return err;
+}
+
+static int fuse_write_begin(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned flags,
+ struct page **pagep, void **fsdata)
+{
+ pgoff_t index = pos >> PAGE_CACHE_SHIFT;
+ struct fuse_conn *fc = get_fuse_conn(file->f_dentry->d_inode);
+
+ BUG_ON(!fc->writeback_cache);
+
+ *pagep = grab_cache_page_write_begin(mapping, index, flags);
+ if (!*pagep)
+ return -ENOMEM;
+
+ return fuse_prepare_write(fc, file, *pagep, pos, len);
+}
+
+static int fuse_commit_write(struct file *file, struct page *page,
+ unsigned from, unsigned to)
+{
+ struct inode *inode = page->mapping->host;
+ loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
+
+ if (!PageUptodate(page))
+ SetPageUptodate(page);
+
+ fuse_write_update_size(inode, pos);
+ set_page_dirty(page);
+ return 0;
+}
+
+static int fuse_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
+{
+ unsigned from = pos & (PAGE_CACHE_SIZE - 1);
+
+ fuse_commit_write(file, page, from, from+copied);
+
+ unlock_page(page);
+ page_cache_release(page);
+
+ return copied;
+}
+
static int fuse_launder_page(struct page *page)
{
int err = 0;
@@ -2436,11 +2713,14 @@ static const struct file_operations fuse_direct_io_file_operations = {
static const struct address_space_operations fuse_file_aops = {
.readpage = fuse_readpage,
.writepage = fuse_writepage,
+ .writepages = fuse_writepages,
.launder_page = fuse_launder_page,
.readpages = fuse_readpages,
.set_page_dirty = __set_page_dirty_nobuffers,
.bmap = fuse_bmap,
.direct_IO = fuse_direct_IO,
+ .write_begin = fuse_write_begin,
+ .write_end = fuse_write_end,
};
void fuse_init_file_inode(struct inode *inode)
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:26:04
|
fuse_writepage_locked() should never submit new i/o for given page->index
if there is another one 'in progress' already. In most cases it's safe to
wait on page writeback. But if it was called due to memory shortage
(WB_SYNC_NONE), we should redirty page rather than blocking caller.
Signed-off-by: Maxim Patlasov <MPa...@pa...>
---
fs/fuse/file.c | 18 +++++++++++++++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3b4dc98..52c7d81 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1472,7 +1472,8 @@ static struct fuse_file *fuse_write_file(struct fuse_conn *fc,
return ff;
}
-static int fuse_writepage_locked(struct page *page)
+static int fuse_writepage_locked(struct page *page,
+ struct writeback_control *wbc)
{
struct address_space *mapping = page->mapping;
struct inode *inode = mapping->host;
@@ -1481,6 +1482,14 @@ static int fuse_writepage_locked(struct page *page)
struct fuse_req *req;
struct page *tmp_page;
+ if (fuse_page_is_writeback(inode, page->index)) {
+ if (wbc->sync_mode != WB_SYNC_ALL) {
+ redirty_page_for_writepage(wbc, page);
+ return 0;
+ }
+ fuse_wait_on_page_writeback(inode, page->index);
+ }
+
set_page_writeback(page);
req = fuse_request_alloc_nofs(1);
@@ -1527,7 +1536,7 @@ static int fuse_writepage(struct page *page, struct writeback_control *wbc)
{
int err;
- err = fuse_writepage_locked(page);
+ err = fuse_writepage_locked(page, wbc);
unlock_page(page);
return err;
@@ -1812,7 +1821,10 @@ static int fuse_launder_page(struct page *page)
int err = 0;
if (clear_page_dirty_for_io(page)) {
struct inode *inode = page->mapping->host;
- err = fuse_writepage_locked(page);
+ struct writeback_control wbc = {
+ .sync_mode = WB_SYNC_ALL,
+ };
+ err = fuse_writepage_locked(page, &wbc);
if (!err)
fuse_wait_on_page_writeback(inode, page->index);
}
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:26:25
|
The aim of .flush fop is to hint file-system that flushing its state or caches or any other important data to reliable storage would be desirable now. fuse_flush() passes this hint by sending FUSE_FLUSH request to userspace. However, dirty pages and pages under writeback may be not visible to userspace yet if we won't ensure it explicitly. Signed-off-by: Maxim Patlasov <MPa...@pa...> --- fs/fuse/file.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 52c7d81..3767824 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -18,6 +18,7 @@ #include <linux/falloc.h> static const struct file_operations fuse_direct_io_file_operations; +static void fuse_sync_writes(struct inode *inode); static int fuse_send_open(struct fuse_conn *fc, u64 nodeid, struct file *file, int opcode, struct fuse_open_out *outargp) @@ -414,6 +415,14 @@ static int fuse_flush(struct file *file, fl_owner_t id) if (fc->no_flush) return 0; + err = filemap_write_and_wait(file->f_mapping); + if (err) + return err; + + mutex_lock(&inode->i_mutex); + fuse_sync_writes(inode); + mutex_unlock(&inode->i_mutex); + req = fuse_get_req_nofail_nopages(fc, file); memset(&inarg, 0, sizeof(inarg)); inarg.fh = ff->fh; |
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:27:23
|
The problem is:
1. write cached data to a file
2. read directly from the same file (via another fd)
The 2nd operation may read stale data, i.e. the one that was in a file
before the 1st op. Problem is in how fuse manages writeback.
When direct op occurs the core kernel code calls filemap_write_and_wait
to flush all the cached ops in flight. But fuse acks the writeback right
after the ->writepages callback exits w/o waiting for the real write to
happen. Thus the subsequent direct op proceeds while the real writeback
is still in flight. This is a problem for backends that reorder operation.
Fix this by making the fuse direct IO callback explicitly wait on the
in-flight writeback to finish.
Changed in v2:
- do not wait on writeback if fuse_direct_io() call came from
CUSE (because it doesn't use fuse inodes)
Original patch by: Pavel Emelyanov <xe...@op...>
Signed-off-by: Maxim Patlasov <MPa...@pa...>
---
fs/fuse/cuse.c | 5 +++--
fs/fuse/file.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
fs/fuse/fuse_i.h | 13 ++++++++++++-
3 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index 6f96a8d..fb63185 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -93,7 +93,7 @@ static ssize_t cuse_read(struct file *file, char __user *buf, size_t count,
loff_t pos = 0;
struct iovec iov = { .iov_base = buf, .iov_len = count };
- return fuse_direct_io(file, &iov, 1, count, &pos, 0);
+ return fuse_direct_io(file, &iov, 1, count, &pos, FUSE_DIO_CUSE);
}
static ssize_t cuse_write(struct file *file, const char __user *buf,
@@ -106,7 +106,8 @@ static ssize_t cuse_write(struct file *file, const char __user *buf,
* No locking or generic_write_checks(), the server is
* responsible for locking and sanity checks.
*/
- return fuse_direct_io(file, &iov, 1, count, &pos, 1);
+ return fuse_direct_io(file, &iov, 1, count, &pos,
+ FUSE_DIO_WRITE | FUSE_DIO_CUSE);
}
static int cuse_open(struct inode *inode, struct file *file)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3767824..e6e064c 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -349,6 +349,31 @@ u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id)
return (u64) v0 + ((u64) v1 << 32);
}
+static bool fuse_range_is_writeback(struct inode *inode, pgoff_t idx_from,
+ pgoff_t idx_to)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ struct fuse_req *req;
+ bool found = false;
+
+ spin_lock(&fc->lock);
+ list_for_each_entry(req, &fi->writepages, writepages_entry) {
+ pgoff_t curr_index;
+
+ BUG_ON(req->inode != inode);
+ curr_index = req->misc.write.in.offset >> PAGE_CACHE_SHIFT;
+ if (!(idx_from >= curr_index + req->num_pages ||
+ idx_to < curr_index)) {
+ found = true;
+ break;
+ }
+ }
+ spin_unlock(&fc->lock);
+
+ return found;
+}
+
/*
* Check if page is under writeback
*
@@ -393,6 +418,19 @@ static int fuse_wait_on_page_writeback(struct inode *inode, pgoff_t index)
return 0;
}
+static void fuse_wait_on_writeback(struct inode *inode, pgoff_t start,
+ size_t bytes)
+{
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ pgoff_t idx_from, idx_to;
+
+ idx_from = start >> PAGE_CACHE_SHIFT;
+ idx_to = (start + bytes - 1) >> PAGE_CACHE_SHIFT;
+
+ wait_event(fi->page_waitq,
+ !fuse_range_is_writeback(inode, idx_from, idx_to));
+}
+
static int fuse_flush(struct file *file, fl_owner_t id)
{
struct inode *inode = file->f_path.dentry->d_inode;
@@ -1245,8 +1283,10 @@ static inline int fuse_iter_npages(const struct iov_iter *ii_p)
ssize_t fuse_direct_io(struct file *file, const struct iovec *iov,
unsigned long nr_segs, size_t count, loff_t *ppos,
- int write)
+ int flags)
{
+ int write = flags & FUSE_DIO_WRITE;
+ int cuse = flags & FUSE_DIO_CUSE;
struct fuse_file *ff = file->private_data;
struct fuse_conn *fc = ff->fc;
size_t nmax = write ? fc->max_write : fc->max_read;
@@ -1271,6 +1311,10 @@ ssize_t fuse_direct_io(struct file *file, const struct iovec *iov,
break;
}
+ if (!cuse)
+ fuse_wait_on_writeback(file->f_mapping->host, pos,
+ nbytes);
+
if (write)
nres = fuse_send_write(req, file, pos, nbytes, owner);
else
@@ -1339,7 +1383,8 @@ static ssize_t __fuse_direct_write(struct file *file, const struct iovec *iov,
res = generic_write_checks(file, ppos, &count, 0);
if (!res) {
- res = fuse_direct_io(file, iov, nr_segs, count, ppos, 1);
+ res = fuse_direct_io(file, iov, nr_segs, count, ppos,
+ FUSE_DIO_WRITE);
if (res > 0) {
struct fuse_inode *fi = get_fuse_inode(inode);
fuse_write_update_size(inode, *ppos);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index fdeccc5..86caa8c 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -823,9 +823,20 @@ int fuse_reverse_inval_entry(struct super_block *sb, u64 parent_nodeid,
int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
bool isdir);
+
+/**
+ * fuse_direct_io() flags
+ */
+
+/** If set, it is WRITE; otherwise - READ */
+#define FUSE_DIO_WRITE (1 << 0)
+
+/** CUSE pass fuse_direct_io() a file which f_mapping->host is not from FUSE */
+#define FUSE_DIO_CUSE (1 << 1)
+
ssize_t fuse_direct_io(struct file *file, const struct iovec *iov,
unsigned long nr_segs, size_t count, loff_t *ppos,
- int write);
+ int flags);
long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
unsigned int flags);
long fuse_ioctl_common(struct file *file, unsigned int cmd,
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:28:03
|
Introduce a bit kernel and userspace exchange between each-other on
the init stage and turn writeback on if the userspace want this and
mount option 'allow_wbcache' is present (controlled by fusermount).
Also add each writable file into per-inode write list and call the
generic_file_aio_write to make use of the Linux page cache engine.
Original patch by: Pavel Emelyanov <xe...@op...>
Signed-off-by: Maxim Patlasov <MPa...@pa...>
---
fs/fuse/file.c | 15 +++++++++++++++
fs/fuse/fuse_i.h | 4 ++++
fs/fuse/inode.c | 13 +++++++++++++
include/uapi/linux/fuse.h | 2 ++
4 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index e6e064c..147e618 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -211,6 +211,8 @@ void fuse_finish_open(struct inode *inode, struct file *file)
spin_unlock(&fc->lock);
fuse_invalidate_attr(inode);
}
+ if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
+ fuse_link_write_file(file);
}
int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
@@ -1096,6 +1098,19 @@ static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
struct iov_iter i;
loff_t endbyte = 0;
+ if (get_fuse_conn(inode)->writeback_cache) {
+ if (!(file->f_flags & O_DIRECT)) {
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ struct fuse_inode *fi = get_fuse_inode(inode);
+
+ spin_lock(&fc->lock);
+ inode->i_mtime = current_fs_time(inode->i_sb);
+ set_bit(FUSE_I_MTIME_UPDATED, &fi->state);
+ spin_unlock(&fc->lock);
+ }
+ return generic_file_aio_write(iocb, iov, nr_segs, pos);
+ }
+
WARN_ON(iocb->ki_pos != pos);
ocount = 0;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 86caa8c..a207744 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -44,6 +44,10 @@
doing the mount will be allowed to access the filesystem */
#define FUSE_ALLOW_OTHER (1 << 1)
+/** If the FUSE_ALLOW_WBCACHE flag is given, the filesystem
+ module will enable support of writback cache */
+#define FUSE_ALLOW_WBCACHE (1 << 2)
+
/** Number of page pointers embedded in fuse_req */
#define FUSE_REQ_INLINE_PAGES 1
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 3687daf..541fc4f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -521,6 +521,7 @@ enum {
OPT_ALLOW_OTHER,
OPT_MAX_READ,
OPT_BLKSIZE,
+ OPT_ALLOW_WBCACHE,
OPT_ERR
};
@@ -533,6 +534,7 @@ static const match_table_t tokens = {
{OPT_ALLOW_OTHER, "allow_other"},
{OPT_MAX_READ, "max_read=%u"},
{OPT_BLKSIZE, "blksize=%u"},
+ {OPT_ALLOW_WBCACHE, "allow_wbcache"},
{OPT_ERR, NULL}
};
@@ -606,6 +608,10 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
d->blksize = value;
break;
+ case OPT_ALLOW_WBCACHE:
+ d->flags |= FUSE_ALLOW_WBCACHE;
+ break;
+
default:
return 0;
}
@@ -633,6 +639,8 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root)
seq_printf(m, ",max_read=%u", fc->max_read);
if (sb->s_bdev && sb->s_blocksize != FUSE_DEFAULT_BLKSIZE)
seq_printf(m, ",blksize=%lu", sb->s_blocksize);
+ if (fc->flags & FUSE_ALLOW_WBCACHE)
+ seq_puts(m, ",allow_wbcache");
return 0;
}
@@ -944,6 +952,9 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
fc->auto_inval_data = 1;
if (arg->flags & FUSE_DO_READDIRPLUS)
fc->do_readdirplus = 1;
+ if (arg->flags & FUSE_WRITEBACK_CACHE &&
+ fc->flags & FUSE_ALLOW_WBCACHE)
+ fc->writeback_cache = 1;
} else {
ra_pages = fc->max_read / PAGE_CACHE_SIZE;
fc->no_lock = 1;
@@ -972,6 +983,8 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
FUSE_SPLICE_WRITE | FUSE_SPLICE_MOVE | FUSE_SPLICE_READ |
FUSE_FLOCK_LOCKS | FUSE_IOCTL_DIR | FUSE_AUTO_INVAL_DATA |
FUSE_DO_READDIRPLUS;
+ if (fc->flags & FUSE_ALLOW_WBCACHE)
+ arg->flags |= FUSE_WRITEBACK_CACHE;
req->in.h.opcode = FUSE_INIT;
req->in.numargs = 1;
req->in.args[0].size = sizeof(*arg);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 5dc1fea..20a9553 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -179,6 +179,7 @@ struct fuse_file_lock {
* FUSE_FLOCK_LOCKS: remote locking for BSD style file locks
* FUSE_HAS_IOCTL_DIR: kernel supports ioctl on directories
* FUSE_AUTO_INVAL_DATA: automatically invalidate cached pages
+ * FUSE_WRITEBACK_CACHE: use writeback cache for buffered writes
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -194,6 +195,7 @@ struct fuse_file_lock {
#define FUSE_HAS_IOCTL_DIR (1 << 11)
#define FUSE_AUTO_INVAL_DATA (1 << 12)
#define FUSE_DO_READDIRPLUS (1 << 13)
+#define FUSE_WRITEBACK_CACHE (1 << 14)
/**
* CUSE INIT request/reply flags
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-01-25 18:28:18
|
Make balance_dirty_pages start the throttling when the WRITEBACK_TEMP counter is high enough. This prevents us from having too many dirty pages on fuse, thus giving the userspace part of it a chance to write stuff properly. Note, that the existing balance logic is per-bdi, i.e. if the fuse user task gets stuck in the function this means, that it either writes to the mountpoint it serves (but it can deadlock even without the writeback) or it is writing to some _other_ dirty bdi and in the latter case someone else will free the memory for it. Signed-off-by: Maxim Patlasov <MPa...@pa...> Signed-off-by: Pavel Emelyanov <xe...@op...> --- mm/page-writeback.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 0713bfb..c47bcd4 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -1235,7 +1235,8 @@ static void balance_dirty_pages(struct address_space *mapping, */ nr_reclaimable = global_page_state(NR_FILE_DIRTY) + global_page_state(NR_UNSTABLE_NFS); - nr_dirty = nr_reclaimable + global_page_state(NR_WRITEBACK); + nr_dirty = nr_reclaimable + global_page_state(NR_WRITEBACK) + + global_page_state(NR_WRITEBACK_TEMP); global_dirty_limits(&background_thresh, &dirty_thresh); |
|
From: Miklos S. <mi...@sz...> - 2013-01-29 10:18:12
|
On Fri, Jan 25, 2013 at 7:22 PM, Maxim V. Patlasov
<MPa...@pa...> wrote:
> Make fuse think that when writeback is on the inode's i_size is always
> up-to-date and not update it with the value received from the userspace.
> This is done because the page cache code may update i_size without letting
> the FS know.
>
> This assumption implies fixing the previously introduced short-read helper --
> when a short read occurs the 'hole' is filled with zeroes.
>
> fuse_file_fallocate() is also fixed because now we should keep i_size up to
> date, so it must be updated if FUSE_FALLOCATE request succeeded.
>
> Changed in v2:
> - improved comment in fuse_short_read()
> - fixed fuse_file_fallocate() for KEEP_SIZE mode
>
> Original patch by: Pavel Emelyanov <xe...@op...>
> Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
> ---
> fs/fuse/dir.c | 9 ++++++---
> fs/fuse/file.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> fs/fuse/inode.c | 6 ++++--
> 3 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index ed8f8c5..ff8b603 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -827,7 +827,7 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
> stat->mtime.tv_nsec = attr->mtimensec;
> stat->ctime.tv_sec = attr->ctime;
> stat->ctime.tv_nsec = attr->ctimensec;
> - stat->size = attr->size;
> + stat->size = i_size_read(inode);
The old code is correct and you break it. We always use the values
returned by GETATTR, instead of the cached ones. The cached ones are
a best guess by the kernel and they may or may not have been correct
at any point in time. The attributes returned by userspace are the
authentic ones.
For the "write cache" case what we want, I think, is a mode where the
kernel always trusts the cached attributes. The attribute cache is
initialized from values returned in LOOKUP and the kernel never needs
to call GETATTR since the attributes are always up-to-date.
Is that correct?
> stat->blocks = attr->blocks;
>
> if (attr->blksize != 0)
> @@ -1541,6 +1541,7 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
> struct fuse_setattr_in inarg;
> struct fuse_attr_out outarg;
> bool is_truncate = false;
> + bool is_wb = fc->writeback_cache;
> loff_t oldsize;
> int err;
>
> @@ -1613,7 +1614,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
> fuse_change_attributes_common(inode, &outarg.attr,
> attr_timeout(&outarg));
> oldsize = inode->i_size;
> - i_size_write(inode, outarg.attr.size);
> + if (!is_wb || is_truncate || !S_ISREG(inode->i_mode))
> + i_size_write(inode, outarg.attr.size);
Okay, I managed to understand what is going on here: if userspace is
behaving badly and is changing the size even if that was not
requested, then we silently reject that. But that's neither clearly
unrestandable (without a comment) nor sensible, I think.
If the filesystem is behaving badly, just let it. Or is there some
other reason why we'd want this check?
>
> if (is_truncate) {
> /* NOTE: this may release/reacquire fc->lock */
> @@ -1625,7 +1627,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
> * Only call invalidate_inode_pages2() after removing
> * FUSE_NOWRITE, otherwise fuse_launder_page() would deadlock.
> */
> - if (S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
> + if ((is_truncate || !is_wb) &&
> + S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
> truncate_pagecache(inode, oldsize, outarg.attr.size);
> invalidate_inode_pages2(inode->i_mapping);
> }
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index b28be33..6b64e11 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/compat.h>
> #include <linux/swap.h>
> +#include <linux/falloc.h>
>
> static const struct file_operations fuse_direct_io_file_operations;
>
> @@ -543,9 +544,31 @@ static void fuse_short_read(struct fuse_req *req, struct inode *inode,
> u64 attr_ver)
> {
> size_t num_read = req->out.args[0].size;
> + struct fuse_conn *fc = get_fuse_conn(inode);
> +
> + if (fc->writeback_cache) {
> + /*
> + * A hole in a file. Some data after the hole are in page cache,
> + * but have not reached the client fs yet. So, the hole is not
> + * present there.
> + */
> + int i;
> + int start_idx = num_read >> PAGE_CACHE_SHIFT;
> + size_t off = num_read & (PAGE_CACHE_SIZE - 1);
>
> - loff_t pos = page_offset(req->pages[0]) + num_read;
> - fuse_read_update_size(inode, pos, attr_ver);
> + for (i = start_idx; i < req->num_pages; i++) {
> + struct page *page = req->pages[i];
> + void *mapaddr = kmap_atomic(page);
> +
> + memset(mapaddr + off, 0, PAGE_CACHE_SIZE - off);
> +
> + kunmap_atomic(mapaddr);
> + off = 0;
> + }
> + } else {
> + loff_t pos = page_offset(req->pages[0]) + num_read;
> + fuse_read_update_size(inode, pos, attr_ver);
> + }
> }
>
> static int fuse_readpage(struct file *file, struct page *page)
> @@ -2285,6 +2308,8 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
> .mode = mode
> };
> int err;
> + bool change_i_size = fc->writeback_cache &&
> + !(mode & FALLOC_FL_KEEP_SIZE);
>
> if (fc->no_fallocate)
> return -EOPNOTSUPP;
> @@ -2293,6 +2318,11 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
> if (IS_ERR(req))
> return PTR_ERR(req);
>
> + if (change_i_size) {
> + struct inode *inode = file->f_mapping->host;
> + mutex_lock(&inode->i_mutex);
> + }
> +
> req->in.h.opcode = FUSE_FALLOCATE;
> req->in.h.nodeid = ff->nodeid;
> req->in.numargs = 1;
> @@ -2306,6 +2336,15 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
> }
> fuse_put_request(fc, req);
>
> + if (change_i_size) {
> + struct inode *inode = file->f_mapping->host;
> +
> + if (!err)
> + fuse_write_update_size(inode, offset + length);
> +
> + mutex_unlock(&inode->i_mutex);
> + }
> +
> return err;
> }
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d95a5a..7e07dbd 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -196,6 +196,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
> {
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> + bool is_wb = fc->writeback_cache;
> loff_t oldsize;
> struct timespec old_mtime;
>
> @@ -209,10 +210,11 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
> fuse_change_attributes_common(inode, attr, attr_valid);
>
> oldsize = inode->i_size;
> - i_size_write(inode, attr->size);
> + if (!is_wb || !S_ISREG(inode->i_mode))
> + i_size_write(inode, attr->size);
Same as the previous comment. I think you simply need to omit these
checks. But if something *is* needed to special case the write cache
case, then it needs some comments to explain what and why.
> spin_unlock(&fc->lock);
>
> - if (S_ISREG(inode->i_mode)) {
> + if (!is_wb && S_ISREG(inode->i_mode)) {
> bool inval = false;
>
> if (oldsize != attr->size) {
>
|
|
From: Maxim V. P. <mpa...@pa...> - 2013-03-25 12:32:22
|
Hi Miklos,
Sorry for long delay, see please inline comments below.
01/29/2013 02:18 PM, Miklos Szeredi пишет:
> On Fri, Jan 25, 2013 at 7:22 PM, Maxim V. Patlasov
> <MPa...@pa...> wrote:
>> Make fuse think that when writeback is on the inode's i_size is always
>> up-to-date and not update it with the value received from the userspace.
>> This is done because the page cache code may update i_size without letting
>> the FS know.
>>
>> This assumption implies fixing the previously introduced short-read helper --
>> when a short read occurs the 'hole' is filled with zeroes.
>>
>> fuse_file_fallocate() is also fixed because now we should keep i_size up to
>> date, so it must be updated if FUSE_FALLOCATE request succeeded.
>>
>> Changed in v2:
>> - improved comment in fuse_short_read()
>> - fixed fuse_file_fallocate() for KEEP_SIZE mode
>>
>> Original patch by: Pavel Emelyanov <xe...@op...>
>> Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
>> ---
>> fs/fuse/dir.c | 9 ++++++---
>> fs/fuse/file.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>> fs/fuse/inode.c | 6 ++++--
>> 3 files changed, 51 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index ed8f8c5..ff8b603 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -827,7 +827,7 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
>> stat->mtime.tv_nsec = attr->mtimensec;
>> stat->ctime.tv_sec = attr->ctime;
>> stat->ctime.tv_nsec = attr->ctimensec;
>> - stat->size = attr->size;
>> + stat->size = i_size_read(inode);
> The old code is correct and you break it.
fuse_fillattr() is called strictly after fuse_change_attributes(). The
latter usually sets local i_size with server value: i_size_write(inode,
attr->size). This makes -/+ lines above equivalent. The only exception
is stale attributes when current fi->attr_version is greater than
attr_version acquired before sending FUSE_GETATTR request. My patch
breaks old behaviour in this special case, I'll fix it, thanks for the
catch.
> We always use the values
> returned by GETATTR, instead of the cached ones. The cached ones are
> a best guess by the kernel and they may or may not have been correct
> at any point in time. The attributes returned by userspace are the
> authentic ones.
Yes, that's correct when "write cache" is off.
> For the "write cache" case what we want, I think, is a mode where the
> kernel always trusts the cached attributes. The attribute cache is
> initialized from values returned in LOOKUP and the kernel never needs
> to call GETATTR since the attributes are always up-to-date.
>
> Is that correct?
No, for "write cache" case the kernel always trusts cached i_size for
regular files. For other attributes (and for !S_ISGREG() files) the
kernel relies on userspace.
>
>> stat->blocks = attr->blocks;
>>
>> if (attr->blksize != 0)
>> @@ -1541,6 +1541,7 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
>> struct fuse_setattr_in inarg;
>> struct fuse_attr_out outarg;
>> bool is_truncate = false;
>> + bool is_wb = fc->writeback_cache;
>> loff_t oldsize;
>> int err;
>>
>> @@ -1613,7 +1614,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
>> fuse_change_attributes_common(inode, &outarg.attr,
>> attr_timeout(&outarg));
>> oldsize = inode->i_size;
>> - i_size_write(inode, outarg.attr.size);
>> + if (!is_wb || is_truncate || !S_ISREG(inode->i_mode))
>> + i_size_write(inode, outarg.attr.size);
> Okay, I managed to understand what is going on here: if userspace is
> behaving badly and is changing the size even if that was not
> requested, then we silently reject that. But that's neither clearly
> unrestandable (without a comment) nor sensible, I think.
>
> If the filesystem is behaving badly, just let it. Or is there some
> other reason why we'd want this check?
The change above has nothing to do with misbehaving userspace. The check
literally means: do not trust attr.size from server if "write cache" is
on and the file is regular and there was no explicit user request to
change size (i.e. ATTR_SIZE bit was not set in attr->ia_valid). The
check is necessary if the user changes some attribute (not related to
i_size) and at the time of processing FUSE_SETATTR server doesn't have
up-to-date info about the size of file. In "write cache" case this
situation is typical for cached writes extending file.
>
>> if (is_truncate) {
>> /* NOTE: this may release/reacquire fc->lock */
>> @@ -1625,7 +1627,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
>> * Only call invalidate_inode_pages2() after removing
>> * FUSE_NOWRITE, otherwise fuse_launder_page() would deadlock.
>> */
>> - if (S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
>> + if ((is_truncate || !is_wb) &&
>> + S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
>> truncate_pagecache(inode, oldsize, outarg.attr.size);
>> invalidate_inode_pages2(inode->i_mapping);
>> }
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index b28be33..6b64e11 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -15,6 +15,7 @@
>> #include <linux/module.h>
>> #include <linux/compat.h>
>> #include <linux/swap.h>
>> +#include <linux/falloc.h>
>>
>> static const struct file_operations fuse_direct_io_file_operations;
>>
>> @@ -543,9 +544,31 @@ static void fuse_short_read(struct fuse_req *req, struct inode *inode,
>> u64 attr_ver)
>> {
>> size_t num_read = req->out.args[0].size;
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> +
>> + if (fc->writeback_cache) {
>> + /*
>> + * A hole in a file. Some data after the hole are in page cache,
>> + * but have not reached the client fs yet. So, the hole is not
>> + * present there.
>> + */
>> + int i;
>> + int start_idx = num_read >> PAGE_CACHE_SHIFT;
>> + size_t off = num_read & (PAGE_CACHE_SIZE - 1);
>>
>> - loff_t pos = page_offset(req->pages[0]) + num_read;
>> - fuse_read_update_size(inode, pos, attr_ver);
>> + for (i = start_idx; i < req->num_pages; i++) {
>> + struct page *page = req->pages[i];
>> + void *mapaddr = kmap_atomic(page);
>> +
>> + memset(mapaddr + off, 0, PAGE_CACHE_SIZE - off);
>> +
>> + kunmap_atomic(mapaddr);
>> + off = 0;
>> + }
>> + } else {
>> + loff_t pos = page_offset(req->pages[0]) + num_read;
>> + fuse_read_update_size(inode, pos, attr_ver);
>> + }
>> }
>>
>> static int fuse_readpage(struct file *file, struct page *page)
>> @@ -2285,6 +2308,8 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
>> .mode = mode
>> };
>> int err;
>> + bool change_i_size = fc->writeback_cache &&
>> + !(mode & FALLOC_FL_KEEP_SIZE);
>>
>> if (fc->no_fallocate)
>> return -EOPNOTSUPP;
>> @@ -2293,6 +2318,11 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
>> if (IS_ERR(req))
>> return PTR_ERR(req);
>>
>> + if (change_i_size) {
>> + struct inode *inode = file->f_mapping->host;
>> + mutex_lock(&inode->i_mutex);
>> + }
>> +
>> req->in.h.opcode = FUSE_FALLOCATE;
>> req->in.h.nodeid = ff->nodeid;
>> req->in.numargs = 1;
>> @@ -2306,6 +2336,15 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
>> }
>> fuse_put_request(fc, req);
>>
>> + if (change_i_size) {
>> + struct inode *inode = file->f_mapping->host;
>> +
>> + if (!err)
>> + fuse_write_update_size(inode, offset + length);
>> +
>> + mutex_unlock(&inode->i_mutex);
>> + }
>> +
>> return err;
>> }
>>
>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>> index 9d95a5a..7e07dbd 100644
>> --- a/fs/fuse/inode.c
>> +++ b/fs/fuse/inode.c
>> @@ -196,6 +196,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
>> {
>> struct fuse_conn *fc = get_fuse_conn(inode);
>> struct fuse_inode *fi = get_fuse_inode(inode);
>> + bool is_wb = fc->writeback_cache;
>> loff_t oldsize;
>> struct timespec old_mtime;
>>
>> @@ -209,10 +210,11 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
>> fuse_change_attributes_common(inode, attr, attr_valid);
>>
>> oldsize = inode->i_size;
>> - i_size_write(inode, attr->size);
>> + if (!is_wb || !S_ISREG(inode->i_mode))
>> + i_size_write(inode, attr->size);
> Same as the previous comment. I think you simply need to omit these
> checks. But if something *is* needed to special case the write cache
> case, then it needs some comments to explain what and why.
Are you OK about a comment like this:
/*
* In case of writeback_cache enabled, the cached writes beyond EOF
* extend local i_size without keeping userspace server in sync. So,
* attr->size coming from server can be stale. We cannot trust it.
*/
Thanks,
Maxim
>
>> spin_unlock(&fc->lock);
>>
>> - if (S_ISREG(inode->i_mode)) {
>> + if (!is_wb && S_ISREG(inode->i_mode)) {
>> bool inval = false;
>>
>> if (oldsize != attr->size) {
>>
|
|
From: Miklos S. <mi...@sz...> - 2013-01-29 22:19:48
|
On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
<MPa...@pa...> wrote:
> If writeback cache is on, buffered write doesn't result in immediate mtime
> update in userspace because the userspace will see modified data later, when
> writeback happens. Consequently, mtime provided by userspace may be older than
> actual time of buffered write.
>
> The problem can be solved by generating mtime locally (will come in next
> patches) and flushing it to userspace periodically. Here we introduce a flag to
> keep the state of fuse_inode: the flag is ON if and only if locally generated
> mtime (stored in inode->i_mtime) was not pushed to the userspace yet.
>
> The patch also implements all bits related to flushing and clearing the flag.
>
> Signed-off-by: Maxim Patlasov <MPa...@pa...>
> ---
> fs/fuse/dir.c | 42 +++++++++++++++++++++++++----
> fs/fuse/file.c | 31 ++++++++++++++++++---
> fs/fuse/fuse_i.h | 13 ++++++++-
> fs/fuse/inode.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 154 insertions(+), 11 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index ff8b603..969c60d 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -177,6 +177,13 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
> if (flags & LOOKUP_RCU)
> return -ECHILD;
>
> + if (test_bit(FUSE_I_MTIME_UPDATED,
> + &get_fuse_inode(inode)->state)) {
> + err = fuse_flush_mtime(inode, 0);
->d_revalidate may be called with or without i_mutex, there's
absolutely no way to know. So this won't work.
I know it was me who suggested this approach, but I have second
thoughts... I really don't like the way this mixes userspace and
kernel updates to mtime. I think it should be either one or the
other.
I don't think you need to much changes to this patch. Just clear
S_NOCMTIME, implement i_op->update_time(), which sets the
FUSE_I_MTIME_UPDATED flag and flush mtime just like you do now.
Except now it doesn't need to take i_mutex since all mtime updates are
now done by the kernel.
Does that make sense?
Thanks,
Miklos
|
|
From: Maxim V. P. <mpa...@pa...> - 2013-03-26 09:58:32
|
Hi Miklos,
Sorry for long delay, see please inline comment below.
01/30/2013 02:19 AM, Miklos Szeredi пишет:
> On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
> <MPa...@pa...> wrote:
>> If writeback cache is on, buffered write doesn't result in immediate mtime
>> update in userspace because the userspace will see modified data later, when
>> writeback happens. Consequently, mtime provided by userspace may be older than
>> actual time of buffered write.
>>
>> The problem can be solved by generating mtime locally (will come in next
>> patches) and flushing it to userspace periodically. Here we introduce a flag to
>> keep the state of fuse_inode: the flag is ON if and only if locally generated
>> mtime (stored in inode->i_mtime) was not pushed to the userspace yet.
>>
>> The patch also implements all bits related to flushing and clearing the flag.
>>
>> Signed-off-by: Maxim Patlasov <MPa...@pa...>
>> ---
>> fs/fuse/dir.c | 42 +++++++++++++++++++++++++----
>> fs/fuse/file.c | 31 ++++++++++++++++++---
>> fs/fuse/fuse_i.h | 13 ++++++++-
>> fs/fuse/inode.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 4 files changed, 154 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index ff8b603..969c60d 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -177,6 +177,13 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
>> if (flags & LOOKUP_RCU)
>> return -ECHILD;
>>
>> + if (test_bit(FUSE_I_MTIME_UPDATED,
>> + &get_fuse_inode(inode)->state)) {
>> + err = fuse_flush_mtime(inode, 0);
> ->d_revalidate may be called with or without i_mutex, there's
> absolutely no way to know. So this won't work.
>
> I know it was me who suggested this approach, but I have second
> thoughts... I really don't like the way this mixes userspace and
> kernel updates to mtime. I think it should be either one or the
> other.
>
> I don't think you need to much changes to this patch. Just clear
> S_NOCMTIME, implement i_op->update_time(), which sets the
> FUSE_I_MTIME_UPDATED flag and flush mtime just like you do now.
> Except now it doesn't need to take i_mutex since all mtime updates are
> now done by the kernel.
>
> Does that make sense?
Yes, but it's not as simple as you described above. mtime updates should
be strictly serialized, I used i_mutex for this purpose. Abandoning
i_mutex, we'll have to introduce another lock for synchronization.
Otherwise, we won't know when it's secure to clear FUSE_I_MTIME_UPDATED
flag. Another approach is to introduce one more state:
FUSE_I_MTIME_UPDATE_IN_PROGRESS. But again, we'll need something like
waitq to wait for mtime update completion.
I'd prefer much more simple solution: clear S_NOCMTIME and implement
i_op->update_time() as you suggested; but flush mtime only on last
close. May be we could extend FUSE_RELEASE request (struct
fuse_release_in) to accommodate mtime. Are you OK about it?
Thanks,
Maxim
>
> Thanks,
> Miklos
>
|
|
From: Miklos S. <mi...@sz...> - 2013-01-29 22:58:58
|
On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
<MPa...@pa...> wrote:
> Any write request requires a file handle to report to the userspace. Thus
> when we close a file (and free the fuse_file with this info) we have to
> flush all the outstanding writeback cache. Note, that simply calling the
> filemap_write_and_wait() is not enough since fuse finishes page writeback
> immediately and thus the -wait part of the mentioned call will be no-op.
> Do real wait on per-inode writepages list.
>
> Signed-off-by: Maxim Patlasov <MPa...@pa...>
> Signed-off-by: Pavel Emelyanov <xe...@op...>
> ---
> fs/fuse/file.c | 26 +++++++++++++++++++++++++-
> 1 files changed, 25 insertions(+), 1 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 4f8fa45..496e74c 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -138,6 +138,12 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> }
> }
>
> +static void __fuse_file_put(struct fuse_file *ff)
> +{
> + if (atomic_dec_and_test(&ff->count))
> + BUG();
> +}
> +
> int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
> bool isdir)
> {
> @@ -286,8 +292,23 @@ static int fuse_open(struct inode *inode, struct file *file)
> return fuse_open_common(inode, file, false);
> }
>
> +static void fuse_flush_writeback(struct inode *inode, struct file *file)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + struct fuse_inode *fi = get_fuse_inode(inode);
> +
> + filemap_write_and_wait(file->f_mapping);
> + wait_event(fi->page_waitq, list_empty_careful(&fi->writepages));
> + spin_unlock_wait(&fc->lock);
> +}
> +
> static int fuse_release(struct inode *inode, struct file *file)
> {
> + struct fuse_conn *fc = get_fuse_conn(inode);
> +
> + if (fc->writeback_cache)
> + fuse_flush_writeback(inode, file);
> +
> fuse_release_common(file, FUSE_RELEASE);
>
> /* return value is ignored by VFS */
> @@ -1343,7 +1364,8 @@ static void fuse_writepage_free(struct fuse_conn *fc, struct fuse_req *req)
>
> for (i = 0; i < req->num_pages; i++)
> __free_page(req->pages[i]);
> - fuse_file_put(req->ff, false);
> + if (!fc->writeback_cache)
> + fuse_file_put(req->ff, false);
> }
>
> static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
> @@ -1360,6 +1382,8 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
> }
> bdi_writeout_inc(bdi);
> wake_up(&fi->page_waitq);
> + if (fc->writeback_cache)
> + __fuse_file_put(req->ff);
> }
I don't see how this belongs in this patch. And I suspect this can be
done unconditionally (for the non-writeback-cache case as well), but
please move it into a separate patch.
Thanks,
Miklos
|
|
From: Maxim V. P. <mpa...@pa...> - 2013-03-26 11:23:50
|
Hi Miklos,
01/30/2013 02:58 AM, Miklos Szeredi пишет:
> On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
> <MPa...@pa...> wrote:
>> Any write request requires a file handle to report to the userspace. Thus
>> when we close a file (and free the fuse_file with this info) we have to
>> flush all the outstanding writeback cache. Note, that simply calling the
>> filemap_write_and_wait() is not enough since fuse finishes page writeback
>> immediately and thus the -wait part of the mentioned call will be no-op.
>> Do real wait on per-inode writepages list.
>>
>> Signed-off-by: Maxim Patlasov <MPa...@pa...>
>> Signed-off-by: Pavel Emelyanov <xe...@op...>
>> ---
>> fs/fuse/file.c | 26 +++++++++++++++++++++++++-
>> 1 files changed, 25 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index 4f8fa45..496e74c 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -138,6 +138,12 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
>> }
>> }
>>
>> +static void __fuse_file_put(struct fuse_file *ff)
>> +{
>> + if (atomic_dec_and_test(&ff->count))
>> + BUG();
>> +}
>> +
>> int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
>> bool isdir)
>> {
>> @@ -286,8 +292,23 @@ static int fuse_open(struct inode *inode, struct file *file)
>> return fuse_open_common(inode, file, false);
>> }
>>
>> +static void fuse_flush_writeback(struct inode *inode, struct file *file)
>> +{
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> + struct fuse_inode *fi = get_fuse_inode(inode);
>> +
>> + filemap_write_and_wait(file->f_mapping);
>> + wait_event(fi->page_waitq, list_empty_careful(&fi->writepages));
>> + spin_unlock_wait(&fc->lock);
>> +}
>> +
>> static int fuse_release(struct inode *inode, struct file *file)
>> {
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> +
>> + if (fc->writeback_cache)
>> + fuse_flush_writeback(inode, file);
>> +
>> fuse_release_common(file, FUSE_RELEASE);
>>
>> /* return value is ignored by VFS */
>> @@ -1343,7 +1364,8 @@ static void fuse_writepage_free(struct fuse_conn *fc, struct fuse_req *req)
>>
>> for (i = 0; i < req->num_pages; i++)
>> __free_page(req->pages[i]);
>> - fuse_file_put(req->ff, false);
>> + if (!fc->writeback_cache)
>> + fuse_file_put(req->ff, false);
>> }
>>
>> static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
>> @@ -1360,6 +1382,8 @@ static void fuse_writepage_finish(struct fuse_conn *fc, struct fuse_req *req)
>> }
>> bdi_writeout_inc(bdi);
>> wake_up(&fi->page_waitq);
>> + if (fc->writeback_cache)
>> + __fuse_file_put(req->ff);
>> }
> I don't see how this belongs in this patch. And I suspect this can be
> done unconditionally (for the non-writeback-cache case as well), but
> please move it into a separate patch.
OK, I'll do.
Thanks,
Maxim
>
> Thanks,
> Miklos
>
|
|
From: Miklos S. <mi...@sz...> - 2013-01-29 23:08:12
|
On Fri, Jan 25, 2013 at 7:25 PM, Maxim V. Patlasov
<MPa...@pa...> wrote:
> The .writepages one is required to make each writeback request carry more than
> one page on it.
>
> Changed in v2:
> - fixed fuse_prepare_write() to avoid reads beyond EOF
> - fixed fuse_prepare_write() to zero uninitialized part of page
>
> Original patch by: Pavel Emelyanov <xe...@op...>
> Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
> ---
> fs/fuse/file.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 281 insertions(+), 1 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 496e74c..3b4dc98 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -722,7 +722,10 @@ static void fuse_send_readpages(struct fuse_req *req, struct file *file)
>
> struct fuse_fill_data {
> struct fuse_req *req;
> - struct file *file;
> + union {
> + struct file *file;
> + struct fuse_file *ff;
> + };
> struct inode *inode;
> unsigned nr_pages;
> };
> @@ -1530,6 +1533,280 @@ static int fuse_writepage(struct page *page, struct writeback_control *wbc)
> return err;
> }
>
> +static int fuse_send_writepages(struct fuse_fill_data *data)
> +{
> + int i, all_ok = 1;
> + struct fuse_req *req = data->req;
> + struct inode *inode = data->inode;
> + struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + struct fuse_inode *fi = get_fuse_inode(inode);
> + loff_t off = -1;
> +
> + if (!data->ff)
> + data->ff = fuse_write_file(fc, fi);
> +
> + if (!data->ff) {
> + for (i = 0; i < req->num_pages; i++)
> + end_page_writeback(req->pages[i]);
> + return -EIO;
> + }
> +
> + req->inode = inode;
> + req->misc.write.in.offset = page_offset(req->pages[0]);
> +
> + spin_lock(&fc->lock);
> + list_add(&req->writepages_entry, &fi->writepages);
> + spin_unlock(&fc->lock);
> +
> + for (i = 0; i < req->num_pages; i++) {
> + struct page *page = req->pages[i];
> + struct page *tmp_page;
> +
> + tmp_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
> + if (tmp_page) {
> + copy_highpage(tmp_page, page);
> + inc_bdi_stat(bdi, BDI_WRITEBACK);
> + inc_zone_page_state(tmp_page, NR_WRITEBACK_TEMP);
> + } else
> + all_ok = 0;
> + req->pages[i] = tmp_page;
> + if (i == 0)
> + off = page_offset(page);
> +
> + end_page_writeback(page);
> + }
> +
> + if (!all_ok) {
> + for (i = 0; i < req->num_pages; i++) {
> + struct page *page = req->pages[i];
> + if (page) {
> + dec_bdi_stat(bdi, BDI_WRITEBACK);
> + dec_zone_page_state(page, NR_WRITEBACK_TEMP);
> + __free_page(page);
> + req->pages[i] = NULL;
> + }
> + }
> +
> + spin_lock(&fc->lock);
> + list_del(&req->writepages_entry);
> + wake_up(&fi->page_waitq);
> + spin_unlock(&fc->lock);
> + return -ENOMEM;
> + }
> +
> + req->ff = fuse_file_get(data->ff);
> + fuse_write_fill(req, data->ff, off, 0);
> +
> + req->misc.write.in.write_flags |= FUSE_WRITE_CACHE;
> + req->in.argpages = 1;
> + fuse_page_descs_length_init(req, 0, req->num_pages);
> + req->end = fuse_writepage_end;
> +
> + spin_lock(&fc->lock);
> + list_add_tail(&req->list, &fi->queued_writes);
> + fuse_flush_writepages(data->inode);
> + spin_unlock(&fc->lock);
> +
> + return 0;
> +}
> +
> +static int fuse_writepages_fill(struct page *page,
> + struct writeback_control *wbc, void *_data)
> +{
> + struct fuse_fill_data *data = _data;
> + struct fuse_req *req = data->req;
> + struct inode *inode = data->inode;
> + struct fuse_conn *fc = get_fuse_conn(inode);
> +
> + if (fuse_page_is_writeback(inode, page->index)) {
> + if (wbc->sync_mode != WB_SYNC_ALL) {
> + redirty_page_for_writepage(wbc, page);
> + unlock_page(page);
> + return 0;
> + }
> + fuse_wait_on_page_writeback(inode, page->index);
> + }
> +
> + if (req->num_pages &&
> + (req->num_pages == FUSE_MAX_PAGES_PER_REQ ||
> + (req->num_pages + 1) * PAGE_CACHE_SIZE > fc->max_write ||
> + req->pages[req->num_pages - 1]->index + 1 != page->index)) {
> + int err;
> +
> + err = fuse_send_writepages(data);
> + if (err) {
> + unlock_page(page);
> + return err;
> + }
> +
> + data->req = req =
> + fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
> + if (!req) {
> + unlock_page(page);
> + return -ENOMEM;
> + }
> + }
> +
> + req->pages[req->num_pages] = page;
> + req->num_pages++;
> +
> + if (test_set_page_writeback(page))
> + BUG();
> +
> + unlock_page(page);
> +
> + return 0;
> +}
> +
> +static int fuse_writepages(struct address_space *mapping,
> + struct writeback_control *wbc)
> +{
> + struct inode *inode = mapping->host;
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + struct fuse_fill_data data;
> + int err;
> +
> + if (!fc->writeback_cache)
> + return generic_writepages(mapping, wbc);
> +
> + err = -EIO;
> + if (is_bad_inode(inode))
> + goto out;
> +
> + data.ff = NULL;
> + data.inode = inode;
> + data.req = fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
> + err = -ENOMEM;
> + if (!data.req)
> + goto out_put;
> +
> + err = write_cache_pages(mapping, wbc, fuse_writepages_fill, &data);
> + if (data.req) {
> + if (!err && data.req->num_pages) {
> + err = fuse_send_writepages(&data);
> + if (err)
> + fuse_put_request(fc, data.req);
> + } else
> + fuse_put_request(fc, data.req);
> + }
> +out_put:
> + if (data.ff)
> + fuse_file_put(data.ff, false);
> +out:
> + return err;
> +}
> +
> +/*
> + * Determine the number of bytes of data the page contains
> + */
> +static inline unsigned fuse_page_length(struct page *page)
> +{
> + loff_t i_size = i_size_read(page_file_mapping(page)->host);
> +
> + if (i_size > 0) {
> + pgoff_t page_index = page_file_index(page);
> + pgoff_t end_index = (i_size - 1) >> PAGE_CACHE_SHIFT;
> + if (page_index < end_index)
> + return PAGE_CACHE_SIZE;
> + if (page_index == end_index)
> + return ((i_size - 1) & ~PAGE_CACHE_MASK) + 1;
> + }
> + return 0;
> +}
> +
> +static int fuse_prepare_write(struct fuse_conn *fc, struct file *file,
> + struct page *page, loff_t pos, unsigned len)
> +{
> + struct fuse_req *req;
> + unsigned num_read;
> + unsigned page_len;
> + int err;
> +
> + if (PageUptodate(page) || (len == PAGE_CACHE_SIZE))
> + return 0;
> +
> + page_len = fuse_page_length(page);
> + if (!page_len) {
> + zero_user(page, 0, PAGE_CACHE_SIZE);
> + return 0;
> + }
> +
> + /*
> + * Page writeback can extend beyond the lifetime of the
> + * page-cache page, so make sure we read a properly synced
> + * page.
> + */
> + fuse_wait_on_page_writeback(page->mapping->host, page->index);
> +
> + req = fuse_get_req(fc, 1);
> + err = PTR_ERR(req);
> + if (IS_ERR(req))
> + goto out;
> +
> + req->out.page_zeroing = 1;
> + req->out.argpages = 1;
> + req->num_pages = 1;
> + req->pages[0] = page;
> + req->page_descs[0].offset = 0;
> + req->page_descs[0].length = PAGE_SIZE;
> + num_read = fuse_send_read(req, file, page_offset(page), page_len, NULL);
> + err = req->out.h.error;
> + fuse_put_request(fc, req);
> +out:
> + if (err) {
> + unlock_page(page);
> + page_cache_release(page);
> + } else if (num_read != PAGE_CACHE_SIZE) {
> + zero_user_segment(page, num_read, PAGE_CACHE_SIZE);
> + }
> + return err;
> +}
Lots of duplication between this and fuse_readpage(). Can some of
that be sanely shared?
> +
> +static int fuse_write_begin(struct file *file, struct address_space *mapping,
> + loff_t pos, unsigned len, unsigned flags,
> + struct page **pagep, void **fsdata)
> +{
> + pgoff_t index = pos >> PAGE_CACHE_SHIFT;
> + struct fuse_conn *fc = get_fuse_conn(file->f_dentry->d_inode);
> +
> + BUG_ON(!fc->writeback_cache);
> +
> + *pagep = grab_cache_page_write_begin(mapping, index, flags);
> + if (!*pagep)
> + return -ENOMEM;
> +
> + return fuse_prepare_write(fc, file, *pagep, pos, len);
> +}
> +
One interesting aspect of this writeback-cache case is the handling of
"disk full" errors. The write-begin function should make sure that
space is reserved on disk for the write, but this patchset doesn't do
that. It's not a huge issue, but maybe worth thinking about (and a
comment above the fuse_write_begin() function).
> +static int fuse_commit_write(struct file *file, struct page *page,
> + unsigned from, unsigned to)
> +{
> + struct inode *inode = page->mapping->host;
> + loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
> +
> + if (!PageUptodate(page))
> + SetPageUptodate(page);
> +
> + fuse_write_update_size(inode, pos);
> + set_page_dirty(page);
> + return 0;
> +}
> +
> +static int fuse_write_end(struct file *file, struct address_space *mapping,
> + loff_t pos, unsigned len, unsigned copied,
> + struct page *page, void *fsdata)
> +{
> + unsigned from = pos & (PAGE_CACHE_SIZE - 1);
> +
> + fuse_commit_write(file, page, from, from+copied);
> +
> + unlock_page(page);
> + page_cache_release(page);
> +
> + return copied;
> +}
> +
> static int fuse_launder_page(struct page *page)
> {
> int err = 0;
> @@ -2436,11 +2713,14 @@ static const struct file_operations fuse_direct_io_file_operations = {
> static const struct address_space_operations fuse_file_aops = {
> .readpage = fuse_readpage,
> .writepage = fuse_writepage,
> + .writepages = fuse_writepages,
> .launder_page = fuse_launder_page,
> .readpages = fuse_readpages,
> .set_page_dirty = __set_page_dirty_nobuffers,
> .bmap = fuse_bmap,
> .direct_IO = fuse_direct_IO,
> + .write_begin = fuse_write_begin,
> + .write_end = fuse_write_end,
> };
>
> void fuse_init_file_inode(struct inode *inode)
>
|
|
From: Maxim V. P. <mpa...@pa...> - 2013-03-27 12:38:50
|
Hi Miklos,
01/30/2013 03:08 AM, Miklos Szeredi пишет:
> On Fri, Jan 25, 2013 at 7:25 PM, Maxim V. Patlasov
> <MPa...@pa...> wrote:
>> The .writepages one is required to make each writeback request carry more than
>> one page on it.
>>
>> Changed in v2:
>> - fixed fuse_prepare_write() to avoid reads beyond EOF
>> - fixed fuse_prepare_write() to zero uninitialized part of page
>>
>> Original patch by: Pavel Emelyanov <xe...@op...>
>> Signed-off-by: Maxim V. Patlasov <MPa...@pa...>
>> ---
>> fs/fuse/file.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 281 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index 496e74c..3b4dc98 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -722,7 +722,10 @@ static void fuse_send_readpages(struct fuse_req *req, struct file *file)
>>
>> struct fuse_fill_data {
>> struct fuse_req *req;
>> - struct file *file;
>> + union {
>> + struct file *file;
>> + struct fuse_file *ff;
>> + };
>> struct inode *inode;
>> unsigned nr_pages;
>> };
>> @@ -1530,6 +1533,280 @@ static int fuse_writepage(struct page *page, struct writeback_control *wbc)
>> return err;
>> }
>>
>> +static int fuse_send_writepages(struct fuse_fill_data *data)
>> +{
>> + int i, all_ok = 1;
>> + struct fuse_req *req = data->req;
>> + struct inode *inode = data->inode;
>> + struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> + struct fuse_inode *fi = get_fuse_inode(inode);
>> + loff_t off = -1;
>> +
>> + if (!data->ff)
>> + data->ff = fuse_write_file(fc, fi);
>> +
>> + if (!data->ff) {
>> + for (i = 0; i < req->num_pages; i++)
>> + end_page_writeback(req->pages[i]);
>> + return -EIO;
>> + }
>> +
>> + req->inode = inode;
>> + req->misc.write.in.offset = page_offset(req->pages[0]);
>> +
>> + spin_lock(&fc->lock);
>> + list_add(&req->writepages_entry, &fi->writepages);
>> + spin_unlock(&fc->lock);
>> +
>> + for (i = 0; i < req->num_pages; i++) {
>> + struct page *page = req->pages[i];
>> + struct page *tmp_page;
>> +
>> + tmp_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
>> + if (tmp_page) {
>> + copy_highpage(tmp_page, page);
>> + inc_bdi_stat(bdi, BDI_WRITEBACK);
>> + inc_zone_page_state(tmp_page, NR_WRITEBACK_TEMP);
>> + } else
>> + all_ok = 0;
>> + req->pages[i] = tmp_page;
>> + if (i == 0)
>> + off = page_offset(page);
>> +
>> + end_page_writeback(page);
>> + }
>> +
>> + if (!all_ok) {
>> + for (i = 0; i < req->num_pages; i++) {
>> + struct page *page = req->pages[i];
>> + if (page) {
>> + dec_bdi_stat(bdi, BDI_WRITEBACK);
>> + dec_zone_page_state(page, NR_WRITEBACK_TEMP);
>> + __free_page(page);
>> + req->pages[i] = NULL;
>> + }
>> + }
>> +
>> + spin_lock(&fc->lock);
>> + list_del(&req->writepages_entry);
>> + wake_up(&fi->page_waitq);
>> + spin_unlock(&fc->lock);
>> + return -ENOMEM;
>> + }
>> +
>> + req->ff = fuse_file_get(data->ff);
>> + fuse_write_fill(req, data->ff, off, 0);
>> +
>> + req->misc.write.in.write_flags |= FUSE_WRITE_CACHE;
>> + req->in.argpages = 1;
>> + fuse_page_descs_length_init(req, 0, req->num_pages);
>> + req->end = fuse_writepage_end;
>> +
>> + spin_lock(&fc->lock);
>> + list_add_tail(&req->list, &fi->queued_writes);
>> + fuse_flush_writepages(data->inode);
>> + spin_unlock(&fc->lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int fuse_writepages_fill(struct page *page,
>> + struct writeback_control *wbc, void *_data)
>> +{
>> + struct fuse_fill_data *data = _data;
>> + struct fuse_req *req = data->req;
>> + struct inode *inode = data->inode;
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> +
>> + if (fuse_page_is_writeback(inode, page->index)) {
>> + if (wbc->sync_mode != WB_SYNC_ALL) {
>> + redirty_page_for_writepage(wbc, page);
>> + unlock_page(page);
>> + return 0;
>> + }
>> + fuse_wait_on_page_writeback(inode, page->index);
>> + }
>> +
>> + if (req->num_pages &&
>> + (req->num_pages == FUSE_MAX_PAGES_PER_REQ ||
>> + (req->num_pages + 1) * PAGE_CACHE_SIZE > fc->max_write ||
>> + req->pages[req->num_pages - 1]->index + 1 != page->index)) {
>> + int err;
>> +
>> + err = fuse_send_writepages(data);
>> + if (err) {
>> + unlock_page(page);
>> + return err;
>> + }
>> +
>> + data->req = req =
>> + fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
>> + if (!req) {
>> + unlock_page(page);
>> + return -ENOMEM;
>> + }
>> + }
>> +
>> + req->pages[req->num_pages] = page;
>> + req->num_pages++;
>> +
>> + if (test_set_page_writeback(page))
>> + BUG();
>> +
>> + unlock_page(page);
>> +
>> + return 0;
>> +}
>> +
>> +static int fuse_writepages(struct address_space *mapping,
>> + struct writeback_control *wbc)
>> +{
>> + struct inode *inode = mapping->host;
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> + struct fuse_fill_data data;
>> + int err;
>> +
>> + if (!fc->writeback_cache)
>> + return generic_writepages(mapping, wbc);
>> +
>> + err = -EIO;
>> + if (is_bad_inode(inode))
>> + goto out;
>> +
>> + data.ff = NULL;
>> + data.inode = inode;
>> + data.req = fuse_request_alloc_nofs(FUSE_MAX_PAGES_PER_REQ);
>> + err = -ENOMEM;
>> + if (!data.req)
>> + goto out_put;
>> +
>> + err = write_cache_pages(mapping, wbc, fuse_writepages_fill, &data);
>> + if (data.req) {
>> + if (!err && data.req->num_pages) {
>> + err = fuse_send_writepages(&data);
>> + if (err)
>> + fuse_put_request(fc, data.req);
>> + } else
>> + fuse_put_request(fc, data.req);
>> + }
>> +out_put:
>> + if (data.ff)
>> + fuse_file_put(data.ff, false);
>> +out:
>> + return err;
>> +}
>> +
>> +/*
>> + * Determine the number of bytes of data the page contains
>> + */
>> +static inline unsigned fuse_page_length(struct page *page)
>> +{
>> + loff_t i_size = i_size_read(page_file_mapping(page)->host);
>> +
>> + if (i_size > 0) {
>> + pgoff_t page_index = page_file_index(page);
>> + pgoff_t end_index = (i_size - 1) >> PAGE_CACHE_SHIFT;
>> + if (page_index < end_index)
>> + return PAGE_CACHE_SIZE;
>> + if (page_index == end_index)
>> + return ((i_size - 1) & ~PAGE_CACHE_MASK) + 1;
>> + }
>> + return 0;
>> +}
>> +
>> +static int fuse_prepare_write(struct fuse_conn *fc, struct file *file,
>> + struct page *page, loff_t pos, unsigned len)
>> +{
>> + struct fuse_req *req;
>> + unsigned num_read;
>> + unsigned page_len;
>> + int err;
>> +
>> + if (PageUptodate(page) || (len == PAGE_CACHE_SIZE))
>> + return 0;
>> +
>> + page_len = fuse_page_length(page);
>> + if (!page_len) {
>> + zero_user(page, 0, PAGE_CACHE_SIZE);
>> + return 0;
>> + }
>> +
>> + /*
>> + * Page writeback can extend beyond the lifetime of the
>> + * page-cache page, so make sure we read a properly synced
>> + * page.
>> + */
>> + fuse_wait_on_page_writeback(page->mapping->host, page->index);
>> +
>> + req = fuse_get_req(fc, 1);
>> + err = PTR_ERR(req);
>> + if (IS_ERR(req))
>> + goto out;
>> +
>> + req->out.page_zeroing = 1;
>> + req->out.argpages = 1;
>> + req->num_pages = 1;
>> + req->pages[0] = page;
>> + req->page_descs[0].offset = 0;
>> + req->page_descs[0].length = PAGE_SIZE;
>> + num_read = fuse_send_read(req, file, page_offset(page), page_len, NULL);
>> + err = req->out.h.error;
>> + fuse_put_request(fc, req);
>> +out:
>> + if (err) {
>> + unlock_page(page);
>> + page_cache_release(page);
>> + } else if (num_read != PAGE_CACHE_SIZE) {
>> + zero_user_segment(page, num_read, PAGE_CACHE_SIZE);
>> + }
>> + return err;
>> +}
> Lots of duplication between this and fuse_readpage(). Can some of
> that be sanely shared?
Yes, I'll factor out the common part. Thanks for finding.
>
>> +
>> +static int fuse_write_begin(struct file *file, struct address_space *mapping,
>> + loff_t pos, unsigned len, unsigned flags,
>> + struct page **pagep, void **fsdata)
>> +{
>> + pgoff_t index = pos >> PAGE_CACHE_SHIFT;
>> + struct fuse_conn *fc = get_fuse_conn(file->f_dentry->d_inode);
>> +
>> + BUG_ON(!fc->writeback_cache);
>> +
>> + *pagep = grab_cache_page_write_begin(mapping, index, flags);
>> + if (!*pagep)
>> + return -ENOMEM;
>> +
>> + return fuse_prepare_write(fc, file, *pagep, pos, len);
>> +}
>> +
> One interesting aspect of this writeback-cache case is the handling of
> "disk full" errors. The write-begin function should make sure that
> space is reserved on disk for the write, but this patchset doesn't do
> that. It's not a huge issue, but maybe worth thinking about (and a
> comment above the fuse_write_begin() function).
Good point! I agree that write-begin may be improved in this direction.
I'll add a comment not to forget about it in the future.
Thanks,
Maxim
>
>> +static int fuse_commit_write(struct file *file, struct page *page,
>> + unsigned from, unsigned to)
>> +{
>> + struct inode *inode = page->mapping->host;
>> + loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
>> +
>> + if (!PageUptodate(page))
>> + SetPageUptodate(page);
>> +
>> + fuse_write_update_size(inode, pos);
>> + set_page_dirty(page);
>> + return 0;
>> +}
>> +
>> +static int fuse_write_end(struct file *file, struct address_space *mapping,
>> + loff_t pos, unsigned len, unsigned copied,
>> + struct page *page, void *fsdata)
>> +{
>> + unsigned from = pos & (PAGE_CACHE_SIZE - 1);
>> +
>> + fuse_commit_write(file, page, from, from+copied);
>> +
>> + unlock_page(page);
>> + page_cache_release(page);
>> +
>> + return copied;
>> +}
>> +
>> static int fuse_launder_page(struct page *page)
>> {
>> int err = 0;
>> @@ -2436,11 +2713,14 @@ static const struct file_operations fuse_direct_io_file_operations = {
>> static const struct address_space_operations fuse_file_aops = {
>> .readpage = fuse_readpage,
>> .writepage = fuse_writepage,
>> + .writepages = fuse_writepages,
>> .launder_page = fuse_launder_page,
>> .readpages = fuse_readpages,
>> .set_page_dirty = __set_page_dirty_nobuffers,
>> .bmap = fuse_bmap,
>> .direct_IO = fuse_direct_IO,
>> + .write_begin = fuse_write_begin,
>> + .write_end = fuse_write_end,
>> };
>>
>> void fuse_init_file_inode(struct inode *inode)
>>
|
|
From: Maxim V. P. <MPa...@pa...> - 2013-04-01 10:39:49
|
Hi,
This is the fourth iteration of Pavel Emelyanov's patch-set implementing
write-back policy for FUSE page cache. Initial patch-set description was
the following:
One of the problems with the existing FUSE implementation is that it uses the
write-through cache policy which results in performance problems on certain
workloads. E.g. when copying a big file into a FUSE file the cp pushes every
128k to the userspace synchronously. This becomes a problem when the userspace
back-end uses networking for storing the data.
A good solution of this is switching the FUSE page cache into a write-back policy.
With this file data are pushed to the userspace with big chunks (depending on the
dirty memory limits, but this is much more than 128k) which lets the FUSE daemons
handle the size updates in a more efficient manner.
The writeback feature is per-connection and is explicitly configurable at the
init stage (is it worth making it CAP_SOMETHING protected?) When the writeback is
turned ON:
* still copy writeback pages to temporary buffer when sending a writeback request
and finish the page writeback immediately
* make kernel maintain the inode's i_size to avoid frequent i_size synchronization
with the user space
* take NR_WRITEBACK_TEMP into account when makeing balance_dirty_pages decision.
This protects us from having too many dirty pages on FUSE
The provided patchset survives the fsx test. Performance measurements are not yet
all finished, but the mentioned copying of a huge file becomes noticeably faster
even on machines with few RAM and doesn't make the system stuck (the dirty pages
balancer does its work OK). Applies on top of v3.5-rc4.
We are currently exploring this with our own distributed storage implementation
which is heavily oriented on storing big blobs of data with extremely rare meta-data
updates (virtual machines' and containers' disk images). With the existing cache
policy a typical usage scenario -- copying a big VM disk into a cloud -- takes way
too much time to proceed, much longer than if it was simply scp-ed over the same
network. The write-back policy (as I mentioned) noticeably improves this scenario.
Kirill (in Cc) can share more details about the performance and the storage concepts
details if required.
Changed in v2:
- numerous bugfixes:
- fuse_write_begin and fuse_writepages_fill and fuse_writepage_locked must wait
on page writeback because page writeback can extend beyond the lifetime of
the page-cache page
- fuse_send_writepages can end_page_writeback on original page only after adding
request to fi->writepages list; otherwise another writeback may happen inside
the gap between end_page_writeback and adding to the list
- fuse_direct_io must wait on page writeback; otherwise data corruption is possible
due to reordering requests
- fuse_flush must flush dirty memory and wait for all writeback on given inode
before sending FUSE_FLUSH to userspace; otherwise FUSE_FLUSH is not reliable
- fuse_file_fallocate must hold i_mutex around FUSE_FALLOCATE and i_size update;
otherwise a race with a writer extending i_size is possible
- fix handling errors in fuse_writepages and fuse_send_writepages
- handle i_mtime intelligently if writeback cache is on (see patch #7 (update i_mtime
on buffered writes) for details.
- put enabling writeback cache under fusermount control; (see mount option
'allow_wbcache' introduced by patch #13 (turn writeback cache on))
- rebased on v3.7-rc5
Changed in v3:
- rebased on for-next branch of the fuse tree (fb05f41f5f96f7423c53da4d87913fb44fd0565d)
Changed in v4:
- rebased on for-next branch of the fuse tree (634734b63ac39e137a1c623ba74f3e062b6577db)
- fixed fuse_fillattr() for non-writeback_chace case
- added comments explaining why we cannot trust size from server
- rewrote patch handling i_mtime; it's titled Trust-kernel-i_mtime-only now
- simplified patch titled Flush-files-on-wb-close
- eliminated code duplications from fuse_readpage() ans fuse_prepare_write()
- added comment about "disk full" errors to fuse_write_begin()
Thanks,
Maxim
---
Maxim V. Patlasov (14):
fuse: Linking file to inode helper
fuse: Getting file for writeback helper
fuse: Prepare to handle short reads
fuse: Prepare to handle multiple pages in writeback
fuse: Connection bit for enabling writeback
fuse: Trust kernel i_size only - v3
fuse: Trust kernel i_mtime only
fuse: Flush files on wb close
fuse: Implement writepages and write_begin/write_end callbacks - v3
fuse: fuse_writepage_locked() should wait on writeback
fuse: fuse_flush() should wait on writeback
fuse: Fix O_DIRECT operations vs cached writeback misorder - v2
fuse: Turn writeback cache on
mm: Account for WRITEBACK_TEMP in balance_dirty_pages
fs/fuse/cuse.c | 5
fs/fuse/dir.c | 127 +++++++++-
fs/fuse/file.c | 575 ++++++++++++++++++++++++++++++++++++++++-----
fs/fuse/fuse_i.h | 26 ++
fs/fuse/inode.c | 37 +++
include/uapi/linux/fuse.h | 2
mm/page-writeback.c | 3
7 files changed, 689 insertions(+), 86 deletions(-)
--
Signature
|
Thanks for helping keep SourceForge clean.
X