Re: xfs: Assertion failed in xfs_ag_resv_init()

[Andre Noll <maan@tuebingen.mpg.de>]

[-- Attachment #1: Type: text/plain, Size: 6529 bytes --]

On Wed, May 01, 08:36, Darrick J. Wong wrote
> > > You could send this patch to the stable list, but my guess is that
> > > they'd prefer a straight backport of all three commits...
> > 
> > Hm, cherry-picking the first commit onto 4.9,171 already gives
> > four conflicting files. The conflicts are trivial to resolve (git
> > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't
> > compile because xfs_btree_query_all() is missing.  So e9a2599a249ed
> > (xfs: create a function to query all records in a btree) is needed as
> > well. But then, applying 86210fbebae (xfs: move various type verifiers
> > to common file) on top of that gives non-trivial conflicts.
> 
> Ah, I suspected that might happen.  Backports are hard. :(
> 
> I suppose one saving grace of the patch you sent is that it'll likely
> break the build if anyone ever /does/ attempt a backport of those first
> two commits.  Perhaps that is the most practical way forward.
> 
> > So, for automatic backporting we would need to cherry-pick even more,
> > and each backported commit should be tested of course. Given this, do
> > you still think Greg prefers a rather large set of straight backports
> > over the simple commit that just pulls in the missing function?
> 
> I think you'd have to ask him that, if you decide not to send
> yesterday's patch.

Let's try. I've added a sentence to the commit message which explains
why a straight backport is not practical, and how to proceed if anyone
wants to backport the earlier commits.

Greg: Under the given circumstances, would you be willing to accept
the patch below for 4.9?

Thanks
Andre
---
commit 6a12a8bda15d5223103df76b8f92cc277c2f5e69
Author: Dave Chinner <dchinner@redhat.com>
Date:   Mon Nov 19 13:31:08 2018 -0800

    xfs: finobt AG reserves don't consider last AG can be a runt
    
    This is a backport of upstream commit c08768977b9 and the part of
    21ec54168b36 which is needed by c08768977b9 to fix a ENOSPC issue
    due to a bug in the finobt per-ag space reservation code. The bug
    was observed on an almost empty 100T large filesystem whose last AG
    happened to be very small.
    
    A direct backport of the above mentioned two commits is not possible
    because the second commit relies on further infrastructure that was
    introduced earlier. If anyone ever attempts to backport those earlier
    commits, this commit has to be reverted first so that the earlier
    commits apply, and c08768977b9 has to be applied on top of that.
    
    Commit log of c08768977b9 follows.
    
            commit c08768977b9a65cab9bcfd1ba30ffb686b2b7c69
            Author: Dave Chinner <dchinner@redhat.com>
            Date:   Mon Nov 19 13:31:08 2018 -0800
    
                xfs: finobt AG reserves don't consider last AG can be a runt
    
                The last AG may be very small comapred to all other AGs, and hence
                AG reservations based on the superblock AG size may actually consume
                more space than the AG actually has. This results on assert failures
                like:
    
                XFS: Assertion failed: xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved + xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <= pag->pagf_freeblks + pag->pagf_flcount, file: fs/xfs/libxfs/xfs_ag_resv.c, line: 319
                [   48.932891]  xfs_ag_resv_init+0x1bd/0x1d0
                [   48.933853]  xfs_fs_reserve_ag_blocks+0x37/0xb0
                [   48.934939]  xfs_mountfs+0x5b3/0x920
                [   48.935804]  xfs_fs_fill_super+0x462/0x640
                [   48.936784]  ? xfs_test_remount_options+0x60/0x60
                [   48.937908]  mount_bdev+0x178/0x1b0
                [   48.938751]  mount_fs+0x36/0x170
                [   48.939533]  vfs_kern_mount.part.43+0x54/0x130
                [   48.940596]  do_mount+0x20e/0xcb0
                [   48.941396]  ? memdup_user+0x3e/0x70
                [   48.942249]  ksys_mount+0xba/0xd0
                [   48.943046]  __x64_sys_mount+0x21/0x30
                [   48.943953]  do_syscall_64+0x54/0x170
                [   48.944835]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
    
                Hence we need to ensure the finobt per-ag space reservations take
                into account the size of the last AG rather than treat it like all
                the other full size AGs.
    
                Note that both refcountbt and rmapbt already take the size of the AG
                into account via reading the AGF length directly.
    
                Signed-off-by: Dave Chinner <dchinner@redhat.com>
                Reviewed-by: Christoph Hellwig <hch@lst.de>
                Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
                Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
    
    Suggested-by: Darrick J. Wong <darrick.wong@oracle.com>
    Tested-by: Andre Noll <maan@tuebingen.mpg.de>

diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index b9c351ff0422..33905989929e 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -502,17 +502,33 @@ xfs_inobt_rec_check_count(
 }
 #endif	/* DEBUG */
 
+/* Find the size of the AG, in blocks. */
+static xfs_agblock_t
+xfs_ag_block_count(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
+{
+	ASSERT(agno < mp->m_sb.sb_agcount);
+
+	if (agno < mp->m_sb.sb_agcount - 1)
+		return mp->m_sb.sb_agblocks;
+	return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
+}
+
 static xfs_extlen_t
 xfs_inobt_max_size(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
 {
+	xfs_agblock_t		agblocks = xfs_ag_block_count(mp, agno);
+
 	/* Bail out if we're uninitialized, which can happen in mkfs. */
 	if (mp->m_inobt_mxr[0] == 0)
 		return 0;
 
 	return xfs_btree_calc_size(mp, mp->m_inobt_mnr,
-		(uint64_t)mp->m_sb.sb_agblocks * mp->m_sb.sb_inopblock /
-				XFS_INODES_PER_CHUNK);
+				(uint64_t)agblocks * mp->m_sb.sb_inopblock /
+					XFS_INODES_PER_CHUNK);
 }
 
 static int
@@ -558,7 +574,7 @@ xfs_finobt_calc_reserves(
 	if (error)
 		return error;
 
-	*ask += xfs_inobt_max_size(mp);
+	*ask += xfs_inobt_max_size(mp, agno);
 	*used += tree_len;
 	return 0;
 }
-- 
Max Planck Institute for Developmental Biology
Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829
http://people.tuebingen.mpg.de/maan/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]