| Message ID | 20190425055000.GH178290@magnolia (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/2] xfs: always rejoin held resources during defer roll | expand |
On Wed, Apr 24, 2019 at 10:50:01PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In xrep_roll_ag_trans, the transaction roll will always set sc->tp to > the new transaction, even if committing the old one fails. A bare > transaction roll leaves the buffer(s) locked but not joined to the new > transaction, so it's not necessary to release the hold if the roll > fails. Remove the incorrect xfs_trans_bhold_release calls. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Makes sense: Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/scrub/repair.c | 25 ++++++++----------------- > 1 file changed, 8 insertions(+), 17 deletions(-) > > diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c > index 5e7e36cdf3d5..eb358f0f5e0a 100644 > --- a/fs/xfs/scrub/repair.c > +++ b/fs/xfs/scrub/repair.c > @@ -136,10 +136,16 @@ xrep_roll_ag_trans( > if (sc->sa.agfl_bp) > xfs_trans_bhold(sc->tp, sc->sa.agfl_bp); > > - /* Roll the transaction. */ > + /* > + * Roll the transaction. We still own the buffer and the buffer lock > + * regardless of whether or not the roll succeeds. If the roll fails, > + * the buffers will be released during teardown on our way out of the > + * kernel. If it succeeds, we join them to the new transaction and > + * move on. > + */ > error = xfs_trans_roll(&sc->tp); > if (error) > - goto out_release; > + return error; > > /* Join AG headers to the new transaction. */ > if (sc->sa.agi_bp) > @@ -150,21 +156,6 @@ xrep_roll_ag_trans( > xfs_trans_bjoin(sc->tp, sc->sa.agfl_bp); > > return 0; > - > -out_release: > - /* > - * Rolling failed, so release the hold on the buffers. The > - * buffers will be released during teardown on our way out > - * of the kernel. > - */ > - if (sc->sa.agi_bp) > - xfs_trans_bhold_release(sc->tp, sc->sa.agi_bp); > - if (sc->sa.agf_bp) > - xfs_trans_bhold_release(sc->tp, sc->sa.agf_bp); > - if (sc->sa.agfl_bp) > - xfs_trans_bhold_release(sc->tp, sc->sa.agfl_bp); > - > - return error; > } > > /*
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c index 5e7e36cdf3d5..eb358f0f5e0a 100644 --- a/fs/xfs/scrub/repair.c +++ b/fs/xfs/scrub/repair.c @@ -136,10 +136,16 @@ xrep_roll_ag_trans( if (sc->sa.agfl_bp) xfs_trans_bhold(sc->tp, sc->sa.agfl_bp); - /* Roll the transaction. */ + /* + * Roll the transaction. We still own the buffer and the buffer lock + * regardless of whether or not the roll succeeds. If the roll fails, + * the buffers will be released during teardown on our way out of the + * kernel. If it succeeds, we join them to the new transaction and + * move on. + */ error = xfs_trans_roll(&sc->tp); if (error) - goto out_release; + return error; /* Join AG headers to the new transaction. */ if (sc->sa.agi_bp) @@ -150,21 +156,6 @@ xrep_roll_ag_trans( xfs_trans_bjoin(sc->tp, sc->sa.agfl_bp); return 0; - -out_release: - /* - * Rolling failed, so release the hold on the buffers. The - * buffers will be released during teardown on our way out - * of the kernel. - */ - if (sc->sa.agi_bp) - xfs_trans_bhold_release(sc->tp, sc->sa.agi_bp); - if (sc->sa.agf_bp) - xfs_trans_bhold_release(sc->tp, sc->sa.agf_bp); - if (sc->sa.agfl_bp) - xfs_trans_bhold_release(sc->tp, sc->sa.agfl_bp); - - return error; } /*