Re: [PATCH v2] f2fs: return symlink writeback errors

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Aug 10, 2026 at 09:38:32PM +0800, Wenjie Qi wrote:
> F2FS writes long symlink data with page_symlink() and then flushes the
> symlink mapping to reduce the chance of exposing a broken symlink.
> 
> That flush result is currently ignored. If the writeback fails, symlink()
> still returns success even though the symlink is not durable and the same
> operation can already surface -EIO through syncfs().
> 
> Return the writeback error to userspace and skip the dirsync flush once the
> symlink data flush has failed.

>  	if (!err) {
> -		filemap_write_and_wait_range(inode->i_mapping, 0,
> -							disk_link.len - 1);
> +		err = filemap_write_and_wait_range(inode->i_mapping, 0,
> +						   disk_link.len - 1);
>  
> -		if (IS_DIRSYNC(dir))
> +		if (!err && IS_DIRSYNC(dir))
>  			f2fs_sync_fs(sbi->sb, 1);
> -	} else {
> -		f2fs_unlink(dir, dentry);
>  	}
>  
> +	if (err)
> +		f2fs_unlink(dir, dentry);

That looks fishy.  At that point you already have dentry hashed and
AFAICS f2fs_unlink() will leave it hashed and attached to the same
inode; sure, memory pressure will eventually evict the sucker, but
until that point any lookups will simply pick it from dcache.

It's not introduced by this patch; the same issue, AFAICS, already exists
in mainline.  Why do we even bother with d_instantiate_new() before we
know that everything's fine, nevermind doing that when we already know
the operation has failed?

Incidentally, is there any reason to add a directory entry before the
inode is set up?  Usually that's the last step, and cleanup tends to
be simpler that way; are there f2fs-specific reasons to do it in the
unusual order?