ocfs2: Flush drive's caches on fdatasync

PATCH Superseded Jan Kara 2 participants 5 messages Aug 2, 2010 – Aug 3, 2010 OCFS2

Share & tools

Jan Kara

author

2010-08-02 13:04 UTC #0 lore raw

PATCH v2 1/2 ocfs2: Flush drive's caches on fdatasync

We have to issue a cache flush during fdatasync even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk to observe fdatasync() guarantees.

Acked-by Tao Ma <[email protected]>

Signed-off-by Jan Kara <[email protected]>

---
 fs/ocfs2/file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

Joel, would you merge this and the following patch please? Thanks.

Honza

fs/ocfs2/file.c +10 -1

index 2b10b36..3ed8efd 100644
@@ -36,6 +36,7 @@
 #include <linux/writeback.h>
 #include <linux/falloc.h>
 #include <linux/quotaops.h>
+#include <linux/blkdev.h>
 
 #define MLOG_MASK_PREFIX ML_INODE
 #include <cluster/masklog.h>

@@ -190,8 +191,16 @@ static int ocfs2_sync_file(struct file *file, int datasync)
 	if (err)
 		goto bail;
 
-	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
+	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) {
+		/*
+		 * We still have to flush drive's caches to get data to the
+		 * platter
+		 */
+		if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
+			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
+					   NULL, BLKDEV_IFL_WAIT);
 		goto bail;
+	}
 
 	journal = osb->journal->j_journal;
 	err = jbd2_journal_force_commit(journal);

Signature · 1 line

1.6.4.2

Jan Kara

author

2010-08-02 13:04 UTC #1 lore raw

PATCH v2 2/2 ocfs2: Remove ocfs2_sync_inode()

ocfs2_sync_inode() is used only from ocfs2_sync_file(). But all data has already been written before calling ocfs2_sync_file() and ocfs2 doesn't use inode's private_list for tracking metadata buffers thus sync_mapping_buffers() is superfluous as well.

Signed-off-by Jan Kara <[email protected]>

---
 fs/ocfs2/file.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

fs/ocfs2/file.c +0 ~~-10~~

index 3ed8efd..b7cfa6b 100644
@@ -64,12 +64,6 @@
 
 #include "buffer_head_io.h"
 
-static int ocfs2_sync_inode(struct inode *inode)
-{
-	filemap_fdatawrite(inode->i_mapping);
-	return sync_mapping_buffers(inode->i_mapping);
-}
-
 static int ocfs2_init_file_private(struct inode *inode, struct file *file)
 {
 	struct ocfs2_file_private *fp;

@@ -187,10 +181,6 @@ static int ocfs2_sync_file(struct file *file, int datasync)
 	mlog_entry("(0x%p, 0x%p, %d, '%.*s')\n", file, dentry, datasync,
 		   dentry->d_name.len, dentry->d_name.name);
 
-	err = ocfs2_sync_inode(dentry->d_inode);
-	if (err)
-		goto bail;
-
 	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) {
 		/*
 		 * We still have to flush drive's caches to get data to the

Signature · 1 line

1.6.4.2

Wengang Wang

2010-08-02 13:55 UTC #2 lore raw

PATCH v2 1/2 ocfs2: Flush drive's caches on fdatasync

In reply to Jan Kara

Hi Jan,

I think it will be better if you mention the BARRIER mount option in the patch description. That will accurately match your patch.

regards,
wengang.

Quoted 52 lines · Jan Kara

We have to issue a cache flush during fdatasync even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk to observe fdatasync() guarantees.

Acked-by Tao Ma <[email protected]>

Signed-off-by Jan Kara <[email protected]>

---
 fs/ocfs2/file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

Joel, would you merge this and the following patch please? Thanks.

Honza

fs/ocfs2/file.c +10 -1

index 2b10b36..3ed8efd 100644
@@ -36,6 +36,7 @@
 #include <linux/writeback.h>
 #include <linux/falloc.h>
 #include <linux/quotaops.h>
+#include <linux/blkdev.h>
 
 #define MLOG_MASK_PREFIX ML_INODE
 #include <cluster/masklog.h>

@@ -190,8 +191,16 @@ static int ocfs2_sync_file(struct file *file, int datasync)
 	if (err)
 		goto bail;
 
-	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
+	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) {
+		/*
+		 * We still have to flush drive's caches to get data to the
+		 * platter
+		 */
+		if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
+			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
+					   NULL, BLKDEV_IFL_WAIT);
 		goto bail;
