[PATCH] f2fs: don't fail namespace operations on dirsync checkpoint errors

[Hengyu Liang]

Commit dafb84f092a6 ("f2fs: fix to propagate error from f2fs_sync_fs()")
made create, link, unlink, symlink, mkdir, mknod and rename return the
result of the checkpoint for a dirsync directory. That checkpoint runs
after the directory has changed, but the VFS takes an error to mean that
nothing happened and skips d_move(), d_delete() and S_DEAD. The dcache no
longer matches the directory.

For example, on a filesystem mounted with -o dirsync whose device was then
set read-only with BLKROSET, rename() fails with EROFS while readdir lists
only the new name and stat() only finds the old one. After a failed
rmdir(), stat() still finds the directory with no links, and a create
inside it fails with EROFS but leaves the file there. Once the device is
writable again, these "failed" changes reach the disk. A write error during
the checkpoint causes the same mismatch with EIO.

As commit 24c1a47f1ed2 ("f2fs: don't leave the hashed inode while it's
unlinked") notes for symlink, the final flush or checkpoint is not a reason
to roll back the operation. Ignore its result again in these paths. Errors
from before the directory changes are still returned, syncfs() still
reports the checkpoint failure, and the remount change from the same commit
is kept.

Fixes: dafb84f092a6 ("f2fs: fix to propagate error from f2fs_sync_fs()")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
---
With this patch a dirsync operation can return 0 although its checkpoint
failed, as before v7.3. In the write error test below the change is not on
disk after a remount, so the EIO was accurate about durability, but the VFS
cannot represent "changed but not durable". The filesystem has stopped by
then: later creates get EIO and syncfs() reports it.

Tested on x86-64 KVM with KASAN, f2fs dev 83f32c773255 with and without
this patch:
- 14 namespace operations (all eight callbacks plus rmdir, overwrite,
  cross-directory rename and whiteout) with -o sync, -o dirsync and
  FS_DIRSYNC_FL on a read-only device: EROFS and stat/readdir disagree
  before, success and they agree after. Remounted contents match on both.
  No change without dirsync or on a writable device.
- dm error target under -o sync, nine operations: EIO and the same mismatch
  before, 0 and no mismatch after.
- ENOTEMPTY, EEXIST, ENOENT and injected ENOSPC are still returned, with
  the namespace intact.
- fsck.f2fs --dry-run exits 0 on all images.

xfstests were not run; without a failing checkpoint the only difference is
ignoring a zero return.

 fs/f2fs/namei.c | 46 ++++++++++++++--------------------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index ce5d536d892b..9b127b38b6c6 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -402,11 +402,8 @@ static int f2fs_create(const struct mnt_idmap *idmap, struct inode *dir,
 
 	d_instantiate_new(dentry, inode);
 
-	if (IS_DIRSYNC(dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return err;
-	}
+	if (IS_DIRSYNC(dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_balance_fs(sbi, true);
 	return 0;
@@ -458,11 +455,8 @@ static int f2fs_link(struct dentry *old_dentry, struct inode *dir,
 
 	d_instantiate(dentry, inode);
 
-	if (IS_DIRSYNC(dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return err;
-	}
+	if (IS_DIRSYNC(dir))
+		f2fs_sync_fs(sbi->sb, 1);
 	return 0;
 out:
 	clear_inode_flag(inode, FI_INC_LINK);
@@ -642,7 +636,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 		d_invalidate(dentry);
 
 	if (IS_DIRSYNC(dir))
-		err = f2fs_sync_fs(F2FS_I_SB(dir)->sb, 1);
+		f2fs_sync_fs(F2FS_I_SB(dir)->sb, 1);
 out:
 	trace_f2fs_unlink_exit(d_inode(dentry), err);
 	return err;
@@ -730,7 +724,7 @@ static int f2fs_symlink(const struct mnt_idmap *idmap, struct inode *dir,
 	ret = filemap_write_and_wait_range(inode->i_mapping, 0,
 					   disk_link.len - 1);
 	if (!ret && IS_DIRSYNC(dir))
-		err = f2fs_sync_fs(sbi->sb, 1);
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_balance_fs(sbi, true);
 out:
@@ -787,11 +781,8 @@ static struct dentry *f2fs_mkdir(const struct mnt_idmap *idmap, struct inode *di
 
 	d_instantiate_new(dentry, inode);
 
-	if (IS_DIRSYNC(dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return ERR_PTR(err);
-	}
+	if (IS_DIRSYNC(dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_balance_fs(sbi, true);
 	return NULL;
@@ -845,11 +836,8 @@ static int f2fs_mknod(const struct mnt_idmap *idmap, struct inode *dir,
 
 	d_instantiate_new(dentry, inode);
 
-	if (IS_DIRSYNC(dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return err;
-	}
+	if (IS_DIRSYNC(dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_balance_fs(sbi, true);
 	return 0;
@@ -1149,11 +1137,8 @@ static int f2fs_rename(const struct mnt_idmap *idmap, struct inode *old_dir,
 
 	f2fs_unlock_op(sbi, &lc);
 
-	if (IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return err;
-	}
+	if (IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_update_time(sbi, REQ_TIME);
 	return 0;
@@ -1321,11 +1306,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	f2fs_unlock_op(sbi, &lc);
 
-	if (IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir)) {
-		err = f2fs_sync_fs(sbi->sb, 1);
-		if (err)
-			return err;
-	}
+	if (IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_update_time(sbi, REQ_TIME);
 	return 0;

base-commit: 83f32c773255720a661aa45d83e36278135f23f5
-- 
2.53.0