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 <[email protected]>
Suggested-by: Christoph Hellwig <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: "Theodore Ts'o" <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
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
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 <[email protected]>
Honza
>
> Suggested-by: Jan Kara <[email protected]>
> Suggested-by: Christoph Hellwig <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Alexander Viro <[email protected]>
> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
> ---
> 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 <[email protected]>
SUSE Labs, CR
On Tue, 29 Apr 2014 18:19:07 +0200
Jan Kara <[email protected]> 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 <[email protected]>
>
> 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