In xfs_inactive(), xfs_reflink_cancel_cow_range() is called
without error handling, risking unnoticed failures and
inconsistent behavior compared to other parts of the code.

Fix this issue by adding an error handling for the
xfs_reflink_cancel_cow_range(), improving code robustness.

Fixes: 6231848c3aa5 ("xfs: check for cow blocks before trying to clear them")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 fs/xfs/xfs_inode.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c8ad2606f928..1ff514b6c035 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1404,8 +1404,11 @@ xfs_inactive(
 		goto out;
 
 	/* Try to clean out the cow blocks if there are any. */
-	if (xfs_inode_has_cow_data(ip))
-		xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
+	if (xfs_inode_has_cow_data(ip)) {
+		error = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
+		if (error)
+			goto out;
+	}
 
 	if (VFS_I(ip)->i_nlink != 0) {
 		/*
-- 
2.42.0.windows.2

On Mon, Jan 20, 2025 at 11:46:24PM +0800, Wentao Liang wrote:
> In xfs_inactive(), xfs_reflink_cancel_cow_range() is called
> without error handling, risking unnoticed failures and
> inconsistent behavior compared to other parts of the code.
> 
> Fix this issue by adding an error handling for the
> xfs_reflink_cancel_cow_range(), improving code robustness.
> 
> Fixes: 6231848c3aa5 ("xfs: check for cow blocks before trying to clear them")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  fs/xfs/xfs_inode.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c8ad2606f928..1ff514b6c035 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1404,8 +1404,11 @@ xfs_inactive(
>  		goto out;
>  
>  	/* Try to clean out the cow blocks if there are any. */
> -	if (xfs_inode_has_cow_data(ip))
> -		xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
> +	if (xfs_inode_has_cow_data(ip)) {
> +		error = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
> +		if (error)
> +			goto out;

If memory serves, we ignored the error return here on the grounds that
the worst that can happen is that we leak some cow staging reservations,
so we should continue freeing the file in the ondisk metadata.

That said, I /think/ the error case corresponds to the log being shut
down or going down due to a corruption error during the cow blocks
cancellation.  So I guess this can go ahead with:

Cc: <stable@vger.kernel.org> # v4.17
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +	}
>  
>  	if (VFS_I(ip)->i_nlink != 0) {
>  		/*
> -- 
> 2.42.0.windows.2
>