[PATCH V3 1/2] FS: Add generic data flush to fsync

[PATCH V3 1/2] FS: Add generic data flush to fsync

[Fabian Frederick]

This patch issues a flush in generic_file_fsync.
(Modern filesystems already do it)

-Behaviour can be reversed using /sys/devices/.../cache_type
-Filesystems can also call __generic_file_fsync with bool flush false

Suggested-by: Jan Kara <jack@suse.cz>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
V3: __generic_file_fsync = no flush
V2: No flag
V1: First version with MS_BARRIER flag

 fs/libfs.c         | 36 +++++++++++++++++++++++++++++++++---
 include/linux/fs.h |  1 +
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index a184424..4877906 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -3,6 +3,7 @@
  *	Library for filesystems writers.
  */
 
+#include <linux/blkdev.h>
 #include <linux/export.h>
 #include <linux/pagemap.h>
 #include <linux/slab.h>
@@ -923,16 +924,19 @@ struct dentry *generic_fh_to_parent(struct super_block *sb, struct fid *fid,
 EXPORT_SYMBOL_GPL(generic_fh_to_parent);
 
 /**
- * generic_file_fsync - generic fsync implementation for simple filesystems
+ * __generic_file_fsync - generic fsync implementation for simple filesystems
+ *
  * @file:	file to synchronize
+ * @start:	start offset in bytes
+ * @end:	end offset in bytes (inclusive)
  * @datasync:	only synchronize essential metadata if true
  *
  * This is a generic implementation of the fsync method for simple
  * filesystems which track all non-inode metadata in the buffers list
  * hanging off the address_space structure.
  */
-int generic_file_fsync(struct file *file, loff_t start, loff_t end,
-		       int datasync)
+int __generic_file_fsync(struct file *file, loff_t start, loff_t end,
+				 int datasync)
 {
 	struct inode *inode = file->f_mapping->host;
 	int err;
@@ -952,10 +956,36 @@ int generic_file_fsync(struct file *file, loff_t start, loff_t end,
 	err = sync_inode_metadata(inode, 1);
 	if (ret == 0)
 		ret = err;
+
 out:
 	mutex_unlock(&inode->i_mutex);
 	return ret;
 }
+EXPORT_SYMBOL(__generic_file_fsync);
+
+/**
+ * generic_file_fsync - generic fsync implementation for simple filesystems
+ *			with flush
+ * @file:	file to synchronize
+ * @start:	start offset in bytes
+ * @end:	end offset in bytes (inclusive)
+ * @datasync:	only synchronize essential metadata if true
+ *
+ */
+
+int generic_file_fsync(struct file *file, loff_t start, loff_t end,
+		       int datasync)
+{
+	struct inode *inode = file->f_mapping->host;
+	int err;
+
+	err = __generic_file_fsync(file, start, end, datasync);
+	if (err)
+		return err;
+
+	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
+
+}
 EXPORT_SYMBOL(generic_file_fsync);
 
 /**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8780312..c3f46e4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2590,6 +2590,7 @@ extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
 extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 		const void __user *from, size_t count);
 
+extern int __generic_file_fsync(struct file *, loff_t, loff_t, int);
 extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
 
 extern int generic_check_addressable(unsigned, u64);
-- 
1.8.4.5

Re: [PATCH V3 1/2] FS: Add generic data flush to fsync

[Jan Kara]

On Mon 28-04-14 23:12:39, Fabian Frederick wrote:
> This patch issues a flush in generic_file_fsync.
> (Modern filesystems already do it)
> 
> -Behaviour can be reversed using /sys/devices/.../cache_type
> -Filesystems can also call __generic_file_fsync with bool flush false
  The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> V3: __generic_file_fsync = no flush
> V2: No flag
> V1: First version with MS_BARRIER flag
> 
>  fs/libfs.c         | 36 +++++++++++++++++++++++++++++++++---
>  include/linux/fs.h |  1 +
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index a184424..4877906 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -3,6 +3,7 @@
>   *	Library for filesystems writers.
>   */
>  
> +#include <linux/blkdev.h>
>  #include <linux/export.h>
>  #include <linux/pagemap.h>
>  #include <linux/slab.h>
> @@ -923,16 +924,19 @@ struct dentry *generic_fh_to_parent(struct super_block *sb, struct fid *fid,
>  EXPORT_SYMBOL_GPL(generic_fh_to_parent);
>  
>  /**
> - * generic_file_fsync - generic fsync implementation for simple filesystems
> + * __generic_file_fsync - generic fsync implementation for simple filesystems
> + *
>   * @file:	file to synchronize
> + * @start:	start offset in bytes
> + * @end:	end offset in bytes (inclusive)
>   * @datasync:	only synchronize essential metadata if true
>   *
>   * This is a generic implementation of the fsync method for simple
>   * filesystems which track all non-inode metadata in the buffers list
>   * hanging off the address_space structure.
>   */
> -int generic_file_fsync(struct file *file, loff_t start, loff_t end,
> -		       int datasync)
> +int __generic_file_fsync(struct file *file, loff_t start, loff_t end,
> +				 int datasync)
>  {
>  	struct inode *inode = file->f_mapping->host;
>  	int err;
> @@ -952,10 +956,36 @@ int generic_file_fsync(struct file *file, loff_t start, loff_t end,
>  	err = sync_inode_metadata(inode, 1);
>  	if (ret == 0)
>  		ret = err;
> +
>  out:
>  	mutex_unlock(&inode->i_mutex);
>  	return ret;
>  }
> +EXPORT_SYMBOL(__generic_file_fsync);
> +
> +/**
> + * generic_file_fsync - generic fsync implementation for simple filesystems
> + *			with flush
> + * @file:	file to synchronize
> + * @start:	start offset in bytes
> + * @end:	end offset in bytes (inclusive)
> + * @datasync:	only synchronize essential metadata if true
> + *
> + */
> +
> +int generic_file_fsync(struct file *file, loff_t start, loff_t end,
> +		       int datasync)
> +{
> +	struct inode *inode = file->f_mapping->host;
> +	int err;
> +
> +	err = __generic_file_fsync(file, start, end, datasync);
> +	if (err)
> +		return err;
> +
> +	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> +
> +}
>  EXPORT_SYMBOL(generic_file_fsync);
>  
>  /**
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8780312..c3f46e4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2590,6 +2590,7 @@ extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
>  extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
>  		const void __user *from, size_t count);
>  
> +extern int __generic_file_fsync(struct file *, loff_t, loff_t, int);
>  extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
>  
>  extern int generic_check_addressable(unsigned, u64);
> -- 
> 1.8.4.5
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V3 1/2] FS: Add generic data flush to fsync

[Fabian Frederick]

On Tue, 29 Apr 2014 18:19:07 +0200
Jan Kara <jack@suse.cz> wrote:

> On Mon 28-04-14 23:12:39, Fabian Frederick wrote:
> > This patch issues a flush in generic_file_fsync.
> > (Modern filesystems already do it)
> > 
> > -Behaviour can be reversed using /sys/devices/.../cache_type
> > -Filesystems can also call __generic_file_fsync with bool flush false
>   The patch looks good. You can add:
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> 								Honza

Just noticed I forgot to remove "with bool flush false" in patch description above
(-Filesystems can also call __generic_file_fsync "with bool flush false")
Tell me if I need to send the patch again.

Fabian