On Tue, May 24, 2022 at 12:21:58PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > xfs/538 on a 1kB block filesystem failed with this assert: > > XFS: Assertion failed: cur->bc_btnum != XFS_BTNUM_BMAP || cur->bc_ino.allocated == 0 || xfs_is_shutdown(cur->bc_mp), file: fs/xfs/libxfs/xfs_btree.c, line: 448 > > The problem was that an allocation failed unexpectedly in > xfs_bmbt_alloc_block() after roughly 150,000 minlen allocation error > injections, resulting in an EFSCORRUPTED error being returned to > xfs_bmapi_write(). The error occurred on extent-to-btree format > conversion allocating the new root block: > > RIP: 0010:xfs_bmbt_alloc_block+0x177/0x210 > Call Trace: > <TASK> > xfs_btree_new_iroot+0xdf/0x520 > xfs_btree_make_block_unfull+0x10d/0x1c0 > xfs_btree_insrec+0x364/0x790 > xfs_btree_insert+0xaa/0x210 > xfs_bmap_add_extent_hole_real+0x1fe/0x9a0 > xfs_bmapi_allocate+0x34c/0x420 > xfs_bmapi_write+0x53c/0x9c0 > xfs_alloc_file_space+0xee/0x320 > xfs_file_fallocate+0x36b/0x450 > vfs_fallocate+0x148/0x340 > __x64_sys_fallocate+0x3c/0x70 > do_syscall_64+0x35/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa > > Why the allocation failed at this point is unknown, but is likely > that we ran the transaction out of reserved space and filesystem out > of space with bmbt blocks because of all the minlen allocations > being done causing worst case fragmentation of a large allocation. > > Regardless of the cause, we've then called xfs_bmapi_finish() which > calls xfs_btree_del_cursor(cur, error) to tear down the cursor. > > So we have a failed operation, error != 0, cur->bc_ino.allocated > 0 > and the filesystem is still up. The assert fails to take into > account that allocation can fail with an error and the transaction > teardown will shut the filesystem down if necessary. i.e. the > assert needs to check "|| error != 0" as well, because at this point > shutdown is pending because the current transaction is dirty.... > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_btree.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index 786ec1cb1bba..32100cfb9dfc 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -445,8 +445,14 @@ xfs_btree_del_cursor( > break; > } > > + /* > + * If we are doing a BMBT update, the number of unaccounted blocks > + * allocated during this cursor life time should be zero. If it's not > + * zero, then we should be shut down or on our way to shutdown due to > + * cancelling a dirty transaction on error. > + */ > ASSERT(cur->bc_btnum != XFS_BTNUM_BMAP || cur->bc_ino.allocated == 0 || > - xfs_is_shutdown(cur->bc_mp)); > + xfs_is_shutdown(cur->bc_mp) || error != 0); Ewww, multiline assertions! 8-D Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> --D > if (unlikely(cur->bc_flags & XFS_BTREE_STAGING)) > kmem_free(cur->bc_ops); > if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS) && cur->bc_ag.pag) > -- > 2.35.1 >
