Skip to content

Commit

Permalink
xfs: gut error handling in xfs_trans_unreserve_and_mod_sb()
Browse files Browse the repository at this point in the history 
xfs: gut error handling in xfs_trans_unreserve_and_mod_sb()

From: Dave Chinner <[email protected]>

The error handling in xfs_trans_unreserve_and_mod_sb() is largely
incorrect - rolling back the changes in the transaction if only one
counter underruns makes all the other counters incorrect. We still
allow the change to proceed and committing the transaction, except
now we have multiple incorrect counters instead of a single
underflow.

Further, we don't actually report the error to the caller, so this
is completely silent except on debug kernels that will assert on
failure before we even get to the rollback code.  Hence this error
handling is broken, untested, and largely unnecessary complexity.

Just remove it.

Signed-off-by: Dave Chinner <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
Signed-off-by: Darrick J. Wong <[email protected]>
  • Loading branch information
@dchinner @djwong
dchinner authored and djwong committed May 27, 2020
1 parent ef83851 commit dc3ffbb
Showing 1 changed file with 20 additions and 143 deletions.
163 changes: 20 additions & 143 deletions fs/xfs/xfs_trans.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,57 +534,9 @@ xfs_trans_apply_sb_deltas(
sizeof(sbp->sb_frextents) - 1);
}

STATIC int
xfs_sb_mod8(
uint8_t *field,
int8_t delta)
{
int8_t counter = *field;

counter += delta;
if (counter < 0) {
ASSERT(0);
return -EINVAL;
}
*field = counter;
return 0;
}

STATIC int
xfs_sb_mod32(
uint32_t *field,
int32_t delta)
{
int32_t counter = *field;

counter += delta;
if (counter < 0) {
ASSERT(0);
return -EINVAL;
}
*field = counter;
return 0;
}

STATIC int
xfs_sb_mod64(
uint64_t *field,
int64_t delta)
{
int64_t counter = *field;

counter += delta;
if (counter < 0) {
ASSERT(0);
return -EINVAL;
}
*field = counter;
return 0;
}

/*
* xfs_trans_unreserve_and_mod_sb() is called to release unused reservations
* and apply superblock counter changes to the in-core superblock. The
* xfs_trans_unreserve_and_mod_sb() is called to release unused reservations and
* apply superblock counter changes to the in-core superblock. The
* t_res_fdblocks_delta and t_res_frextents_delta fields are explicitly NOT
* applied to the in-core superblock. The idea is that that has already been
* done.
Expand Down Expand Up @@ -629,116 +581,41 @@ xfs_trans_unreserve_and_mod_sb(
/* apply the per-cpu counters */
if (blkdelta) {
error = xfs_mod_fdblocks(mp, blkdelta, rsvd);
if (error)
goto out;
ASSERT(!error);
}

if (idelta) {
error = xfs_mod_icount(mp, idelta);
if (error)
goto out_undo_fdblocks;
ASSERT(!error);
}

if (ifreedelta) {
error = xfs_mod_ifree(mp, ifreedelta);
if (error)
goto out_undo_icount;
ASSERT(!error);
}

if (rtxdelta == 0 && !(tp->t_flags & XFS_TRANS_SB_DIRTY))
return;

/* apply remaining deltas */
spin_lock(&mp->m_sb_lock);
if (rtxdelta) {
error = xfs_sb_mod64(&mp->m_sb.sb_frextents, rtxdelta);
if (error)
goto out_undo_ifree;
}

