[PATCH 41/41] fs: Unify generic_file_fsync() with mmb methods

[Jan Kara <jack@suse.cz>]

__generic_file_fsync() is practically identical to
generic_mmb_fsync_noflush() with one subtle difference:

1) __generic_file_fsync() takes inode lock when calling writing out the
inode.
2) generic_mmb_fsync_noflush() calls mmb_sync_buffers().

Taking inode lock when writing out the inode seems pointless in
particular because there are lots of places (most notably sync(2) path)
that don't do that so hardly anything can depend on it. When NULL is
passed to generic_mmb_fsync_noflush(), mmb_sync_buffers() is not called
so that difference is not a problem.

So let's remove __generic_file_fsync() and use
generic_mmb_fsync_noflush() instead to reduce code duplication. Arguably
this leaks a bit of buffer_head knowledge into fs/libfs.c which is not
great but avoiding the duplication seems worth it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/buffer.c                 | 74 -------------------------------------
 fs/exfat/file.c             |  2 +-
 fs/libfs.c                  | 57 ++++++++++++++++------------
 include/linux/buffer_head.h |  5 ---
 include/linux/fs.h          | 12 +++++-
 5 files changed, 45 insertions(+), 105 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 43aca5b7969f..591aed740601 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -621,80 +621,6 @@ int mmb_sync_buffers(struct mapping_metadata_bhs *mmb)
 }
 EXPORT_SYMBOL(mmb_sync_buffers);
 
-/**
- * generic_mmb_fsync_noflush - generic buffer fsync implementation
- * for simple filesystems with no inode lock
- *
- * @file:	file to synchronize
- * @mmb:	list of metadata bhs to flush
- * @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_mmb_fsync_noflush(struct file *file,
-			      struct mapping_metadata_bhs *mmb,
-			      loff_t start, loff_t end, bool datasync)
-{
-	struct inode *inode = file->f_mapping->host;
-	int err;
-	int ret = 0;
-
-	err = file_write_and_wait_range(file, start, end);
-	if (err)
-		return err;
-
-	if (mmb)
-		ret = mmb_sync_buffers(mmb);
-	if (!(inode_state_read_once(inode) & I_DIRTY_ALL))
-		goto out;
-	if (datasync && !(inode_state_read_once(inode) & I_DIRTY_DATASYNC))
-		goto out;
-
-	err = sync_inode_metadata(inode, 1);
-	if (ret == 0)
-		ret = err;
-
-out:
-	/* check and advance again to catch errors after syncing out buffers */
-	err = file_check_and_advance_wb_err(file);
-	if (ret == 0)
-		ret = err;
-	return ret;
-}
-EXPORT_SYMBOL(generic_mmb_fsync_noflush);
-
-/**
- * generic_mmb_fsync - generic buffer fsync implementation
- * for simple filesystems with no inode lock
- *
- * @file:	file to synchronize
- * @mmb:	list of metadata bhs to flush
- * @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. This also makes sure that
- * a device cache flush operation is called at the end.
- */
-int generic_mmb_fsync(struct file *file, struct mapping_metadata_bhs *mmb,
-		      loff_t start, loff_t end, bool datasync)
-{
-	struct inode *inode = file->f_mapping->host;
-	int ret;
-
-	ret = generic_mmb_fsync_noflush(file, mmb, start, end, datasync);
-	if (!ret)
-		ret = blkdev_issue_flush(inode->i_sb->s_bdev);
-	return ret;
-}
-EXPORT_SYMBOL(generic_mmb_fsync);
-
 /*
  * Called when we've recently written block `bblock', and it is known that
  * `bblock' was for a buffer_boundary() buffer.  This means that the block at
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 90cd540afeaa..fe6eb391eb4e 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -577,7 +577,7 @@ int exfat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 	if (unlikely(exfat_forced_shutdown(inode->i_sb)))
 		return -EIO;
 
-	err = __generic_file_fsync(filp, start, end, datasync);
+	err = generic_mmb_fsync_noflush(filp, NULL, start, end, datasync);
 	if (err)
 		return err;
 
diff --git a/fs/libfs.c b/fs/libfs.c
index 548e119668df..7c1d78862e39 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -18,7 +18,7 @@
 #include <linux/exportfs.h>
 #include <linux/iversion.h>
 #include <linux/writeback.h>
-#include <linux/buffer_head.h> /* sync_mapping_buffers */
+#include <linux/buffer_head.h> /* mmb_sync_buffers() */
 #include <linux/fs_context.h>
 #include <linux/pseudo_fs.h>
 #include <linux/fsnotify.h>
