[PATCH] xfs: buffer ->bi_end_io function requires irq-safe lock

[Brian Foster <bfoster@redhat.com>]

Reports have surfaced of a lockdep splat complaining about an irq-safe
-> irq-unsafe locking order in the xfs_buf_bio_end_io() bio completion
handler. This only occurs when I/O errors are present because bp->b_lock
is only acquired in this context to protect setting an error on the
buffer. The problem is that this lock can be acquired with the
(request_queue) q->queue_lock held. See scsi_end_request() or
ata_qc_schedule_eh(), for example.

Replace bp->b_lock with a new irq-safe lock for use in bio completion.
This avoids the bad lock ordering and should be otherwise low impact as
the lock is only acquired on I/O error.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

Hi all,

I'm not totally sure this is a real issue, but the fix seemed simple
enough...

Brian

 fs/xfs/xfs_buf.c | 10 ++++++----
 fs/xfs/xfs_buf.h |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 9a2191b..aa0ce4a 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -174,6 +174,7 @@ _xfs_buf_alloc(
 	RB_CLEAR_NODE(&bp->b_rbnode);
 	sema_init(&bp->b_sema, 0); /* held, no waiters */
 	spin_lock_init(&bp->b_lock);
+	spin_lock_init(&bp->b_error_lock);
 	XB_SET_OWNER(bp);
 	bp->b_target = target;
 	bp->b_flags = flags;
@@ -1100,21 +1101,22 @@ xfs_bwrite(
 	return error;
 }
 
-STATIC void
+static void
 xfs_buf_bio_end_io(
 	struct bio		*bio)
 {
-	xfs_buf_t		*bp = (xfs_buf_t *)bio->bi_private;
+	struct xfs_buf		*bp = (struct xfs_buf *)bio->bi_private;
+	unsigned long		flags;
 
 	/*
 	 * don't overwrite existing errors - otherwise we can lose errors on
 	 * buffers that require multiple bios to complete.
 	 */
 	if (bio->bi_error) {
-		spin_lock(&bp->b_lock);
+		spin_lock_irqsave(&bp->b_error_lock, flags);
 		if (!bp->b_io_error)
 			bp->b_io_error = bio->bi_error;
-		spin_unlock(&bp->b_lock);
+		spin_unlock_irqrestore(&bp->b_error_lock, flags);
 	}
 
 	if (!bp->b_error && xfs_buf_is_vmapped(bp) && (bp->b_flags & XBF_READ))
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 4eb89bd..c84529c 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -161,6 +161,7 @@ typedef struct xfs_buf {
 	spinlock_t		b_lock;		/* internal state lock */
 	unsigned int		b_state;	/* internal state flags */
 	int			b_io_error;	/* internal IO error state */
+	spinlock_t		b_error_lock;	/* irq-safe IO error lock */
 	wait_queue_head_t	b_waiters;	/* unpin waiters */
 	struct list_head	b_list;
 	struct xfs_perag	*b_pag;		/* contains rbtree root */
-- 
2.4.11

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs