Skip to content

Commit

Permalink
xfs: drop all altpath buffers at the end of the sibling check
Browse files Browse the repository at this point in the history
The dirattr btree checking code uses the altpath substructure of the
dirattr state structure to check the sibling pointers of dir/attr tree
blocks.  At the end of sibling checks, xfs_da3_path_shift could have
changed multiple levels of buffer pointers in the altpath structure.
Although we release the leaf level buffer, this isn't enough -- we also
need to release the node buffers that are unique to the altpath.

Not releasing all of the altpath buffers leaves them locked to the
transaction.  This is suboptimal because we should release resources
when we don't need them anymore.  Fix the function to loop all levels of
the altpath, and fix the return logic so that we always run the loop.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
  • Loading branch information
Darrick J. Wong committed Mar 26, 2020
1 parent 5885539 commit afbabf5
Showing 1 changed file with 25 additions and 17 deletions.
42 changes: 25 additions & 17 deletions fs/xfs/scrub/dabtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,47 +219,55 @@ xchk_da_btree_block_check_sibling(
int direction,
xfs_dablk_t sibling)
{
struct xfs_da_state_path *path = &ds->state->path;
struct xfs_da_state_path *altpath = &ds->state->altpath;
int retval;
int plevel;
int error;

memcpy(&ds->state->altpath, &ds->state->path,
sizeof(ds->state->altpath));
memcpy(altpath, path, sizeof(ds->state->altpath));

/*
* If the pointer is null, we shouldn't be able to move the upper
* level pointer anywhere.
*/
if (sibling == 0) {
error = xfs_da3_path_shift(ds->state, &ds->state->altpath,
direction, false, &retval);
error = xfs_da3_path_shift(ds->state, altpath, direction,
false, &retval);
if (error == 0 && retval == 0)
xchk_da_set_corrupt(ds, level);
error = 0;
goto out;
}

/* Move the alternate cursor one block in the direction given. */
error = xfs_da3_path_shift(ds->state, &ds->state->altpath,
direction, false, &retval);
error = xfs_da3_path_shift(ds->state, altpath, direction, false,
&retval);
if (!xchk_da_process_error(ds, level, &error))
return error;
goto out;
if (retval) {
xchk_da_set_corrupt(ds, level);
return error;
goto out;
}
if (ds->state->altpath.blk[level].bp)
xchk_buffer_recheck(ds->sc,
ds->state->altpath.blk[level].bp);
if (altpath->blk[level].bp)
xchk_buffer_recheck(ds->sc, altpath->blk[level].bp);

/* Compare upper level pointer to sibling pointer. */
if (ds->state->altpath.blk[level].blkno != sibling)
if (altpath->blk[level].blkno != sibling)
xchk_da_set_corrupt(ds, level);
if (ds->state->altpath.blk[level].bp) {
xfs_trans_brelse(ds->dargs.trans,
ds->state->altpath.blk[level].bp);
ds->state->altpath.blk[level].bp = NULL;
}

out:
/* Free all buffers in the altpath that aren't referenced from path. */
for (plevel = 0; plevel < altpath->active; plevel++) {
if (altpath->blk[plevel].bp == NULL ||
(plevel < path->active &&
altpath->blk[plevel].bp == path->blk[plevel].bp))
continue;

xfs_trans_brelse(ds->dargs.trans, altpath->blk[plevel].bp);
altpath->blk[plevel].bp = NULL;
}

return error;
}

Expand Down

0 comments on commit afbabf5

Please sign in to comment.