linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: nickpiggin@yahoo.com.au, hch@infradead.org
Cc: dhowells@redhat.com, linux-fsdevel@vger.kernel.org,
	nfsv4@linux-nfs.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/4] CacheFiles: Use the ->write() file op rather than a special kernel aop
Date: Fri, 03 Apr 2009 10:41:38 +0100	[thread overview]
Message-ID: <20090403094138.9510.80681.stgit@warthog.procyon.org.uk> (raw)

Make CacheFiles use the ->write() file operation rather than a special kernel
address space operation optimised for writing whole pages at page boundaries.

This reverts the patch:

	CacheFiles: Add a hook to write a single page of data to an inode

	Add an address space operation to write one single page of data to an
	inode at a page-aligned location (thus permitting the implementation to
	be highly optimised).  The data source is a single page.

	This is used by CacheFiles to store the contents of netfs pages into
	their backing file pages.

	Supply a generic implementation for this that uses the write_begin()
	and write_end() address_space operations to bind a copy directly into
	the page cache.

	Hook the Ext2 and Ext3 operations to the generic implementation.

And then opens the target file for each write, calls ->write() and then
fput()'s the file.  The file has to be released, rather than being cached for
multiple uses, as caching it is liable to cause ENFILE errors to start cropping
up when a large tar job is run on a netfs.  Optimisation may be possible to
write several pages to the same file in one go, thus reducing the amount of
dentry_open()/fput() calls required.

Despite executing a fair bit more code, this appears to perform much better
than use of the write_one_page() address space op.  There are two possible
reasons for this:

 (1) ->write() is doing some scheduling stuff that write_one_page() is not.

 (2) fput() is doing some scheduling stuff, probably by way of the ->release()
     file op.

Whether the cache would still perform better under a real load with this patch
is hard to measure.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/cachefiles/namei.c |    3 +--
 fs/cachefiles/rdwr.c  |   42 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 35 insertions(+), 10 deletions(-)


diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 83b1988..4ce818a 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -446,8 +446,7 @@ lookup_again:
 
 			ret = -EPERM;
 			aops = object->dentry->d_inode->i_mapping->a_ops;
-			if (!aops->bmap ||
-			    !aops->write_one_page)
+			if (!aops->bmap)
 				goto check_error;
 
 			object->backer = object->dentry;
diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
index 84bb8b7..a69787e 100644
--- a/fs/cachefiles/rdwr.c
+++ b/fs/cachefiles/rdwr.c
@@ -9,6 +9,8 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#include <linux/mount.h>
+#include <linux/file.h>
 #include "internal.h"
 
 /*
@@ -796,7 +798,11 @@ int cachefiles_allocate_pages(struct fscache_retrieval *op,
 int cachefiles_write_page(struct fscache_storage *op, struct page *page)
 {
 	struct cachefiles_object *object;
-	struct address_space *mapping;
+	struct cachefiles_cache *cache;
+	mm_segment_t old_fs;
+	struct file *file;
+	loff_t pos;
+	void *data;
 	int ret;
 
 	ASSERT(op != NULL);
@@ -814,13 +820,33 @@ int cachefiles_write_page(struct fscache_storage *op, struct page *page)
 
 	ASSERT(S_ISREG(object->backer->d_inode->i_mode));
 
-	/* copy the page to ext3 and let it store it in its own time */
-	mapping = object->backer->d_inode->i_mapping;
-	ret = -EIO;
-	if (mapping->a_ops->write_one_page) {
-		ret = mapping->a_ops->write_one_page(mapping, page->index,
-						     page);
-		_debug("write_one_page -> %d", ret);
+	cache = container_of(object->fscache.cache,
+			     struct cachefiles_cache, cache);
+
+	/* write the page to the backing filesystem and let it store it in its
+	 * own time */
+	dget(object->backer);
+	mntget(cache->mnt);
+	file = dentry_open(object->backer, cache->mnt, O_RDWR,
+			   cache->cache_cred);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+	} else {
+		ret = -EIO;
+		if (file->f_op->write) {
+			pos = (loff_t) page->index << PAGE_SHIFT;
+			data = kmap(page);
+			old_fs = get_fs();
+			set_fs(KERNEL_DS);
+			ret = file->f_op->write(
+				file, (const void __user *) data, PAGE_SIZE,
+				&pos);
+			set_fs(old_fs);
+			kunmap(page);
+			if (ret != PAGE_SIZE)
+				ret = -EIO;
+		}
+		fput(file);
 	}
 
 	if (ret < 0) {

             reply	other threads:[~2009-04-03  9:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-03  9:41 David Howells [this message]
2009-04-03  9:41 ` [PATCH 2/4] CacheFiles: Revert the addition of write_one_page() David Howells
2009-04-03 13:50   ` Christoph Hellwig
2009-04-03 13:53   ` David Howells
2009-04-04  5:55     ` Nick Piggin
2009-04-03  9:41 ` [PATCH 3/4] FS-Cache: Use a radix tree to track pages being written rather than a page flag David Howells
2009-04-03  9:41 ` [PATCH 4/4] FS-Cache: Revert the addition of PG_owner_priv2/PG_fscache_write David Howells
2009-04-03  9:51 ` [PATCH 1/4] CacheFiles: Use the ->write() file op rather than a special kernel aop David Howells
2009-04-03 13:49 ` Christoph Hellwig
2009-04-03 13:57 ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090403094138.9510.80681.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nfsv4@linux-nfs.org \
    --cc=nickpiggin@yahoo.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).