| Message ID | 169577059164.3312911.8148982456892861553.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: reserve disk space for online repairs | expand |
On Tue, Sep 26, 2023 at 04:31:22PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > When someone tries to add a deferred work item to xfs_defer_add, it will > try to attach the work item to the most recently added xfs_defer_pending > object attached to the transaction. However, it doesn't check if the > pending object has a log intent item attached to it. This is incorrect > behavior because we cannot add more work to an object that has already > been committed to the ondisk log. > > Therefore, change the behavior not to append to pending items with a non > null dfp_intent. In practice this has not been an issue because the > only way xfs_defer_add gets called after log intent items have been > committed is from the defer ops ->finish_item functions themselves, and > the @dop_pending isolation in xfs_defer_finish_noroll protects the > pending items that have already been logged. > > However, the next patch will add the ability to pause a deferred extent > free object during online btree rebuilding, and any new extfree work > items need to have their own pending event. > > While we're at it, hoist the predicate to its own static inline function > for readability. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_defer.c | 48 ++++++++++++++++++++++++++++++++++----------- > 1 file changed, 36 insertions(+), 12 deletions(-) Looks fine. Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c index bcfb6a4203cdd..ad41c6d0113ce 100644 --- a/fs/xfs/libxfs/xfs_defer.c +++ b/fs/xfs/libxfs/xfs_defer.c @@ -617,6 +617,40 @@ xfs_defer_cancel( xfs_defer_cancel_list(mp, &tp->t_dfops); } +/* + * Decide if we can add a deferred work item to the last dfops item attached + * to the transaction. + */ +static inline struct xfs_defer_pending * +xfs_defer_try_append( + struct xfs_trans *tp, + enum xfs_defer_ops_type type, + const struct xfs_defer_op_type *ops) +{ + struct xfs_defer_pending *dfp = NULL; + + /* No dfops at all? */ + if (list_empty(&tp->t_dfops)) + return NULL; + + dfp = list_last_entry(&tp->t_dfops, struct xfs_defer_pending, + dfp_list); + + /* Wrong type? */ + if (dfp->dfp_type != type) + return NULL; + + /* Already logged? */ + if (dfp->dfp_intent) + return NULL; + + /* Already full? */ + if (ops->max_items && dfp->dfp_count >= ops->max_items) + return NULL; + + return dfp; +} + /* Add an item for later deferred processing. */ void xfs_defer_add( @@ -630,19 +664,9 @@ xfs_defer_add( ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); BUILD_BUG_ON(ARRAY_SIZE(defer_op_types) != XFS_DEFER_OPS_TYPE_MAX); - /* - * Add the item to a pending item at the end of the intake list. - * If the last pending item has the same type, reuse it. Else, - * create a new pending item at the end of the intake list. - */ - if (!list_empty(&tp->t_dfops)) { - dfp = list_last_entry(&tp->t_dfops, - struct xfs_defer_pending, dfp_list); - if (dfp->dfp_type != type || - (ops->max_items && dfp->dfp_count >= ops->max_items)) - dfp = NULL; - } + dfp = xfs_defer_try_append(tp, type, ops); if (!dfp) { + /* Create a new pending item at the end of the intake list. */ dfp = kmem_cache_zalloc(xfs_defer_pending_cache, GFP_NOFS | __GFP_NOFAIL); dfp->dfp_type = type;