Re: [PATCH] xfs: truncate transaction does not modify the inobt

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Nov 3, 2018 at 7:15 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> From: Brian Foster <bfoster@redhat.com>
>
> The truncate transaction does not ever modify the inode btree, but
> includes an associated log reservation. Update
> xfs_calc_itruncate_reservation() to remove the reservation
> associated with inobt updates.
>
> [Amir:  This commit was merged for kernel v4.16 and a twin commit was
>         merged for xfsprogs v4.16. As a result, a small xfs filesystem
>         formatted with features -m rmapbt=1,reflink=1 using mkfs.xfs
>         version >= v4.16 cannot be mounted with kernel < v4.16.
>
>         For example, xfstests generic/17{1,2,3} format a small fs and
>         when trying to mount it, they fail with an assert on this very
>         demonic line:
>
>  XFS (vdc): Log size 3075 blocks too small, minimum size is 3717 blocks
>  XFS (vdc): AAIEEE! Log failed size checks. Abort!
>  XFS: Assertion failed: 0, file: src/linux/fs/xfs/xfs_log.c, line: 666
>
>         The simple solution for stable kernels is to apply this patch,
>         because mkfs.xfs v4.16 is already in the wild, so we have to
>         assume that xfs filesystems with a "too small" log exist.
>         Regardless, xfsprogs maintainers should also consider reverting
>         the twin patch to stop creating those filesystems for the sake
>         of users with unpatched kernels.]
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Cc: <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Darrick/Dave,
>
> It took me a while to figure out what was going on with my test systems
> when small test partitions (10G) stopped working with older kernels.
>
> Please bless this change for stable and consider the remedie for mkfs.xfs
> I verified that patch cleanly applies to stable kernels 4.14.y and 4.9.y
> and that I can mount a filsystem created with new mkfs.xfs.
>
> I am now running quick tests on stable 4.14.y with configs 4k, 1k,
> reflink,reflink+overlay to verify no regressions from this patch.
>

FYI no regressions detected.

Thoughts?

Thanks,
Amir.

>
>
>  fs/xfs/libxfs/xfs_trans_resv.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 6bd916bd35e2..48eff18c5496 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -232,8 +232,6 @@ xfs_calc_write_reservation(
>   *    the super block to reflect the freed blocks: sector size
>   *    worst case split in allocation btrees per extent assuming 4 extents:
>   *             4 exts * 2 trees * (2 * max depth - 1) * block size
> - *    the inode btree: max depth * blocksize
> - *    the allocation btrees: 2 trees * (max depth - 1) * block size
>   */
>  STATIC uint
>  xfs_calc_itruncate_reservation(
> @@ -245,12 +243,7 @@ xfs_calc_itruncate_reservation(
>                                       XFS_FSB_TO_B(mp, 1))),
>                     (xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
>                      xfs_calc_buf_res(xfs_allocfree_log_count(mp, 4),
> -                                     XFS_FSB_TO_B(mp, 1)) +
> -                   xfs_calc_buf_res(5, 0) +
> -                   xfs_calc_buf_res(xfs_allocfree_log_count(mp, 1),
> -                                    XFS_FSB_TO_B(mp, 1)) +
> -                   xfs_calc_buf_res(2 + mp->m_ialloc_blks +
> -                                    mp->m_in_maxlevels, 0)));
> +                                     XFS_FSB_TO_B(mp, 1))));
>  }
>
>  /*
> --
> 2.17.1
>