diff mbox series

[5/5] xfs: xfs_defer_capture should absorb remaining transaction reservation

Message ID 160140142459.830233.7194402837807253154.stgit@magnolia
State Superseded
Headers show
Series xfs: fix how we deal with new intents during recovery | expand

Commit Message

Darrick J. Wong Sept. 29, 2020, 5:43 p.m. UTC
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>
---
 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(-)

Comments

Brian Foster Oct. 1, 2020, 5:32 p.m. UTC | #1
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;
>  
>
Darrick J. Wong Oct. 1, 2020, 5:52 p.m. UTC | #2
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 mbox series

Patch

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;