From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 9/9] xfs: log head and tail aren't reliable during shutdown
Date: Wed, 30 Jun 2021 16:38:13 +1000 [thread overview]
Message-ID: <20210630063813.1751007-10-david@fromorbit.com> (raw)
In-Reply-To: <20210630063813.1751007-1-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
I'm seeing assert failures from xlog_space_left() after a shutdown
has begun that look like:
XFS (dm-0): log I/O error -5
XFS (dm-0): xfs_do_force_shutdown(0x2) called from line 1338 of file fs/xfs/xfs_log.c. Return address = xlog_ioend_work+0x64/0xc0
XFS (dm-0): Log I/O Error Detected.
XFS (dm-0): Shutting down filesystem. Please unmount the filesystem and rectify the problem(s)
XFS (dm-0): xlog_space_left: head behind tail
XFS (dm-0): tail_cycle = 6, tail_bytes = 2706944
XFS (dm-0): GH cycle = 6, GH bytes = 1633867
XFS: Assertion failed: 0, file: fs/xfs/xfs_log.c, line: 1310
------------[ cut here ]------------
Call Trace:
xlog_space_left+0xc3/0x110
xlog_grant_push_threshold+0x3f/0xf0
xlog_grant_push_ail+0x12/0x40
xfs_log_reserve+0xd2/0x270
? __might_sleep+0x4b/0x80
xfs_trans_reserve+0x18b/0x260
.....
There are two things here. Firstly, after a shutdown, the log head
and tail can be out of whack as things abort and release (or don't
release) resources, so checking them for sanity doesn't make much
sense. Secondly, xfs_log_reserve() can race with shutdown and so it
can still fail like this even though it has already checked for a
log shutdown before calling xlog_grant_push_ail().
So, before ASSERT failing in xlog_space_left(), make sure we haven't
already shut down....
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_log.c | 51 ++++++++++++++++++++++++------------------------
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index caa07631b2e5..be066a8f90b8 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1269,16 +1269,18 @@ xlog_assign_tail_lsn(
* wrap the tail, we should blow up. Rather than catch this case here,
* we depend on other ASSERTions in other parts of the code. XXXmiken
*
- * This code also handles the case where the reservation head is behind
- * the tail. The details of this case are described below, but the end
- * result is that we return the size of the log as the amount of space left.
+ * If reservation head is behind the tail, we have a problem. Warn about it,
+ * but then treat it as if the log is empty.
+ *
+ * If the log is shut down, the head and tail may be invalid or out of whack, so
+ * shortcut invalidity asserts in this case so that we don't trigger them
+ * falsely.
*/
STATIC int
xlog_space_left(
struct xlog *log,
atomic64_t *head)
{
- int free_bytes;
int tail_bytes;
int tail_cycle;
int head_cycle;
@@ -1287,30 +1289,29 @@ xlog_space_left(
xlog_crack_grant_head(head, &head_cycle, &head_bytes);
xlog_crack_atomic_lsn(&log->l_tail_lsn, &tail_cycle, &tail_bytes);
tail_bytes = BBTOB(tail_bytes);
- if (tail_cycle == head_cycle && head_bytes >= tail_bytes)
- free_bytes = log->l_logsize - (head_bytes - tail_bytes);
- else if (tail_cycle + 1 < head_cycle)
+ if (tail_cycle == head_cycle && head_bytes >= tail_bytes) {
+ return log->l_logsize - (head_bytes - tail_bytes);
+ } else if (tail_cycle + 1 < head_cycle) {
return 0;
- else if (tail_cycle < head_cycle) {
+ } else if (xlog_is_shutdown(log)) {
+ /* Ignore potential inconsistency when shutdown. */
+ return log->l_logsize;
+ } else if (tail_cycle < head_cycle) {
ASSERT(tail_cycle == (head_cycle - 1));
- free_bytes = tail_bytes - head_bytes;
- } else {
- /*
- * The reservation head is behind the tail.
- * In this case we just want to return the size of the
- * log as the amount of space left.
- */
- xfs_alert(log->l_mp, "xlog_space_left: head behind tail");
- xfs_alert(log->l_mp,
- " tail_cycle = %d, tail_bytes = %d",
- tail_cycle, tail_bytes);
- xfs_alert(log->l_mp,
- " GH cycle = %d, GH bytes = %d",
- head_cycle, head_bytes);
- ASSERT(0);
- free_bytes = log->l_logsize;
+ return tail_bytes - head_bytes;
}
- return free_bytes;
+
+ /*
+ * The reservation head is behind the tail. In this case we just want to
+ * return the size of the log as the amount of space left.
+ */
+ xfs_alert(log->l_mp, "xlog_space_left: head behind tail");
+ xfs_alert(log->l_mp, " tail_cycle = %d, tail_bytes = %d",
+ tail_cycle, tail_bytes);
+ xfs_alert(log->l_mp, " GH cycle = %d, GH bytes = %d",
+ head_cycle, head_bytes);
+ ASSERT(0);
+ return log->l_logsize;
}
--
2.31.1
next prev parent reply other threads:[~2021-06-30 6:38 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-30 6:38 [PATCH 0/9] xfs: shutdown is a racy mess Dave Chinner
2021-06-30 6:38 ` [PATCH 1/9] xfs: convert XLOG_FORCED_SHUTDOWN() to xlog_is_shutdown() Dave Chinner
2021-06-30 16:10 ` Darrick J. Wong
2021-07-02 7:48 ` Christoph Hellwig
2021-07-02 8:45 ` Dave Chinner
2021-07-02 9:27 ` Christoph Hellwig
2021-06-30 6:38 ` [PATCH 2/9] xfs: XLOG_STATE_IOERROR must die Dave Chinner
2021-06-30 15:22 ` kernel test robot
2021-06-30 15:22 ` [PATCH] xfs: fix semicolon.cocci warnings kernel test robot
2021-07-02 8:00 ` [PATCH 2/9] xfs: XLOG_STATE_IOERROR must die Christoph Hellwig
2021-07-02 8:55 ` Dave Chinner
2021-06-30 6:38 ` [PATCH 3/9] xfs: move recovery needed state updates to xfs_log_mount_finish Dave Chinner
2021-07-02 8:10 ` Christoph Hellwig
2021-06-30 6:38 ` [PATCH 4/9] xfs: convert log flags to an operational state field Dave Chinner
2021-07-02 8:15 ` Christoph Hellwig
2021-07-09 4:33 ` Darrick J. Wong
2021-06-30 6:38 ` [PATCH 5/9] xfs: make forced shutdown processing atomic Dave Chinner
2021-07-02 8:24 ` Christoph Hellwig
2021-07-05 1:28 ` Dave Chinner
2021-07-09 4:40 ` Darrick J. Wong
2021-07-14 3:15 ` Dave Chinner
2021-07-14 5:02 ` Darrick J. Wong
2021-06-30 6:38 ` [PATCH 6/9] xfs: reowrk up xlog_state_do_callback Dave Chinner
2021-07-02 8:28 ` Christoph Hellwig
2021-07-09 4:42 ` Darrick J. Wong
2021-06-30 6:38 ` [PATCH 7/9] xfs: separate out log shutdown callback processing Dave Chinner
2021-07-02 8:36 ` Christoph Hellwig
2021-07-05 1:46 ` Dave Chinner
2021-06-30 6:38 ` [PATCH 8/9] xfs: don't run shutdown callbacks on active iclogs Dave Chinner
2021-07-02 8:48 ` Christoph Hellwig
2021-07-05 2:04 ` Dave Chinner
2021-06-30 6:38 ` Dave Chinner [this message]
2021-07-02 8:53 ` [PATCH 9/9] xfs: log head and tail aren't reliable during shutdown Christoph Hellwig
2021-07-05 2:22 ` Dave Chinner
2021-07-14 3:19 [PATCH 0/9 v2] xfs: shutdown is a racy mess Dave Chinner
2021-07-14 3:19 ` [PATCH 9/9] xfs: log head and tail aren't reliable during shutdown Dave Chinner
2021-07-14 22:12 ` Darrick J. Wong
2021-08-10 5:18 [PATCH 0/9 v3] xfs: shutdown is a racy mess Dave Chinner
2021-08-10 5:18 ` [PATCH 9/9] xfs: log head and tail aren't reliable during shutdown Dave Chinner
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=20210630063813.1751007-10-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).