Commit ea78d808 authored by Darrick J. Wong's avatar Darrick J. Wong Committed by Dave Chinner

xfs: track log done items directly in the deferred pending work item

Christoph reports slab corruption when a deferred refcount update
aborts during _defer_finish().  The cause of this was broken log item
state tracking in xfs_defer_pending -- upon an abort,
_defer_trans_abort() will call abort_intent on all intent items,
including the ones that have already had a done item attached.

This is incorrect because each intent item has 2 refcount: the first
is released when the intent item is committed to the log; and the
second is released when the _done_ item is committed to the log, or
by the intent creator if there is no done item.  In other words, once
we log the done item, responsibility for releasing the intent item's
second refcount is transferred to the done item and /must not/ be
performed by anything else.

The dfp_committed flag should have been tracking whether or not we had
a done item so that _defer_trans_abort could decide if it needs to
abort the intent item, but due to a thinko this was not the case.  Rip
it out and track the done item directly so that we do the right thing
w.r.t. intent item freeing.
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Reported-by: default avatarChristoph Hellwig <hch@infradead.org>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 17de0a9f
...@@ -194,7 +194,7 @@ xfs_defer_trans_abort( ...@@ -194,7 +194,7 @@ xfs_defer_trans_abort(
/* Abort intent items. */ /* Abort intent items. */
list_for_each_entry(dfp, &dop->dop_pending, dfp_list) { list_for_each_entry(dfp, &dop->dop_pending, dfp_list) {
trace_xfs_defer_pending_abort(tp->t_mountp, dfp); trace_xfs_defer_pending_abort(tp->t_mountp, dfp);
if (dfp->dfp_committed) if (!dfp->dfp_done)
dfp->dfp_type->abort_intent(dfp->dfp_intent); dfp->dfp_type->abort_intent(dfp->dfp_intent);
} }
...@@ -290,7 +290,6 @@ xfs_defer_finish( ...@@ -290,7 +290,6 @@ xfs_defer_finish(
struct xfs_defer_pending *dfp; struct xfs_defer_pending *dfp;
struct list_head *li; struct list_head *li;
struct list_head *n; struct list_head *n;
void *done_item = NULL;
void *state; void *state;
int error = 0; int error = 0;
void (*cleanup_fn)(struct xfs_trans *, void *, int); void (*cleanup_fn)(struct xfs_trans *, void *, int);
...@@ -309,19 +308,11 @@ xfs_defer_finish( ...@@ -309,19 +308,11 @@ xfs_defer_finish(
if (error) if (error)
goto out; goto out;
/* Mark all pending intents as committed. */
list_for_each_entry_reverse(dfp, &dop->dop_pending, dfp_list) {
if (dfp->dfp_committed)
break;
trace_xfs_defer_pending_commit((*tp)->t_mountp, dfp);
dfp->dfp_committed = true;
}
/* Log an intent-done item for the first pending item. */ /* Log an intent-done item for the first pending item. */
dfp = list_first_entry(&dop->dop_pending, dfp = list_first_entry(&dop->dop_pending,
struct xfs_defer_pending, dfp_list); struct xfs_defer_pending, dfp_list);
trace_xfs_defer_pending_finish((*tp)->t_mountp, dfp); trace_xfs_defer_pending_finish((*tp)->t_mountp, dfp);
done_item = dfp->dfp_type->create_done(*tp, dfp->dfp_intent, dfp->dfp_done = dfp->dfp_type->create_done(*tp, dfp->dfp_intent,
dfp->dfp_count); dfp->dfp_count);
cleanup_fn = dfp->dfp_type->finish_cleanup; cleanup_fn = dfp->dfp_type->finish_cleanup;
...@@ -331,7 +322,7 @@ xfs_defer_finish( ...@@ -331,7 +322,7 @@ xfs_defer_finish(
list_del(li); list_del(li);
dfp->dfp_count--; dfp->dfp_count--;
error = dfp->dfp_type->finish_item(*tp, dop, li, error = dfp->dfp_type->finish_item(*tp, dop, li,
done_item, &state); dfp->dfp_done, &state);
if (error) { if (error) {
/* /*
* Clean up after ourselves and jump out. * Clean up after ourselves and jump out.
...@@ -428,8 +419,8 @@ xfs_defer_add( ...@@ -428,8 +419,8 @@ xfs_defer_add(
dfp = kmem_alloc(sizeof(struct xfs_defer_pending), dfp = kmem_alloc(sizeof(struct xfs_defer_pending),
KM_SLEEP | KM_NOFS); KM_SLEEP | KM_NOFS);
dfp->dfp_type = defer_op_types[type]; dfp->dfp_type = defer_op_types[type];
dfp->dfp_committed = false;
dfp->dfp_intent = NULL; dfp->dfp_intent = NULL;
dfp->dfp_done = NULL;
dfp->dfp_count = 0; dfp->dfp_count = 0;
INIT_LIST_HEAD(&dfp->dfp_work); INIT_LIST_HEAD(&dfp->dfp_work);
list_add_tail(&dfp->dfp_list, &dop->dop_intake); list_add_tail(&dfp->dfp_list, &dop->dop_intake);
......
...@@ -30,8 +30,8 @@ struct xfs_defer_op_type; ...@@ -30,8 +30,8 @@ struct xfs_defer_op_type;
struct xfs_defer_pending { struct xfs_defer_pending {
const struct xfs_defer_op_type *dfp_type; /* function pointers */ const struct xfs_defer_op_type *dfp_type; /* function pointers */
struct list_head dfp_list; /* pending items */ struct list_head dfp_list; /* pending items */
bool dfp_committed; /* committed trans? */
void *dfp_intent; /* log intent item */ void *dfp_intent; /* log intent item */
void *dfp_done; /* log done item */
struct list_head dfp_work; /* work items */ struct list_head dfp_work; /* work items */
unsigned int dfp_count; /* # extent items */ unsigned int dfp_count; /* # extent items */
}; };
......
...@@ -2295,7 +2295,7 @@ DECLARE_EVENT_CLASS(xfs_defer_pending_class, ...@@ -2295,7 +2295,7 @@ DECLARE_EVENT_CLASS(xfs_defer_pending_class,
__entry->dev = mp ? mp->m_super->s_dev : 0; __entry->dev = mp ? mp->m_super->s_dev : 0;
__entry->type = dfp->dfp_type->type; __entry->type = dfp->dfp_type->type;
__entry->intent = dfp->dfp_intent; __entry->intent = dfp->dfp_intent;
__entry->committed = dfp->dfp_committed; __entry->committed = dfp->dfp_done != NULL;
__entry->nr = dfp->dfp_count; __entry->nr = dfp->dfp_count;
), ),
TP_printk("dev %d:%d optype %d intent %p committed %d nr %d\n", TP_printk("dev %d:%d optype %d intent %p committed %d nr %d\n",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment