From: Dave Chinner <david@fromorbit.com>
To: xfs@oss.sgi.com
Subject: [PATCH 1/6] xfs: don't try to mark uncached buffers stale on error.
Date: Thu, 12 Dec 2013 16:34:33 +1100 [thread overview]
Message-ID: <1386826478-13846-2-git-send-email-david@fromorbit.com> (raw)
In-Reply-To: <1386826478-13846-1-git-send-email-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
fsstress failed during a shutdown with the following assert:
XFS: Assertion failed: xfs_buf_islocked(bp), file: fs/xfs/xfs_buf.c, line: 143
.....
xfs_buf_stale+0x3f/0xf0
xfs_bioerror_relse+0x2d/0x90
xfsbdstrat+0x51/0xa0
xfs_zero_remaining_bytes+0x1d1/0x2d0
xfs_free_file_space+0x5d0/0x600
xfs_change_file_space+0x251/0x3a0
xfs_ioc_space+0xcc/0x130
.....
xfs_zero_remaining_bytes() works with uncached buffers, and hence if
we are preventing IO due to a shutdown, we should not be marking it
stale as that is only for cached buffers. Instead, just mark it with
an error and make sure it gets to the caller.
[ 7732.193441] XFS: Assertion failed: xfs_buf_islocked(bp), file: fs/xfs/xfs_buf.c, line: 96
[ 7732.195036] ------------[ cut here ]------------
[ 7732.195890] kernel BUG at fs/xfs/xfs_message.c:107!
[ 7732.196018] invalid opcode: 0000 [#1] SMP
[ 7732.196018] Modules linked in:
[ 7732.196018] CPU: 0 PID: 2899 Comm: fsstress Not tainted 3.12.0-rc7-dgc+ #47
[ 7732.196018] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 7732.196018] task: ffff88003a83ada0 ti: ffff88002c0ae000 task.ti: ffff88002c0ae000
[ 7732.196018] RIP: 0010:[<ffffffff81488462>] [<ffffffff81488462>] assfail+0x22/0x30
[ 7732.196018] RSP: 0000:ffff88002c0afae8 EFLAGS: 00010292
[ 7732.196018] RAX: 000000000000004d RBX: ffff880002e59600 RCX: 0000000000000000
[ 7732.196018] RDX: ffff88003fc0ed68 RSI: ffff88003fc0d3f8 RDI: 0000000000000246
[ 7732.196018] RBP: ffff88002c0afae8 R08: 0000000000000096 R09: 00000000000012ec
[ 7732.196018] R10: 0000000000000000 R11: 00000000000012eb R12: 0000000000100002
[ 7732.196018] R13: ffffffff81473523 R14: 0000000000009fff R15: 0000000000009fff
[ 7732.196018] FS: 00007f8017e4a700(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 7732.196018] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 7732.196018] CR2: 00007ff8d020a000 CR3: 0000000009b43000 CR4: 00000000000006f0
[ 7732.196018] Stack:
[ 7732.196018] ffff88002c0afb08 ffffffff8147476e ffff880002e59600 0000000000100002
[ 7732.196018] ffff88002c0afb28 ffffffff814762cd ffff880002e59600 ffff880002e59600
[ 7732.196018] ffff88002c0afb58 ffffffff81476841 0000000000009f89 ffff880002e59600
[ 7732.196018] Call Trace:
[ 7732.196018] [<ffffffff8147476e>] xfs_buf_stale+0x2e/0xb0
[ 7732.196018] [<ffffffff814762cd>] xfs_bioerror_relse+0x2d/0x90
[ 7732.196018] [<ffffffff81476841>] xfsbdstrat+0x51/0x90
[ 7732.196018] [<ffffffff81473523>] xfs_zero_remaining_bytes+0x1d3/0x2d0
[ 7732.196018] [<ffffffff81473ba6>] xfs_free_file_space+0x586/0x5b0
[ 7732.196018] [<ffffffff81190002>] ? slabs_cpu_partial_show+0xd2/0x120
[ 7732.196018] [<ffffffff811b9399>] ? mntput_no_expire+0x49/0x160
[ 7732.196018] [<ffffffff811b8812>] ? mnt_clone_write+0x12/0x30
[ 7732.196018] [<ffffffff81aba3f6>] ? down_write+0x16/0x40
[ 7732.196018] [<ffffffff81481c14>] xfs_ioc_space+0x2d4/0x450
[ 7732.196018] [<ffffffff811a700b>] ? path_lookupat+0x6b/0x760
[ 7732.196018] [<ffffffff8148f57e>] ? xfs_trans_free+0x6e/0x80
[ 7732.196018] [<ffffffff81192a81>] ? kmem_cache_alloc+0x31/0x150
[ 7732.196018] [<ffffffff8148326b>] xfs_file_ioctl+0x48b/0xae0
[ 7732.196018] [<ffffffff811a60d6>] ? final_putname+0x26/0x50
[ 7732.196018] [<ffffffff8110b6a2>] ? from_kgid+0x12/0x20
[ 7732.196018] [<ffffffff8110b6be>] ? from_kgid_munged+0xe/0x20
[ 7732.196018] [<ffffffff8119f856>] ? cp_new_stat+0x146/0x160
[ 7732.196018] [<ffffffff811ace02>] do_vfs_ioctl+0x452/0x530
[ 7732.196018] [<ffffffff8119fd35>] ? SYSC_newfstat+0x25/0x30
[ 7732.196018] [<ffffffff811acf71>] SyS_ioctl+0x91/0xb0
[ 7732.196018] [<ffffffff81ac6029>] system_call_fastpath+0x16/0x1b
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_buf.c | 47 ++++++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index ce01c1a..27dc152 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1076,7 +1076,14 @@ xfs_bioerror(
*/
XFS_BUF_UNREAD(bp);
XFS_BUF_UNDONE(bp);
- xfs_buf_stale(bp);
+
+ /*
+ * we might be handling an uncached buffer here, in which case the
+ * stale buffer handling is irrelevant as is doing IO with the buffer
+ * locked. Hence we don't mark them stale.
+ */
+ if (bp->b_pag)
+ xfs_buf_stale(bp);
xfs_buf_ioend(bp, 0);
@@ -1094,25 +1101,20 @@ xfs_bioerror_relse(
struct xfs_buf *bp)
{
int64_t fl = bp->b_flags;
+
/*
- * No need to wait until the buffer is unpinned.
- * We aren't flushing it.
- *
- * chunkhold expects B_DONE to be set, whether
- * we actually finish the I/O or not. We don't want to
- * change that interface.
+ * No need to wait until the buffer is unpinned. We aren't flushing it.
*/
XFS_BUF_UNREAD(bp);
XFS_BUF_DONE(bp);
xfs_buf_stale(bp);
bp->b_iodone = NULL;
+
+ /*
+ * There's no reason to mark error for ASYNC buffers as there is no-one
+ * waiting to collect the error.
+ */
if (!(fl & XBF_ASYNC)) {
- /*
- * Mark b_error and B_ERROR _both_.
- * Lot's of chunkcache code assumes that.
- * There's no reason to mark error for
- * ASYNC buffers.
- */
xfs_buf_ioerror(bp, EIO);
complete(&bp->b_iowait);
} else {
@@ -1129,14 +1131,13 @@ xfs_bdstrat_cb(
if (XFS_FORCED_SHUTDOWN(bp->b_target->bt_mount)) {
trace_xfs_bdstrat_shut(bp, _RET_IP_);
/*
- * Metadata write that didn't get logged but
- * written delayed anyway. These aren't associated
- * with a transaction, and can be ignored.
+ * If this is a cached write, then it is likely to be a delayed
+ * write metadata buffer that can be ignored because the
+ * contents are logged.
*/
if (!bp->b_iodone && !XFS_BUF_ISREAD(bp))
return xfs_bioerror_relse(bp);
- else
- return xfs_bioerror(bp);
+ return xfs_bioerror(bp);
}
xfs_buf_iorequest(bp);
@@ -1176,7 +1177,15 @@ xfsbdstrat(
{
if (XFS_FORCED_SHUTDOWN(mp)) {
trace_xfs_bdstrat_shut(bp, _RET_IP_);
- xfs_bioerror_relse(bp);
+ /*
+ * we could be handling uncached IO here, in which case there is
+ * always a caller waiting to collect the error and releas the
+ * buffer.
+ */
+ if (bp->b_pag)
+ xfs_bioerror_relse(bp);
+ else
+ xfs_bioerror(bp);
return;
}
--
1.8.4.rc3
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-12-12 5:34 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-12 5:34 [PATCH 0/6] xfs: fixes for 3.13-rc4 Dave Chinner
2013-12-12 5:34 ` Dave Chinner [this message]
2013-12-12 9:30 ` [PATCH 1/6] xfs: don't try to mark uncached buffers stale on error Jeff Liu
2013-12-12 10:09 ` Dave Chinner
2013-12-13 4:47 ` Jeff Liu
2013-12-12 16:36 ` Christoph Hellwig
2013-12-12 22:24 ` Dave Chinner
2013-12-13 11:01 ` Christoph Hellwig
2013-12-13 13:02 ` Christoph Hellwig
2013-12-16 22:44 ` Ben Myers
2013-12-17 8:03 ` [PATCH v2] xfs: remove xfsbdstrat error Christoph Hellwig
2013-12-12 5:34 ` [PATCH 2/6] xfs: prevent spurious "head behind tail" warnings Dave Chinner
2013-12-12 5:34 ` [PATCH 3/6] xfs: prevent spurious "space > BBTOB(tail_blocks)" warnings Dave Chinner
2013-12-12 5:34 ` [PATCH 4/6] xfs: swalloc doesn't align allocations properly Dave Chinner
2013-12-13 12:01 ` Christoph Hellwig
2013-12-16 23:14 ` Ben Myers
2013-12-17 3:39 ` Dave Chinner
2013-12-17 14:59 ` Ben Myers
2013-12-12 5:34 ` [PATCH 5/6] xfs: xlog_recover_process_data leaks like a sieve Dave Chinner
2013-12-13 12:32 ` Christoph Hellwig
2013-12-13 22:11 ` Dave Chinner
2013-12-16 15:23 ` Christoph Hellwig
2013-12-17 17:58 ` Mark Tinguely
2013-12-12 5:34 ` [PATCH 6/6] xfs: abort metadata writeback on permanent errors Dave Chinner
2013-12-13 12:33 ` Christoph Hellwig
2013-12-17 16:02 ` [PATCH 0/6] xfs: fixes for 3.13-rc4 Ben Myers
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=1386826478-13846-2-git-send-email-david@fromorbit.com \
--to=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.