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@xxxxxxxxxx>
> ---
>  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;