+	}
 
 	journal = osb->journal->j_journal;
 	err = jbd2_journal_force_commit(journal);

Signature · 7 lines

1.6.4.2


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Jan Kara

author

2010-08-02 16:38 UTC #3 lore raw

PATCH v2 1/2 ocfs2: Flush drive's caches on fdatasync

Hi,

Quoted 2 lines · Wengang Wang

I think it will be better if you mention the BARRIER mount option in the patch description. That will accurately match your patch.

OK. So the description could be:
When 'barrier' mount option is specified, we have to issue a cache flush during fdatasync. We have to do this even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk so that they are not lost in case of crash.

Better?

Honza

Quoted 53 lines · Jan Kara

Quoted 52 lines

We have to issue a cache flush during fdatasync even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk to observe fdatasync() guarantees.

Acked-by Tao Ma <[email protected]>

Signed-off-by Jan Kara <[email protected]>

---
 fs/ocfs2/file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

Joel, would you merge this and the following patch please? Thanks.

Honza

fs/ocfs2/file.c +10 -1

index 2b10b36..3ed8efd 100644
@@ -36,6 +36,7 @@
 #include <linux/writeback.h>
 #include <linux/falloc.h>
 #include <linux/quotaops.h>
+#include <linux/blkdev.h>
 
 #define MLOG_MASK_PREFIX ML_INODE
 #include <cluster/masklog.h>

@@ -190,8 +191,16 @@ static int ocfs2_sync_file(struct file *file, int datasync)
 	if (err)
 		goto bail;
 
-	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
+	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) {
+		/*
+		 * We still have to flush drive's caches to get data to the
+		 * platter
+		 */
+		if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
+			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
+					   NULL, BLKDEV_IFL_WAIT);
 		goto bail;
+	}
 
 	journal = osb->journal->j_journal;
 	err = jbd2_journal_force_commit(journal);

Signature · 7 lines

1.6.4.2


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Wengang Wang

2010-08-03 01:13 UTC #4 lore raw

PATCH v2 1/2 ocfs2: Flush drive's caches on fdatasync

Appreciated!

thanks a lot.
wengang.

Quoted 70 lines · Jan Kara

Hi,
Quoted 2 lines · Wengang Wang
I think it will be better if you mention the BARRIER mount option in the patch description. That will accurately match your patch.
OK. So the description could be:
When 'barrier' mount option is specified, we have to issue a cache flush during fdatasync. We have to do this even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk so that they are not lost in case of crash.
Better?
Honza
Quoted 53 lines · Jan Kara
Quoted 52 lines
We have to issue a cache flush during fdatasync even if inode doesn't have I_DIRTY_DATASYNC set because we still have to get written *data* to disk to observe fdatasync() guarantees.
Acked-by Tao Ma <[email protected]>
Signed-off-by Jan Kara <[email protected]>
---
 fs/ocfs2/file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)
Joel, would you merge this and the following patch please? Thanks.
Honza
fs/ocfs2/file.c +10 -1
index 2b10b36..3ed8efd 100644
@@ -36,6 +36,7 @@
 #include <linux/writeback.h>
 #include <linux/falloc.h>
 #include <linux/quotaops.h>
+#include <linux/blkdev.h>
 
 #define MLOG_MASK_PREFIX ML_INODE
 #include <cluster/masklog.h>
@@ -190,8 +191,16 @@ static int ocfs2_sync_file(struct file *file, int datasync)
 	if (err)
 		goto bail;
 
-	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
+	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) {
+		/*
+		 * We still have to flush drive's caches to get data to the
+		 * platter
+		 */
+		if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
+			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
+					   NULL, BLKDEV_IFL_WAIT);
 		goto bail;
+	}
 
 	journal = osb->journal->j_journal;
 	err = jbd2_journal_force_commit(journal);
Signature · 7 lines
1.6.4.2


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR