linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: open-osd mailing-list <osd-dev@open-osd.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Al Viro <viro@ZenIV.linux.org.uk>
Subject: [PATCH version 2] exofs: Avoid using file_fsync()
Date: Mon, 15 Jun 2009 20:07:44 +0300	[thread overview]
Message-ID: <4A367FE0.4060508@panasas.com> (raw)
In-Reply-To: <4A367C51.4080305@panasas.com>

I'll test with below patch and also with Christoph version which
does:

diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index 0c13650..2c8abdd 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -46,16 +46,10 @@ static int exofs_file_fsync(struct file *filp, struct dentry *dentry,
 			    int datasync)
 {
 	int ret;
-	struct address_space *mapping = filp->f_mapping;
 	struct inode * inode = dentry->d_inode;
 	struct super_block * sb;
 
-	ret = filemap_write_and_wait(mapping);
-	if (ret)
-		return ret;
-
-	/* sync the inode attributes */
-	ret = write_inode_now(inode, 1);
+	ret = simple_fsync(filp, dentry, 1);
 
 	/* This is a good place to write the sb */
 	/* TODO: Sechedule an sb-sync on create */

On top of the below one. See which one feels better. What should I test for?

Thanks
Boaz

---
The use of file_fsync() in exofs_file_sync() is not necessary since it
does some extra stuff not used by exofs. Open code just the parts that
are currently needed.

TODO: Farther optimization can be done to sync the sb only on inode
update of new files, Usually the sb update is not needed in exofs.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/exofs.h |    3 +++
 fs/exofs/file.c  |   17 ++++++++++++-----
 fs/exofs/super.c |    2 +-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 0fd4c78..5d15512 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -156,6 +156,9 @@ ino_t exofs_parent_ino(struct dentry *child);
 int exofs_set_link(struct inode *, struct exofs_dir_entry *, struct page *,
 		    struct inode *);
 
+/* super.c               */
+int exofs_sync_fs(struct super_block *sb, int wait);
+
 /*********************
  * operation vectors *
  *********************/
diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index 6ed7fe4..0c13650 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -47,16 +47,23 @@ static int exofs_file_fsync(struct file *filp, struct dentry *dentry,
 {
 	int ret;
 	struct address_space *mapping = filp->f_mapping;
+	struct inode * inode = dentry->d_inode;
+	struct super_block * sb;
 
 	ret = filemap_write_and_wait(mapping);
 	if (ret)
 		return ret;
 
-	/*Note: file_fsync below also calles sync_blockdev, which is a no-op
-	 *      for exofs, but other then that it does sync_inode and
-	 *      sync_superblock which is what we need here.
-	 */
-	return file_fsync(filp, dentry, datasync);
+	/* sync the inode attributes */
+	ret = write_inode_now(inode, 1);
+
+	/* This is a good place to write the sb */
+	/* TODO: Sechedule an sb-sync on create */
+	sb = inode->i_sb;
+	if (sb->s_dirt)
+		exofs_sync_fs(sb, 1);
+
+	return ret;
 }
 
 static int exofs_flush(struct file *file, fl_owner_t id)
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 8216c5b..e4fa0ce 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -200,7 +200,7 @@ static const struct export_operations exofs_export_ops;
 /*
  * Write the superblock to the OSD
  */
-static int exofs_sync_fs(struct super_block *sb, int wait)
+int exofs_sync_fs(struct super_block *sb, int wait)
 {
 	struct exofs_sb_info *sbi;
 	struct exofs_fscb *fscb;
-- 
1.6.2.1


  reply	other threads:[~2009-06-15 17:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-15 13:31 [PATCH] exofs: Avoid using file_fsync() Boaz Harrosh
2009-06-15 13:50 ` Christoph Hellwig
2009-06-15 14:30   ` Boaz Harrosh
2009-06-15 16:52   ` Boaz Harrosh
2009-06-15 17:07     ` Boaz Harrosh [this message]
2009-06-15 17:21   ` Jeff Garzik
2009-06-15 17:23     ` Boaz Harrosh

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=4A367FE0.4060508@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=osd-dev@open-osd.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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).