On Tue, May 11, 2021 at 06:52:44PM -0700, Darrick J. Wong wrote:
> > is unpinned if the associated item has been aborted and will require
> > a simulated I/O failure. The hold is already required for the
> > simulated I/O failure, so the ordering simply guarantees the unpin
> > handler access to the buffer before it is unpinned and thus
> > processed by the AIL. This particular ordering is required so long
> > as the AIL does not acquire a reference on the bli, which is the
> > long term solution to this problem.
>
> Are you working on that too, or are we just going to let that lie for
> the time being? :)
Wouldn't that be as simple as something like the untested patch below?
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index fb69879e4b2b..07e08713ecd4 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -471,6 +471,7 @@ xfs_buf_item_pin(
trace_xfs_buf_item_pin(bip);
atomic_inc(&bip->bli_refcount);
+ xfs_buf_hold(bip->bli_buf);
atomic_inc(&bip->bli_buf->b_pin_count);
}
@@ -552,14 +553,15 @@ xfs_buf_item_unpin(
xfs_buf_relse(bp);
} else if (freed && remove) {
/*
- * The buffer must be locked and held by the caller to simulate
- * an async I/O failure.
+ * The buffer must be locked to simulate an async I/O failure.
+ * xfs_buf_ioend_fail will drop our buffer reference.
*/
xfs_buf_lock(bp);
- xfs_buf_hold(bp);
bp->b_flags |= XBF_ASYNC;
xfs_buf_ioend_fail(bp);
+ return;
}
+ xfs_buf_rele(bp);
}
STATIC uint
