| Message ID | 160140142459.830233.7194402837807253154.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: fix how we deal with new intents during recovery | expand |
On Tue, Sep 29, 2020 at 10:43:44AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > When xfs_defer_capture extracts the deferred ops and transaction state > from a transaction, it should record the transaction reservation type > from the old transaction so that when we continue the dfops chain, we > still use the same reservation parameters. > > Doing this means that the log item recovery functions get to determine > the transaction reservation instead of abusing tr_itruncate in yet > another part of xfs. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Much nicer, and FWIW this is pretty much the approach I was wondering about wrt to the block reservation in the previous patch.. > fs/xfs/libxfs/xfs_defer.c | 9 +++++++++ > fs/xfs/libxfs/xfs_defer.h | 1 + > fs/xfs/xfs_log_recover.c | 4 ++-- > 3 files changed, 12 insertions(+), 2 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 0cceebb390c4..4caaf5527403 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -579,6 +579,15 @@ xfs_defer_ops_capture( > dfc->dfc_blkres = tp->t_blk_res - tp->t_blk_res_used; > tp->t_blk_res = tp->t_blk_res_used; > > + /* > + * Preserve the transaction reservation type. The logcount is > + * hardwired to 1 to so that we can make forward progress in recovery > + * no matter how full the log might be, at a cost of more regrants. > + */ > + dfc->dfc_tres.tr_logres = tp->t_log_res; > + dfc->dfc_tres.tr_logcount = 1; > + dfc->dfc_tres.tr_logflags = XFS_TRANS_PERM_LOG_RES; Any real need to allocate these last two fields in every captured chain when they're basically hardcoded? If not, it might be a bit more efficient to put an xfs_trans_res on the stack in xlog_finish_defer_ops() and just save the logres value here. Brian > + > return dfc; > } > > diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h > index b1c7b761afd5..c447c79bbe74 100644 > --- a/fs/xfs/libxfs/xfs_defer.h > +++ b/fs/xfs/libxfs/xfs_defer.h > @@ -76,6 +76,7 @@ struct xfs_defer_capture { > struct list_head dfc_dfops; > unsigned int dfc_tpflags; > unsigned int dfc_blkres; > + struct xfs_trans_res dfc_tres; > }; > > /* > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index b06c9881a13d..46e750279634 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2442,8 +2442,8 @@ xlog_finish_defer_ops( > int error = 0; > > list_for_each_entry_safe(dfc, next, capture_list, dfc_list) { > - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, > - 0, XFS_TRANS_RESERVE, &tp); > + error = xfs_trans_alloc(mp, &dfc->dfc_tres, 0, 0, > + XFS_TRANS_RESERVE, &tp); > if (error) > return error; > >
On Thu, Oct 01, 2020 at 01:32:56PM -0400, Brian Foster wrote: > On Tue, Sep 29, 2020 at 10:43:44AM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > When xfs_defer_capture extracts the deferred ops and transaction state > > from a transaction, it should record the transaction reservation type > > from the old transaction so that when we continue the dfops chain, we > > still use the same reservation parameters. > > > > Doing this means that the log item recovery functions get to determine > > the transaction reservation instead of abusing tr_itruncate in yet > > another part of xfs. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > Much nicer, and FWIW this is pretty much the approach I was wondering > about wrt to the block reservation in the previous patch.. > > > fs/xfs/libxfs/xfs_defer.c | 9 +++++++++ > > fs/xfs/libxfs/xfs_defer.h | 1 + > > fs/xfs/xfs_log_recover.c | 4 ++-- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > > index 0cceebb390c4..4caaf5527403 100644 > > --- a/fs/xfs/libxfs/xfs_defer.c > > +++ b/fs/xfs/libxfs/xfs_defer.c > > @@ -579,6 +579,15 @@ xfs_defer_ops_capture( > > dfc->dfc_blkres = tp->t_blk_res - tp->t_blk_res_used; > > tp->t_blk_res = tp->t_blk_res_used; > > > > + /* > > + * Preserve the transaction reservation type. The logcount is > > + * hardwired to 1 to so that we can make forward progress in recovery > > + * no matter how full the log might be, at a cost of more regrants. > > + */ > > + dfc->dfc_tres.tr_logres = tp->t_log_res; > > + dfc->dfc_tres.tr_logcount = 1; > > + dfc->dfc_tres.tr_logflags = XFS_TRANS_PERM_LOG_RES; > > Any real need to allocate these last two fields in every captured chain > when they're basically hardcoded? If not, it might be a bit more > efficient to put an xfs_trans_res on the stack in > xlog_finish_defer_ops() and just save the logres value here. Ok, will do. --D > > Brian > > > + > > return dfc; > > } > > > > diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h > > index b1c7b761afd5..c447c79bbe74 100644 > > --- a/fs/xfs/libxfs/xfs_defer.h > > +++ b/fs/xfs/libxfs/xfs_defer.h > > @@ -76,6 +76,7 @@ struct xfs_defer_capture { > > struct list_head dfc_dfops; > > unsigned int dfc_tpflags; > > unsigned int dfc_blkres; > > + struct xfs_trans_res dfc_tres; > > }; > > > > /* > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > > index b06c9881a13d..46e750279634 100644 > > --- a/fs/xfs/xfs_log_recover.c > > +++ b/fs/xfs/xfs_log_recover.c > > @@ -2442,8 +2442,8 @@ xlog_finish_defer_ops( > > int error = 0; > > > > list_for_each_entry_safe(dfc, next, capture_list, dfc_list) { > > - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, > > - 0, XFS_TRANS_RESERVE, &tp); > > + error = xfs_trans_alloc(mp, &dfc->dfc_tres, 0, 0, > > + XFS_TRANS_RESERVE, &tp); > > if (error) > > return error; > > > > >
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c index 0cceebb390c4..4caaf5527403 100644 --- a/fs/xfs/libxfs/xfs_defer.c +++ b/fs/xfs/libxfs/xfs_defer.c @@ -579,6 +579,15 @@ xfs_defer_ops_capture( dfc->dfc_blkres = tp->t_blk_res - tp->t_blk_res_used; tp->t_blk_res = tp->t_blk_res_used; + /* + * Preserve the transaction reservation type. The logcount is + * hardwired to 1 to so that we can make forward progress in recovery + * no matter how full the log might be, at a cost of more regrants. + */ + dfc->dfc_tres.tr_logres = tp->t_log_res; + dfc->dfc_tres.tr_logcount = 1; + dfc->dfc_tres.tr_logflags = XFS_TRANS_PERM_LOG_RES; + return dfc; } diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h index b1c7b761afd5..c447c79bbe74 100644 --- a/fs/xfs/libxfs/xfs_defer.h +++ b/fs/xfs/libxfs/xfs_defer.h @@ -76,6 +76,7 @@ struct xfs_defer_capture { struct list_head dfc_dfops; unsigned int dfc_tpflags; unsigned int dfc_blkres; + struct xfs_trans_res dfc_tres; }; /* diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index b06c9881a13d..46e750279634 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2442,8 +2442,8 @@ xlog_finish_defer_ops( int error = 0; list_for_each_entry_safe(dfc, next, capture_list, dfc_list) { - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, - 0, XFS_TRANS_RESERVE, &tp); + error = xfs_trans_alloc(mp, &dfc->dfc_tres, 0, 0, + XFS_TRANS_RESERVE, &tp); if (error) return error;