[Devel] [PATCH VZ10 v2 2/2] ext4: rework mfsync support

[Pavel Tikhomirov]

General improvements:

* Remove unused datawriteback.
* Check nr_files before accesing members in files[0].
* Remove excess {} in oneliner if condition.
* Check lack of journal early.
* Remove and add some newlines and spaces to improve readability.
* Add err3 to fix error handling in blkdev_issue_flush error path.

Make similar changes like in mainstream in ext4_sync_file:

* Replace J_ASSERT to ASSERT similar to [1].
* Remove inode_lock as it's not needed after [2].
* Force commit only for non-regular files, similar to [3] + [4].
* Use ext4_fc_commit instead of jbd2_complete_transaction as in [5].
* Track writeback errors in ext4_sync_files as in [6].
* Replace mapping->nrpages with equivalent mapping_needs_writeback()
  helper.

837c23fbc1b8 ("ext4: use ASSERT() to replace J_ASSERT()") [1]
92e6222dfb85 ("ext4: remove i_mutex from ext4_file_sync()") [2]
e360c6ed7274 ("ext4: Drop special handling of journalled data from ext4_sync_file()") [3]
1077b2d53ef5 ("ext4: fix fsync for non-directories") [4]
aa75f4d3daae ("ext4: main fast-commit commit path") [5]
95cb67138746 ("ext4: track writeback errors using the generic trackinginfrastructure") [6]

https://virtuozzo.atlassian.net/browse/VSTOR-107255
Fixes: c266e3fb67ca ("ext4: add mfsync support")
Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

Feature: ext4: optimized sync of a set of files - mfsync()
---
v2: use mapping_needs_writeback helper
---
 fs/ext4/fsync.c | 51 +++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index 9819848c17a3..e0b0e0d0295c 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -180,25 +180,30 @@ int ext4_sync_files(struct file **files, unsigned int *flags, unsigned int nr_fi
 {
 	struct super_block *sb;
 	journal_t *journal;
-	int err = 0, err2 = 0, i = 0, j = 0;
-	int force_commit = 0, datawriteback = 0;
+	int err = 0, err2 = 0, err3 = 0;
+	int i = 0, j = 0;
+	int force_commit = 0;
 	tid_t commit_tid = 0;
 	int need_barrier = 0;
 	int ret;
 
+	if (!nr_files)
+		return 0;
 	sb = files[0]->f_mapping->host->i_sb;
+
 	ret = ext4_emergency_state(sb);
 	if (unlikely(ret))
 		return ret;
 
-	J_ASSERT(ext4_journal_current_handle() == NULL);
-	if (!nr_files)
+	ASSERT(ext4_journal_current_handle() == NULL);
+
+	if (sb_rdonly(sb))
 		return 0;
 
 	journal = EXT4_SB(sb)->s_journal;
-	if (sb_rdonly(sb)) {
-		return 0;
-	}
+	if (!journal)
+		 return -ENOTSUPP;
+
 	for (i = 0; i < nr_files; i++) {
 		struct address_space * mapping = files[i]->f_mapping;
 		struct inode *inode = mapping->host;
@@ -207,13 +212,13 @@ int ext4_sync_files(struct file **files, unsigned int *flags, unsigned int nr_fi
 			err = -EINVAL;
 			goto out;
 		}
-		if (!mapping->nrpages)
+
+		if (!mapping_needs_writeback(mapping))
 			continue;
 
 		err = filemap_fdatawrite(mapping);
 		if (err)
 			break;
-
 	}
 	/*
 	 * Even if the above returned error, the pages may be
@@ -225,32 +230,27 @@ int ext4_sync_files(struct file **files, unsigned int *flags, unsigned int nr_fi
 		goto out;
 
 	for (j = 0; j < i; j++) {
-		struct address_space * mapping = files[j]->f_mapping;
+		struct address_space *mapping = files[j]->f_mapping;
 		struct inode *inode = mapping->host;
 		struct ext4_inode_info *ei = EXT4_I(inode);
 		unsigned int datasync = flags[j];
 		tid_t tid;
 
-		if (mapping->nrpages) {
+		if (mapping_needs_writeback(mapping))
 			err2 = filemap_fdatawait(mapping);
 			if (!err || err2 == -EIO)
 				err = err2;
 		}
 
-		inode_lock_shared(inode);
-		force_commit  |= ext4_should_journal_data(inode);
-		datawriteback |= ext4_should_writeback_data(inode);
+		force_commit |= !S_ISREG(inode->i_mode);
 		tid = datasync ? ei->i_datasync_tid : ei->i_sync_tid;
-		inode_unlock_shared(inode);
 		trace_ext4_sync_files_iterate(files[j]->f_path.dentry, tid, datasync);
 		if (j == 0 || !tid_geq(commit_tid, tid))
 			commit_tid = tid;
 	}
 
 	/* Ext4 specific stuff starts here */
-	if (!journal) {
-		 return -ENOTSUPP;
-	} else if (force_commit) {
+	if (force_commit) {
 		/* data=journal:
 		 *  filemap_fdatawrite won't do anything (the buffers are clean).
 		 *  ext4_force_commit will write the file data into the journal and
@@ -271,17 +271,26 @@ int ext4_sync_files(struct file **files, unsigned int *flags, unsigned int nr_fi
 		    !jbd2_trans_will_send_data_barrier(journal, commit_tid))
 			need_barrier = true;
 
-		err2 = jbd2_complete_transaction(journal, commit_tid);
+		err2 = ext4_fc_commit(journal, commit_tid);
 		/* Even if we had to wait for commit completion, it does not
 		 * mean a flush has been issued after data demanded by this
 		 * fsync were written back. Commit could be in state after
 		 * it is already done, but not yet in state where we should
 		 * not wait.
 		 */
-		if (need_barrier)
-			err2 = blkdev_issue_flush(sb->s_bdev);
+		if (need_barrier) {
+			err3 = blkdev_issue_flush(sb->s_bdev);
+			if (!err2 || err3 == -EIO)
+				err2 = err3;
+		}
 	}
 out:
+	for (i = 0; i < nr_files; i++) {
+		err3 = file_check_and_advance_wb_err(files[i]);
+		if (!err2 || err3 == -EIO)
+			err2 = err3;
+	}
+
 	trace_ext4_sync_files_exit(files[0]->f_path.dentry, commit_tid, need_barrier);
 	if (!err || err2 == -EIO)
 		err = err2;
-- 
2.50.1