diff mbox series

[Bug,202053,xfstests,generic/464] : XFS corruption and Assertion failed: 0, file: fs/xfs/xfs_super.c, line: 985

Message ID bug-202053-201763-NmI066HwJr@https.bugzilla.kernel.org/ (mailing list archive)
State New, archived
Headers show
Series [Bug,202053,xfstests,generic/464] : XFS corruption and Assertion failed: 0, file: fs/xfs/xfs_super.c, line: 985 | expand

Commit Message

bugzilla-daemon@bugzilla.kernel.org Jan. 4, 2019, 12:52 p.m. UTC 
https://bugzilla.kernel.org/show_bug.cgi?id=202053

--- Comment #7 from bfoster@redhat.com ---
On Fri, Jan 04, 2019 at 07:32:17AM -0500, Brian Foster wrote:
> On Tue, Dec 25, 2018 at 06:10:59AM +0000, bugzilla-daemon@bugzilla.kernel.org
> wrote:
> > https://bugzilla.kernel.org/show_bug.cgi?id=202053
> > 
> > --- Comment #5 from Zorro Lang (zlang@redhat.com) ---
> > (In reply to Zorro Lang from comment #4)
> > > I never hit this bug before, just a similar bug which has been fixed one
> > > year ago, by:
> > > commit 40214d128e07dd21bb07a8ed6a7fe2f911281ab2
> > > Author: Brian Foster <bfoster@redhat.com>
> > > Date:   Fri Oct 13 09:47:46 2017 -0700
> > > 
> > >     xfs: trim writepage mapping to within eof
> > > 
> > > So I doubt if this's a regression issue?
> > 
> > I just reproduced this issue on kernel 4.19, so it's not a regression from
> > v4.19:
> > 
> > [ 1297.449750] XFS: Assertion failed: XFS_FORCED_SHUTDOWN(ip->i_mount) ||
> > ip->i_delayed_blks == 0, file: fs/xfs/xfs_super.c, line: 954
> > [ 1297.463147] WARNING: CPU: 20 PID: 26952 at fs/xfs/xfs_message.c:104
> > assfail+0x54/0x57 [xfs]
> > [ 1297.472473] Modules linked in: sunrpc intel_rapl sb_edac
> > x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass
> > crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ipmi_ssif 
> > intel_cstate intel_uncore iTCO_wdt iTCO_vendor_support ipmi_si sg
> > intel_rapl_perf ipmi_devintf wmi ioatdma i2c_i801 pcspkr ipmi_msghandler
> > lpc_ich xfs libcrc32c sd_mod mgag200 drm_kms_helper 
> > syscopyarea sysfillrect sysimgblt igb fb_sys_fops ttm dca drm crc32c_intel
> > megaraid_sas i2c_algo_bit cdc_ether usbnet mii dm_mirror dm_region_hash
> dm_log
> > dm_mod
> > [ 1297.525374] CPU: 20 PID: 26952 Comm: umount Not tainted 4.19.0-mainline
> #1
> > 
> 
> I can reproduce this problem and it appears to be somewhat related to
> the commit referenced above, mainly because the placement of the imap
> trim leaves a larger than necessary window to race with external changes
> to the extent map.
> 
> For example, a trace dump shows the following sequence of events:
> 
> - writepages is in progress on a particular file that has decently sized
>   post-eof speculative preallocation
> - writepages gets to the point where it looks up or allocates a new imap
>   that includes the preallocation, the allocation/lookup result is
>   stored in wpc
> - the file is closed by one process, killing off preallocation, then
>   immediately appended to by another, updating the file size by a few
>   bytes
> - writepages comes back around to xfs_map_blocks() and trims imap to the
>   current size, but imap still includes one block of the original speculative
>   prealloc (that was truncated and recreated) because the size increased
>   between the time imap was stored and trimmed
> 
> The EOF trim approach is known to be a bandaid and potentially racy, but
> ISTM that this problem can be trivially avoided by moving or adding
> trims of wpc->imap immediately after a new one is cached. I don't
> reproduce the problem so far with a couple such extra calls in place.
> 
> Bigger picture, we need some kind of invalidation mechanism similar to
> what we're already doing for dealing with the COW fork in this writeback
> context. I'm not sure the broad semantics used by the COW fork sequence
> counter mechanism is really suitable for the data fork because any
> extent-related change in the fork would cause an invalidation, but I am
> wondering if we could define some subset of less frequent operations for
> the same mechanism to reliably invalidate (e.g., on eofblocks trims, for
> starters).
> 

Zorro,

Can you still reproduce with the following patch?

Brian

--- 8< ---
diff mbox series

Patch

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 338b9d9984e0..d9048bcea49c 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -449,6 +449,7 @@  xfs_map_blocks(
        }

        wpc->imap = imap;
+       xfs_trim_extent_eof(&wpc->imap, ip);
        trace_xfs_map_blocks_found(ip, offset, count, wpc->io_type, &imap);
        return 0;
 allocate_blocks:
@@ -459,6 +460,7 @@  xfs_map_blocks(
        ASSERT(whichfork == XFS_COW_FORK || cow_fsb == NULLFILEOFF ||
               imap.br_startoff + imap.br_blockcount <= cow_fsb);
        wpc->imap = imap;
+       xfs_trim_extent_eof(&wpc->imap, ip);
        trace_xfs_map_blocks_alloc(ip, offset, count, wpc->io_type, &imap);
        return 0;
 }