[PATCH] f2fs: don't leave the hashed inode while it's unlinked

[PATCH] f2fs: don't leave the hashed inode while it's unlinked

[Jaegeuk Kim]

f2fs_symlink()
1. f2fs_new_inode
2. f2fs_add_link
3. page_symlink
4. flush dirty pages and or checkpoint

Step 4 is nice to succeed, which doesn't become a reason to roll back
the created symlink. OTOH, if we get an error till step 3, let's roll
back and remove the cached inode.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/namei.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 37897f4321c0..7b318cee2ed6 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -704,14 +704,16 @@ static int f2fs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	f2fs_alloc_nid_done(sbi, inode->i_ino);
 
 	err = fscrypt_encrypt_symlink(inode, symname, len, &disk_link);
-	if (err)
-		goto err_out;
-
-	err = page_symlink(inode, disk_link.name, disk_link.len);
+	if (!err)
+		err = page_symlink(inode, disk_link.name, disk_link.len);
 
-err_out:
 	d_instantiate_new(dentry, inode);
 
+	if (err) {
+		f2fs_unlink(dir, dentry);
+		goto out_f2fs_handle_failed_inode;
+	}
+
 	/*
 	 * Let's flush symlink data in order to avoid broken symlink as much as
 	 * possible. Nevertheless, fsyncing is the best way, but there is no
@@ -721,16 +723,10 @@ static int f2fs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	 * If the symlink path is stored into inline_data, there is no
 	 * performance regression.
 	 */
-	if (!err) {
-		err = filemap_write_and_wait_range(inode->i_mapping, 0,
-						   disk_link.len - 1);
-
-		if (!err && IS_DIRSYNC(dir))
-			err = f2fs_sync_fs(sbi->sb, 1);
-	}
-
-	if (err)
-		f2fs_unlink(dir, dentry);
+	err = filemap_write_and_wait_range(inode->i_mapping, 0,
+					   disk_link.len - 1);
+	if (!err && IS_DIRSYNC(dir))
+		f2fs_sync_fs(sbi->sb, 1);
 
 	f2fs_balance_fs(sbi, true);
 	goto out_free_encrypted_link;
-- 
2.55.0.737.g08866a6d13-goog

Re: [f2fs-dev] [PATCH] f2fs: don't leave the hashed inode while it's unlinked

[Chao Yu]

On 8/19/26 04:01, Jaegeuk Kim via Linux-f2fs-devel wrote:
> f2fs_symlink()
> 1. f2fs_new_inode
> 2. f2fs_add_link
> 3. page_symlink
> 4. flush dirty pages and or checkpoint
> 
> Step 4 is nice to succeed, which doesn't become a reason to roll back
> the created symlink. OTOH, if we get an error till step 3, let's roll
> back and remove the cached inode.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/namei.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 37897f4321c0..7b318cee2ed6 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -704,14 +704,16 @@ static int f2fs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  	f2fs_alloc_nid_done(sbi, inode->i_ino);
>  
>  	err = fscrypt_encrypt_symlink(inode, symname, len, &disk_link);
> -	if (err)
> -		goto err_out;
> -
> -	err = page_symlink(inode, disk_link.name, disk_link.len);
> +	if (!err)
> +		err = page_symlink(inode, disk_link.name, disk_link.len);
>  
> -err_out:
>  	d_instantiate_new(dentry, inode);

IIUC, Al means we'd better not call d_instantiate_new() before every thing is
done? IOW, do not call d_instantiate_new() if we're in error handling.

Let me know if I missed something.

Thanks,

>  
> +	if (err) {
> +		f2fs_unlink(dir, dentry);
> +		goto out_f2fs_handle_failed_inode;
> +	}
> +
>  	/*
>  	 * Let's flush symlink data in order to avoid broken symlink as much as
>  	 * possible. Nevertheless, fsyncing is the best way, but there is no
> @@ -721,16 +723,10 @@ static int f2fs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  	 * If the symlink path is stored into inline_data, there is no
>  	 * performance regression.
>  	 */
> -	if (!err) {
> -		err = filemap_write_and_wait_range(inode->i_mapping, 0,
> -						   disk_link.len - 1);
> -
> -		if (!err && IS_DIRSYNC(dir))
> -			err = f2fs_sync_fs(sbi->sb, 1);
> -	}
> -
> -	if (err)
> -		f2fs_unlink(dir, dentry);
> +	err = filemap_write_and_wait_range(inode->i_mapping, 0,
> +					   disk_link.len - 1);
> +	if (!err && IS_DIRSYNC(dir))
> +		f2fs_sync_fs(sbi->sb, 1);
>  
>  	f2fs_balance_fs(sbi, true);
>  	goto out_free_encrypted_link;