On Mon, Jul 15, 2024 at 07:09:15PM +0200, Christoph Hellwig wrote:
> scan_bmapbt tries to track btree blocks in the RT duplicate extent
> AVL tree if the inode has the realtime flag set. Given that the
> RT subvolume is only ever used for file data this is incorrect.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Did you actually hit this, or did you find it through code inspection?
AFAICT for attr forks, the (whichfork != XFS_DATA_FORK) check should
have been saving us this whole time? And the (type != XR_INO_RTDATA)
check did the job for the data fork?
IOWs, is this merely cleaning out a logic bomb, or is it resolving some
false positive/customer complaint?
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> repair/scan.c | 21 +++++----------------
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/repair/scan.c b/repair/scan.c
> index 338308ef8..8352b3ccf 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
> @@ -390,22 +390,11 @@ _("bad state %d, inode %" PRIu64 " bmap block 0x%" PRIx64 "\n"),
> break;
> }
> pthread_mutex_unlock(&ag_locks[agno].lock);
> - } else {
> - /*
> - * attribute fork for realtime files is in the regular
> - * filesystem
> - */
> - if (type != XR_INO_RTDATA || whichfork != XFS_DATA_FORK) {
> - if (search_dup_extent(XFS_FSB_TO_AGNO(mp, bno),
> - XFS_FSB_TO_AGBNO(mp, bno),
> - XFS_FSB_TO_AGBNO(mp, bno) + 1))
> - return(1);
> - } else {
> - xfs_rtxnum_t ext = xfs_rtb_to_rtx(mp, bno);
> -
> - if (search_rt_dup_extent(mp, ext))
> - return 1;
> - }
> + } else {
> + if (search_dup_extent(XFS_FSB_TO_AGNO(mp, bno),
> + XFS_FSB_TO_AGBNO(mp, bno),
> + XFS_FSB_TO_AGBNO(mp, bno) + 1))
> + return 1;
> }
> (*tot)++;
> numrecs = be16_to_cpu(block->bb_numrecs);
> --
> 2.43.0
>
>
