[Ocfs2-devel] [PATCH 1/2 v2] ocfs2: Flush drive's caches on fdatasync

[Ocfs2-devel] [PATCH 1/2 v2] ocfs2: Flush drive's caches on fdatasync

[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 <tao.ma@oracle.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ocfs2/file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

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

								Honza

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 2b10b36..3ed8efd 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -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);
-- 
1.6.4.2

[Ocfs2-devel] [PATCH 2/2 v2] ocfs2: Remove ocfs2_sync_inode()

[Jan Kara]

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 <jack@suse.cz>
---
 fs/ocfs2/file.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 3ed8efd..b7cfa6b 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -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
-- 
1.6.4.2

[Ocfs2-devel] [PATCH 1/2 v2] ocfs2: Flush drive's caches on fdatasync

[Wengang Wang]

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. 
On 10-08-02 15:04, Jan Kara wrote:
> 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 <tao.ma@oracle.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ocfs2/file.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
>   Joel, would you merge this and the following patch please? Thanks.
> 
> 								Honza
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 2b10b36..3ed8efd 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -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);
> -- 
> 1.6.4.2
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/2 v2] ocfs2: Flush drive's caches on fdatasync

[Jan Kara]

  Hi,

On Mon 02-08-10 21:55:39, Wengang Wang wrote:
> 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
> On 10-08-02 15:04, Jan Kara wrote:
> > 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 <tao.ma@oracle.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/ocfs2/file.c |   11 ++++++++++-
> >  1 files changed, 10 insertions(+), 1 deletions(-)
> > 
> >   Joel, would you merge this and the following patch please? Thanks.
> > 
> > 								Honza
> > 
> > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> > index 2b10b36..3ed8efd 100644
> > --- a/fs/ocfs2/file.c
> > +++ b/fs/ocfs2/file.c
> > @@ -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);
> > -- 
> > 1.6.4.2
> > 
> > 
> > _______________________________________________
> > Ocfs2-devel mailing list
> > Ocfs2-devel at oss.oracle.com
> > http://oss.oracle.com/mailman/listinfo/ocfs2-devel
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Ocfs2-devel] [PATCH 1/2 v2] ocfs2: Flush drive's caches on fdatasync

[Wengang Wang]

Appreciated!

thanks a lot.
wengang.
On 10-08-02 18:38, Jan Kara wrote:
>   Hi,
> 
> On Mon 02-08-10 21:55:39, Wengang Wang wrote:
> > 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
> > On 10-08-02 15:04, Jan Kara wrote:
> > > 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 <tao.ma@oracle.com>
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > ---
> > >  fs/ocfs2/file.c |   11 ++++++++++-
> > >  1 files changed, 10 insertions(+), 1 deletions(-)
> > > 
> > >   Joel, would you merge this and the following patch please? Thanks.
> > > 
> > > 								Honza
> > > 
> > > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> > > index 2b10b36..3ed8efd 100644
> > > --- a/fs/ocfs2/file.c
> > > +++ b/fs/ocfs2/file.c
> > > @@ -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);
> > > -- 
> > > 1.6.4.2
> > > 
> > > 
> > > _______________________________________________
> > > Ocfs2-devel mailing list
> > > Ocfs2-devel at oss.oracle.com
> > > http://oss.oracle.com/mailman/listinfo/ocfs2-devel
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR