On Fri, Dec 22, 2017 at 04:44:24PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Scrub should make sure that each bnobt record has a corresponding
> cntbt record.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/scrub/agheader.c | 26 ++++++++++++++++++++++++++
> fs/xfs/scrub/alloc.c | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 63 insertions(+)
>
>
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index 3bb0f96..e87022f 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -444,6 +444,7 @@ xfs_scrub_agf_xref(
> struct xfs_btree_cur **pcur;
> xfs_agblock_t bno;
> xfs_extlen_t blocks;
> + int have;
> int error;
>
> bno = XFS_AGF_BLOCK(mp);
> @@ -465,6 +466,31 @@ xfs_scrub_agf_xref(
> xfs_scrub_block_xref_set_corrupt(sc, sc->sa.agf_bp);
> }
>
> + /* Cross-reference with the cntbt. */
> + pcur = &sc->sa.cnt_cur;
> + while (*pcur) {
> + xfs_agblock_t agbno;
> +
> + /* Any freespace at all? */
> + error = xfs_alloc_lookup_le(*pcur, 0, -1U, &have);
> + if (!xfs_scrub_should_xref(sc, &error, pcur))
> + break;
> + if (!have) {
> + if (agf->agf_freeblks != be32_to_cpu(0))
No need to endian convert 0.
> + xfs_scrub_block_xref_set_corrupt(sc,
> + sc->sa.agf_bp);
> + break;
> + }
> +
> + /* Check agf_longest */
> + error = xfs_alloc_get_rec(*pcur, &agbno, &blocks, &have);
> + if (!xfs_scrub_should_xref(sc, &error, pcur))
> + break;
> + if (!have || blocks != be32_to_cpu(agf->agf_longest))
> + xfs_scrub_block_xref_set_corrupt(sc, sc->sa.agf_bp);
> + break;
> + }
Why is this a while loop? It'll break out at the end of the first
loop - did you forget to remove that last break statement?
> +
> /* scrub teardown will take care of sc->sa for us */
> }
>
> diff --git a/fs/xfs/scrub/alloc.c b/fs/xfs/scrub/alloc.c
> index 3d6f8cc..9a28e3d 100644
> --- a/fs/xfs/scrub/alloc.c
> +++ b/fs/xfs/scrub/alloc.c
> @@ -31,6 +31,7 @@
> #include "xfs_sb.h"
> #include "xfs_alloc.h"
> #include "xfs_rmap.h"
> +#include "xfs_alloc.h"
> #include "scrub/xfs_scrub.h"
> #include "scrub/scrub.h"
> #include "scrub/common.h"
> @@ -57,6 +58,42 @@ xfs_scrub_allocbt_xref(
> xfs_agblock_t bno,
> xfs_extlen_t len)
> {
> + struct xfs_btree_cur **pcur;
> + struct xfs_scrub_ag *psa = &sc->sa;
> + xfs_agblock_t fbno;
> + xfs_extlen_t flen;
> + int has_otherrec;
> + int error;
> +
> + /*
> + * Ensure there's a corresponding cntbt/bnobt record matching
> + * this bnobt/cntbt record, respectively.
> + */
> + if (sc->sm->sm_type == XFS_SCRUB_TYPE_BNOBT)
> + pcur = &psa->cnt_cur;
> + else
> + pcur = &psa->bno_cur;
> + while (*pcur) {
> + error = xfs_alloc_lookup_le(*pcur, bno, len, &has_otherrec);
> + if (!xfs_scrub_should_xref(sc, &error, pcur))
> + break;
> + if (!has_otherrec) {
> + xfs_scrub_btree_xref_set_corrupt(sc, *pcur, 0);
> + break;
> + }
> +
> + error = xfs_alloc_get_rec(*pcur, &fbno, &flen, &has_otherrec);
> + if (!xfs_scrub_should_xref(sc, &error, pcur))
> + break;
> + if (!has_otherrec) {
> + xfs_scrub_btree_xref_set_corrupt(sc, *pcur, 0);
> + break;
> + }
> +
> + if (fbno != bno || flen != len)
> + xfs_scrub_btree_xref_set_corrupt(sc, *pcur, 0);
> + break;
> + }
Same again - I don't see why this is a while loop.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
