[PATCH v2 0/2] xfs: fix CoW fork repair error handling

Yingjie Gao posted 2 patches 1 week, 5 days ago
There is a newer version of this series
fs/xfs/scrub/cow_repair.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
[PATCH v2 0/2] xfs: fix CoW fork repair error handling
Posted by Yingjie Gao 1 week, 5 days ago
Fix two error handling paths in CoW fork repair.

Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
It also lets a force-rebuild bitmap error fall through to the cleanup
labels instead of returning directly.

Patch 2 applies the same fall-through pattern to the realtime helper
so that scrub rtgroup state is released before returning.

Changes since v1:
  - Let the force-rebuild error paths fall through instead of adding
    gotos, as Carlos pointed out.

v1:
  https://lore.kernel.org/linux-xfs/20260526123243.1078867-1-gaoyingjie@uniontech.com/

Yingjie Gao (2):
  xfs: fix error returns in CoW fork repair
  xfs: fix rtgroup cleanup in CoW fork repair

 fs/xfs/scrub/cow_repair.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
2.20.1
[PATCH v3 0/2] xfs: fix CoW fork repair error handling
Posted by Yingjie Gao 1 week, 5 days ago
Fix two error handling paths in CoW fork repair.

Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
It also lets a force-rebuild bitmap error fall through to the cleanup
labels instead of returning directly.

Patch 2 applies the same fall-through pattern to the realtime helper
so that scrub rtgroup state is released before returning.

Changes since v2:
  - Drop the braces around the single-statement force-rebuild if bodies
    in both patches.

v2:
  https://lore.kernel.org/linux-xfs/20260527033902.1524007-1-gaoyingjie@uniontech.com/

Changes since v1:
  - Let the force-rebuild error paths fall through instead of adding
    gotos, as Carlos pointed out.

v1:
  https://lore.kernel.org/linux-xfs/20260526123243.1078867-1-gaoyingjie@uniontech.com/

Yingjie Gao (2):
  xfs: fix error returns in CoW fork repair
  xfs: fix rtgroup cleanup in CoW fork repair

 fs/xfs/scrub/cow_repair.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

-- 
2.20.1
Re: [PATCH v3 0/2] xfs: fix CoW fork repair error handling
Posted by Carlos Maiolino 1 week, 2 days ago
On Wed, 27 May 2026 12:31:32 +0800, Yingjie Gao wrote:
> Fix two error handling paths in CoW fork repair.
> 
> Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
> It also lets a force-rebuild bitmap error fall through to the cleanup
> labels instead of returning directly.
> 
> Patch 2 applies the same fall-through pattern to the realtime helper
> so that scrub rtgroup state is released before returning.
> 
> [...]

Applied to for-next, thanks!

[1/2] xfs: fix error returns in CoW fork repair
      commit: fcf4faba9f986b3bb528da11913c9ec5d6e8f689
[2/2] xfs: fix rtgroup cleanup in CoW fork repair
      commit: c3e073894379532c00cca7ba5762e18fafe29da8

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>
[PATCH v3 1/2] xfs: fix error returns in CoW fork repair
Posted by Yingjie Gao 1 week, 5 days ago
xrep_cow_find_bad() returns success after the cleanup labels even if
AG setup, btree queries, or bitmap updates failed. This can make
repair continue with an incomplete bad-file-offset bitmap instead of
stopping at the original error.

The force-rebuild path has a related cleanup problem. If
xrep_cow_mark_file_range() fails, the function returns directly and
skips the scrub AG context and perag cleanup.

Let the force-rebuild path fall through to the existing cleanup code
and return the saved error after cleanup.

Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index bffc4666ce60..a6ff09ace43d 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -300,18 +300,15 @@ xrep_cow_find_bad(
 	 * on the debugging knob, replace everything in the CoW fork.
 	 */
 	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
-	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
+	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			return error;
-	}
 
 out_sa:
 	xchk_ag_free(sc, &sc->sa);
 out_pag:
 	xfs_perag_put(pag);
-	return 0;
+	return error;
 }
 
 /*
-- 
2.20.1
Re: [PATCH v3 1/2] xfs: fix error returns in CoW fork repair
Posted by Darrick J. Wong 1 week, 4 days ago
On Wed, May 27, 2026 at 12:31:33PM +0800, Yingjie Gao wrote:
> xrep_cow_find_bad() returns success after the cleanup labels even if
> AG setup, btree queries, or bitmap updates failed. This can make
> repair continue with an incomplete bad-file-offset bitmap instead of
> stopping at the original error.
> 
> The force-rebuild path has a related cleanup problem. If
> xrep_cow_mark_file_range() fails, the function returns directly and
> skips the scrub AG context and perag cleanup.
> 
> Let the force-rebuild path fall through to the existing cleanup code
> and return the saved error after cleanup.
> 

Cc: <stable@vger.kernel.org> # v6.8

(please make life easier for Theodorics A and B who are half-assing LTS
QA these days)

> Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> ---
>  fs/xfs/scrub/cow_repair.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> index bffc4666ce60..a6ff09ace43d 100644
> --- a/fs/xfs/scrub/cow_repair.c
> +++ b/fs/xfs/scrub/cow_repair.c
> @@ -300,18 +300,15 @@ xrep_cow_find_bad(
>  	 * on the debugging knob, replace everything in the CoW fork.
>  	 */
>  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
>  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
>  				xc->irec.br_blockcount);
> -		if (error)
> -			return error;

