On Wed, Sep 20, 2017 at 05:18:20PM -0700, Darrick J. Wong wrote:
> +/* Check for btree operation errors . */
> +bool
> +xfs_scrub_btree_op_ok(
> + struct xfs_scrub_context *sc,
> + struct xfs_btree_cur *cur,
> + int level,
> + int *error)
> +{
> + if (*error == 0)
> + return true;
> +
> + switch (*error) {
> + case -EDEADLOCK:
> + /* Used to restart an op with deadlock avoidance. */
> + trace_xfs_scrub_deadlock_retry(sc->ip, sc->sm, *error);
> + break;
> + case -EFSBADCRC:
> + case -EFSCORRUPTED:
> + /* Note the badness but don't abort. */
> + sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
> + *error = 0;
> + /* fall through */
> + default:
> + if (cur->bc_flags & XFS_BTREE_ROOT_IN_INODE)
> + trace_xfs_scrub_ifork_btree_op_error(sc, cur, level,
> + *error, __return_address);
> + else
> + trace_xfs_scrub_btree_op_error(sc, cur, level,
> + *error, __return_address);
Why different tracepoints when you could just output the
cur->bc_flags in the trace output and use the same tracepoint?
Doing that looks like it would simplify the tracepoint code
in this patch....
> +/* Figure out which block the btree cursor was pointing to. */
> +static inline xfs_fsblock_t
> +xfs_scrub_btree_cur_fsbno(
> + struct xfs_btree_cur *cur,
> + int level)
> +{
> + if (level < cur->bc_nlevels && cur->bc_bufs[level])
> + return XFS_DADDR_TO_FSB(cur->bc_mp, cur->bc_bufs[level]->b_bn);
> + else if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
no need for "else if", just "if" will do.
> + return XFS_INO_TO_FSB(cur->bc_mp, cur->bc_private.b.ip->i_ino);
This makes no sense to me. Why are we returning the block address of
the inode here? It's not part of the btree....
> + return XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_private.a.agno, 0);
Nor is the first block of the AG....
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
