| Message ID | 20220216013713.1191082-3-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Log Attribute Replay | expand |
On Tue, Feb 15, 2022 at 06:37:00PM -0700, Allison Henderson wrote: > If the first operation in a string of defer ops has no intents, > then there is no reason to commit it before running the first call > to xfs_defer_finish_one(). This allows the defer ops to be used > effectively for non-intent based operations without requiring an > unnecessary extra transaction commit when first called. > > This fixes a regression in per-attribute modification transaction > count when delayed attributes are not being used. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/libxfs/xfs_defer.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 6dac8d6b8c21..26680e9f50f5 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -510,9 +510,16 @@ xfs_defer_finish_noroll( > xfs_defer_create_intents(*tp); > list_splice_init(&(*tp)->t_dfops, &dop_pending); > > - error = xfs_defer_trans_roll(tp); > - if (error) > - goto out_shutdown; > + /* > + * We must ensure the transaction is clean before we try to finish > + * deferred work by committing logged intent items and anything > + * else that dirtied the transaction. Hm. I think the comment needs a slight tweak: "...before we try to finish the next deferred work item by committing..." With that fixed, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > + */ > + if ((*tp)->t_flags & XFS_TRANS_DIRTY) { > + error = xfs_defer_trans_roll(tp); > + if (error) > + goto out_shutdown; > + } > > /* Possibly relog intent items to keep the log moving. */ > error = xfs_defer_relog(tp, &dop_pending); > -- > 2.25.1 >
On 16 Feb 2022 at 07:07, Allison Henderson wrote: > If the first operation in a string of defer ops has no intents, > then there is no reason to commit it before running the first call > to xfs_defer_finish_one(). This allows the defer ops to be used > effectively for non-intent based operations without requiring an > unnecessary extra transaction commit when first called. > > This fixes a regression in per-attribute modification transaction > count when delayed attributes are not being used. > Looks good to me. Reviewed-by: Chandan Babu R <chandan.babu@oracle.com> > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/libxfs/xfs_defer.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 6dac8d6b8c21..26680e9f50f5 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -510,9 +510,16 @@ xfs_defer_finish_noroll( > xfs_defer_create_intents(*tp); > list_splice_init(&(*tp)->t_dfops, &dop_pending); > > - error = xfs_defer_trans_roll(tp); > - if (error) > - goto out_shutdown; > + /* > + * We must ensure the transaction is clean before we try to finish > + * deferred work by committing logged intent items and anything > + * else that dirtied the transaction. > + */ > + if ((*tp)->t_flags & XFS_TRANS_DIRTY) { > + error = xfs_defer_trans_roll(tp); > + if (error) > + goto out_shutdown; > + } > > /* Possibly relog intent items to keep the log moving. */ > error = xfs_defer_relog(tp, &dop_pending);
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c index 6dac8d6b8c21..26680e9f50f5 100644 --- a/fs/xfs/libxfs/xfs_defer.c +++ b/fs/xfs/libxfs/xfs_defer.c @@ -510,9 +510,16 @@ xfs_defer_finish_noroll( xfs_defer_create_intents(*tp); list_splice_init(&(*tp)->t_dfops, &dop_pending); - error = xfs_defer_trans_roll(tp); - if (error) - goto out_shutdown; + /* + * We must ensure the transaction is clean before we try to finish + * deferred work by committing logged intent items and anything + * else that dirtied the transaction. + */ + if ((*tp)->t_flags & XFS_TRANS_DIRTY) { + error = xfs_defer_trans_roll(tp); + if (error) + goto out_shutdown; + } /* Possibly relog intent items to keep the log moving. */ error = xfs_defer_relog(tp, &dop_pending);