I personally would have stuck with the v1 logic of turning that into a
"goto out_sa" but either way is correct.

With the cc tag added,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> -	}
>  
>  out_sa:
>  	xchk_ag_free(sc, &sc->sa);
>  out_pag:
>  	xfs_perag_put(pag);
> -	return 0;
> +	return error;
>  }
>  
>  /*
> -- 
> 2.20.1
> 
>
Re: [PATCH v3 1/2] xfs: fix error returns in CoW fork repair
Posted by Carlos Maiolino 1 week, 4 days ago
On Wed, May 27, 2026 at 09:44:42PM -0700, Darrick J. Wong wrote:
> On Wed, May 27, 2026 at 12:31:33PM +0800, Yingjie Gao wrote:
> > xrep_cow_find_bad() returns success after the cleanup labels even if
> > AG setup, btree queries, or bitmap updates failed. This can make
> > repair continue with an incomplete bad-file-offset bitmap instead of
> > stopping at the original error.
> > 
> > The force-rebuild path has a related cleanup problem. If
> > xrep_cow_mark_file_range() fails, the function returns directly and
> > skips the scrub AG context and perag cleanup.
> > 
> > Let the force-rebuild path fall through to the existing cleanup code
> > and return the saved error after cleanup.
> > 
> 
> Cc: <stable@vger.kernel.org> # v6.8
> 
> (please make life easier for Theodorics A and B who are half-assing LTS
> QA these days)

I can add this at commit time

> 
> > Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
> > Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> > ---
> >  fs/xfs/scrub/cow_repair.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> > index bffc4666ce60..a6ff09ace43d 100644
> > --- a/fs/xfs/scrub/cow_repair.c
> > +++ b/fs/xfs/scrub/cow_repair.c
> > @@ -300,18 +300,15 @@ xrep_cow_find_bad(
> >  	 * on the debugging knob, replace everything in the CoW fork.
> >  	 */
> >  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> > -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> > +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
> >  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
> >  				xc->irec.br_blockcount);
> > -		if (error)
> > -			return error;
> 
> I personally would have stuck with the v1 logic of turning that into a
> "goto out_sa" but either way is correct.
> 
> With the cc tag added,
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
> 
> > -	}
> >  
> >  out_sa:
> >  	xchk_ag_free(sc, &sc->sa);
> >  out_pag:
> >  	xfs_perag_put(pag);
> > -	return 0;
> > +	return error;
> >  }
> >  
> >  /*
> > -- 
> > 2.20.1
> > 
> > 
>
[PATCH v3 2/2] xfs: fix rtgroup cleanup in CoW fork repair
Posted by Yingjie Gao 1 week, 5 days ago
xrep_cow_find_bad_rt() initializes scrub rtgroup state before the
force-rebuild path calls xrep_cow_mark_file_range(). If that call
fails, the code jumps directly to out_rtg, which skips the scrub
rtgroup cleanup and only drops the local rtgroup reference.

Remove the unnecessary jump so the function falls through to out_sr,
ensuring the realtime cursors, lock state, and sr->rtg reference are
released before returning.

Fixes: fd97fe111208 ("xfs: fix CoW forks for realtime files")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index a6ff09ace43d..c25716fc4fee 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -382,12 +382,9 @@ xrep_cow_find_bad_rt(
 	 * CoW fork and then scan for staging extents in the refcountbt.
 	 */
 	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
-	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
+	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			goto out_rtg;
-	}
 
 out_sr:
 	xchk_rtgroup_btcur_free(&sc->sr);
-- 
2.20.1
Re: [PATCH v3 2/2] xfs: fix rtgroup cleanup in CoW fork repair
Posted by Darrick J. Wong 1 week, 4 days ago
On Wed, May 27, 2026 at 12:31:34PM +0800, Yingjie Gao wrote:
> xrep_cow_find_bad_rt() initializes scrub rtgroup state before the
> force-rebuild path calls xrep_cow_mark_file_range(). If that call
> fails, the code jumps directly to out_rtg, which skips the scrub
> rtgroup cleanup and only drops the local rtgroup reference.
> 
> Remove the unnecessary jump so the function falls through to out_sr,
> ensuring the realtime cursors, lock state, and sr->rtg reference are
> released before returning.

Cc: <stable@vger.kernel.org> # v6.14

> Fixes: fd97fe111208 ("xfs: fix CoW forks for realtime files")
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>

With the cc tag, this looks good to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/cow_repair.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> index a6ff09ace43d..c25716fc4fee 100644
> --- a/fs/xfs/scrub/cow_repair.c
> +++ b/fs/xfs/scrub/cow_repair.c
> @@ -382,12 +382,9 @@ xrep_cow_find_bad_rt(
>  	 * CoW fork and then scan for staging extents in the refcountbt.
>  	 */
>  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
>  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
>  				xc->irec.br_blockcount);
> -		if (error)
> -			goto out_rtg;
> -	}
>  
>  out_sr:
>  	xchk_rtgroup_btcur_free(&sc->sr);
> -- 
> 2.20.1
> 
>