@@ -1539,19 +1539,22 @@ 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_mmb_fsync_noflush - generic buffer fsync implementation
+ * for simple filesystems with no inode lock
  *
- * @file:	file to synchronize
- * @start:	start offset in bytes
- * @end:	end offset in bytes (inclusive)
- * @datasync:	only synchronize essential metadata if true
+ * @file:       file to synchronize
+ * @mmb:        list of metadata bhs to flush
+ * @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_mmb_fsync_noflush(struct file *file,
+			      struct mapping_metadata_bhs *mmb,
+			      loff_t start, loff_t end, bool datasync)
 {
 	struct inode *inode = file->f_mapping->host;
 	int err;
@@ -1561,45 +1564,53 @@ int __generic_file_fsync(struct file *file, loff_t start, loff_t end,
 	if (err)
 		return err;
 
-	inode_lock(inode);
+	if (mmb)
+		ret = mmb_sync_buffers(mmb);
 	if (!(inode_state_read_once(inode) & I_DIRTY_ALL))
 		goto out;
 	if (datasync && !(inode_state_read_once(inode) & I_DIRTY_DATASYNC))
 		goto out;
 
-	ret = sync_inode_metadata(inode, 1);
+	err = sync_inode_metadata(inode, 1);
+	if (ret == 0)
+		ret = err;
+
 out:
-	inode_unlock(inode);
 	/* check and advance again to catch errors after syncing out buffers */
 	err = file_check_and_advance_wb_err(file);
 	if (ret == 0)
 		ret = err;
 	return ret;
 }
-EXPORT_SYMBOL(__generic_file_fsync);
+EXPORT_SYMBOL(generic_mmb_fsync_noflush);
 
 /**
- * generic_file_fsync - generic fsync implementation for simple filesystems
- *			with flush
+ * generic_mmb_fsync - generic buffer fsync implementation
+ * for simple filesystems with no inode lock
+ *
  * @file:	file to synchronize
+ * @mmb:	list of metadata bhs to flush
  * @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. This also makes sure that
+ * a device cache flush operation is called at the end.
  */
-
-int generic_file_fsync(struct file *file, loff_t start, loff_t end,
-		       int datasync)
+int generic_mmb_fsync(struct file *file, struct mapping_metadata_bhs *mmb,
+		      loff_t start, loff_t end, bool datasync)
 {
 	struct inode *inode = file->f_mapping->host;
-	int err;
+	int ret;
 
-	err = __generic_file_fsync(file, start, end, datasync);
-	if (err)
-		return err;
-	return blkdev_issue_flush(inode->i_sb->s_bdev);
+	ret = generic_mmb_fsync_noflush(file, mmb, start, end, datasync);
+	if (!ret)
+		ret = blkdev_issue_flush(inode->i_sb->s_bdev);
+	return ret;
 }
-EXPORT_SYMBOL(generic_file_fsync);
+EXPORT_SYMBOL(generic_mmb_fsync);
 
 /**
  * generic_check_addressable - Check addressability of file system
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 74fcc9a03c32..f003a1937826 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -207,11 +207,6 @@ void end_buffer_write_sync(struct buffer_head *bh, int uptodate);
 
 /* Things to do with metadata buffers list */
 void mmb_mark_buffer_dirty(struct buffer_head *bh, struct mapping_metadata_bhs *mmb);
-int generic_mmb_fsync_noflush(struct file *file,
-			      struct mapping_metadata_bhs *mmb,
-			      loff_t start, loff_t end, bool datasync);
-int generic_mmb_fsync(struct file *file, struct mapping_metadata_bhs *mmb,
-		      loff_t start, loff_t end, bool datasync);
 void clean_bdev_aliases(struct block_device *bdev, sector_t block,
 			sector_t len);
 static inline void clean_bdev_bh_alias(struct buffer_head *bh)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index caa9203ed213..32178e53d448 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3298,8 +3298,16 @@ void simple_offset_destroy(struct offset_ctx *octx);
 
 extern const struct file_operations simple_offset_dir_operations;
 
-extern int __generic_file_fsync(struct file *, loff_t, loff_t, int);
-extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
+int generic_mmb_fsync_noflush(struct file *file,
+			      struct mapping_metadata_bhs *mmb,
+			      loff_t start, loff_t end, bool datasync);
+int generic_mmb_fsync(struct file *file, struct mapping_metadata_bhs *mmb,
+		      loff_t start, loff_t end, bool datasync);
+static inline int generic_file_fsync(struct file *file,
+				     loff_t start, loff_t end, int datasync)
+{
+	return generic_mmb_fsync(file, NULL, start, end, datasync);
+}
 
 extern int generic_check_addressable(unsigned, u64);
 
-- 
2.51.0