if (tp->t_dblocks_delta != 0) {
error = xfs_sb_mod64(&mp->m_sb.sb_dblocks, tp->t_dblocks_delta);
if (error)
goto out_undo_frextents;
}
if (tp->t_agcount_delta != 0) {
error = xfs_sb_mod32(&mp->m_sb.sb_agcount, tp->t_agcount_delta);
if (error)
goto out_undo_dblocks;
}
if (tp->t_imaxpct_delta != 0) {
error = xfs_sb_mod8(&mp->m_sb.sb_imax_pct, tp->t_imaxpct_delta);
if (error)
goto out_undo_agcount;
}
if (tp->t_rextsize_delta != 0) {
error = xfs_sb_mod32(&mp->m_sb.sb_rextsize,
tp->t_rextsize_delta);
if (error)
goto out_undo_imaxpct;
}
if (tp->t_rbmblocks_delta != 0) {
error = xfs_sb_mod32(&mp->m_sb.sb_rbmblocks,
tp->t_rbmblocks_delta);
if (error)
goto out_undo_rextsize;
}
if (tp->t_rblocks_delta != 0) {
error = xfs_sb_mod64(&mp->m_sb.sb_rblocks, tp->t_rblocks_delta);
if (error)
goto out_undo_rbmblocks;
}
if (tp->t_rextents_delta != 0) {
error = xfs_sb_mod64(&mp->m_sb.sb_rextents,
tp->t_rextents_delta);
if (error)
goto out_undo_rblocks;
}
if (tp->t_rextslog_delta != 0) {
error = xfs_sb_mod8(&mp->m_sb.sb_rextslog,
tp->t_rextslog_delta);
if (error)
goto out_undo_rextents;
}
mp->m_sb.sb_frextents += rtxdelta;
mp->m_sb.sb_dblocks += tp->t_dblocks_delta;
mp->m_sb.sb_agcount += tp->t_agcount_delta;
mp->m_sb.sb_imax_pct += tp->t_imaxpct_delta;
mp->m_sb.sb_rextsize += tp->t_rextsize_delta;
mp->m_sb.sb_rbmblocks += tp->t_rbmblocks_delta;
mp->m_sb.sb_rblocks += tp->t_rblocks_delta;
mp->m_sb.sb_rextents += tp->t_rextents_delta;
mp->m_sb.sb_rextslog += tp->t_rextslog_delta;
spin_unlock(&mp->m_sb_lock);
return;

out_undo_rextents:
if (tp->t_rextents_delta)
xfs_sb_mod64(&mp->m_sb.sb_rextents, -tp->t_rextents_delta);
out_undo_rblocks:
if (tp->t_rblocks_delta)
xfs_sb_mod64(&mp->m_sb.sb_rblocks, -tp->t_rblocks_delta);
out_undo_rbmblocks:
if (tp->t_rbmblocks_delta)
xfs_sb_mod32(&mp->m_sb.sb_rbmblocks, -tp->t_rbmblocks_delta);
out_undo_rextsize:
if (tp->t_rextsize_delta)
xfs_sb_mod32(&mp->m_sb.sb_rextsize, -tp->t_rextsize_delta);
out_undo_imaxpct:
if (tp->t_rextsize_delta)
xfs_sb_mod8(&mp->m_sb.sb_imax_pct, -tp->t_imaxpct_delta);
out_undo_agcount:
if (tp->t_agcount_delta)
xfs_sb_mod32(&mp->m_sb.sb_agcount, -tp->t_agcount_delta);
out_undo_dblocks:
if (tp->t_dblocks_delta)
xfs_sb_mod64(&mp->m_sb.sb_dblocks, -tp->t_dblocks_delta);
out_undo_frextents:
if (rtxdelta)
xfs_sb_mod64(&mp->m_sb.sb_frextents, -rtxdelta);
out_undo_ifree:
spin_unlock(&mp->m_sb_lock);
if (ifreedelta)
xfs_mod_ifree(mp, -ifreedelta);
out_undo_icount:
if (idelta)
xfs_mod_icount(mp, -idelta);
out_undo_fdblocks:
if (blkdelta)
xfs_mod_fdblocks(mp, -blkdelta, rsvd);
out:
ASSERT(error == 0);
/*
* Debug checks outside of the spinlock so they don't lock up the
* machine if they fail.
*/
ASSERT(mp->m_sb.sb_imax_pct >= 0);
ASSERT(mp->m_sb.sb_rextslog >= 0);
return;
}

Expand Down

0 comments on commit dc3ffbb

Please sign in to comment.