linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 4/9] xfs: don't assert fail with AIL lock held
Date: Tue,  8 May 2018 13:41:57 +1000	[thread overview]
Message-ID: <20180508034202.10136-5-david@fromorbit.com> (raw)
In-Reply-To: <20180508034202.10136-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

Been hitting AIL ordering assert failures recently, but been unable
to trace them down because the system immediately hangs up onteh
spinlock that was held when this assert fires:

XFS: Assertion failed: XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0, file: fs/xfs/xfs_trans_ail.c, line: 52

Move the assertions outside of the spinlock so the corpse can
be dissected. Thanks to Brian Foster for supplying a clean
way of doing this.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_trans_ail.c | 43 +++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 50611d2bcbc2..41e280ef1483 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -32,30 +32,51 @@
 #ifdef DEBUG
 /*
  * Check that the list is sorted as it should be.
+ *
+ * Called with the ail lock held, but we don't want to assert fail with it
+ * held otherwise we'll lock everything up and won't be able to debug the
+ * cause. Hence we sample and check the state under the AIL lock and return if
+ * everything is fine, otherwise we drop the lock and run the ASSERT checks.
+ * Asserts may not be fatal, so pick the lock back up and continue onwards.
  */
 STATIC void
 xfs_ail_check(
-	struct xfs_ail	*ailp,
-	xfs_log_item_t	*lip)
+	struct xfs_ail		*ailp,
+	struct xfs_log_item	*lip)
 {
-	xfs_log_item_t	*prev_lip;
+	struct xfs_log_item	*prev_lip;
+	struct xfs_log_item	*next_lip;
+	xfs_lsn_t		prev_lsn = NULLCOMMITLSN;
+	xfs_lsn_t		next_lsn = NULLCOMMITLSN;
+	xfs_lsn_t		lsn;
+	bool			in_ail;
+
 
 	if (list_empty(&ailp->ail_head))
 		return;
 
 	/*
-	 * Check the next and previous entries are valid.
+	 * Sample then check the next and previous entries are valid.
 	 */
-	ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));
-	prev_lip = list_entry(lip->li_ail.prev, xfs_log_item_t, li_ail);
+	in_ail = test_bit(XFS_LI_IN_AIL, &lip->li_flags);
+	prev_lip = list_entry(lip->li_ail.prev, struct xfs_log_item, li_ail);
 	if (&prev_lip->li_ail != &ailp->ail_head)
-		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
-
-	prev_lip = list_entry(lip->li_ail.next, xfs_log_item_t, li_ail);
-	if (&prev_lip->li_ail != &ailp->ail_head)
-		ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) >= 0);
+		prev_lsn = prev_lip->li_lsn;
+	next_lip = list_entry(lip->li_ail.next, struct xfs_log_item, li_ail);
+	if (&next_lip->li_ail != &ailp->ail_head)
+		next_lsn = next_lip->li_lsn;
+	lsn = lip->li_lsn;
 
+	if (in_ail &&
+	    (prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0) &&
+	    (next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0))
+		return;
 
+	spin_unlock(&ailp->ail_lock);
+	ASSERT(in_ail);
+	ASSERT(prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0);
+	ASSERT(next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0);
+	spin_lock(&ailp->ail_lock);
 }
 #else /* !DEBUG */
 #define	xfs_ail_check(a,l)
-- 
2.17.0


  parent reply	other threads:[~2018-05-08  3:42 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08  3:41 [PATCH 0/9 v2] xfs: log item and transaction cleanups Dave Chinner
2018-05-08  3:41 ` [PATCH 1/9] xfs: log item flags are racy Dave Chinner
2018-05-09 14:51   ` Darrick J. Wong
2018-05-08  3:41 ` [PATCH 2/9] xfs: add tracing to high level transaction operations Dave Chinner
2018-05-09 14:51   ` Darrick J. Wong
2018-05-08  3:41 ` [PATCH 3/9] xfs: adder caller IP to xfs_defer* tracepoints Dave Chinner
2018-05-09 14:52   ` Darrick J. Wong
2018-05-08  3:41 ` Dave Chinner [this message]
2018-05-08 14:18   ` [PATCH 4/9] xfs: don't assert fail with AIL lock held Brian Foster
2018-05-09  6:13   ` Christoph Hellwig
2018-05-09 14:52   ` Darrick J. Wong
2018-05-08  3:41 ` [PATCH 5/9] xfs: fix double ijoin in xfs_inactive_symlink_rmt() Dave Chinner
2018-05-08 14:18   ` Brian Foster
2018-05-09  0:24     ` Dave Chinner
2018-05-09 10:10       ` Brian Foster
2018-05-09 15:02         ` Darrick J. Wong
2018-05-11  2:04           ` Dave Chinner
2018-05-11 13:24             ` Brian Foster
2018-05-12  2:00               ` Dave Chinner
2018-05-12 14:17                 ` Brian Foster
2018-05-08  3:41 ` [PATCH 6/9] xfs: fix double ijoin in xfs_reflink_cancel_cow_range Dave Chinner
2018-05-08 14:18   ` Brian Foster
2018-05-09 15:17   ` Darrick J. Wong
2018-05-08  3:42 ` [PATCH 7/9] xfs: fix double ijoin in xfs_reflink_clear_inode_flag() Dave Chinner
2018-05-08 14:18   ` Brian Foster
2018-05-09  0:40     ` Dave Chinner
2018-05-09 10:12       ` Brian Foster
2018-05-09 15:19         ` Darrick J. Wong
2018-05-08  3:42 ` [PATCH 8/9] xfs: add some more debug checks to buffer log item reuse Dave Chinner
2018-05-08 14:18   ` Brian Foster
2018-05-09 15:19   ` Darrick J. Wong
2018-05-08  3:42 ` [PATCH 9/9] xfs: get rid of the log item descriptor Dave Chinner
2018-05-08 14:18   ` Brian Foster
2018-05-09  6:27   ` Christoph Hellwig
2018-05-09 15:19   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180508034202.10136-5-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).