xfs: truncate transaction does not modify the inobt

From: Brian Foster <[email protected]>

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 <[email protected]>

Reviewed-by Dave Chinner <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Darrick J. Wong <[email protected]>

Cc <[email protected]> # v4.9+

Signed-off-by Amir Goldstein <[email protected]>

---

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.

 fs/xfs/libxfs/xfs_trans_resv.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

fs/xfs/libxfs/xfs_trans_resv.c +1 -9

index 6bd916bd35e2..48eff18c5496 100644
@@ -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

Quoted 45 lines · Amir Goldstein

From: Brian Foster <[email protected]>

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 <[email protected]>

Reviewed-by Dave Chinner <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Darrick J. Wong <[email protected]>

Cc <[email protected]> # v4.9+

Signed-off-by Amir Goldstein <[email protected]>

---

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.

Quoted 3 lines

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?

Quoted 51 lines · Amir Goldstein

Quoted 48 lines · Amir Goldstein

From: Brian Foster <[email protected]>

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 <[email protected]>

Reviewed-by Dave Chinner <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Darrick J. Wong <[email protected]>

Cc <[email protected]> # v4.9+

Signed-off-by Amir Goldstein <[email protected]>

---

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.

Quoted 4 lines

FYI no regressions detected.

Thoughts?

Maybe you'd want to chalk it up to reflink/rmapbt being Experimental before kernel 4.16? so the change in "minimum log size" is an on-disk format change prior to removing the Experimental label??

Thanks,
Amir.

Quoted 54 lines · Amir Goldstein

Quoted 53 lines · Amir Goldstein

Quoted 41 lines · Amir Goldstein

From: Brian Foster <[email protected]>

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 <[email protected]>

Reviewed-by Dave Chinner <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Darrick J. Wong <[email protected]>

Cc <[email protected]> # v4.9+

Signed-off-by Amir Goldstein <[email protected]>

---

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.

Quoted 7 lines

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.

Quoted 6 lines

Quoted 2 lines

Thoughts?

Maybe you'd want to chalk it up to reflink/rmapbt being Experimental before kernel 4.16? so the change in "minimum log size" is an on-disk format change prior to removing the Experimental label??

TBH nobody should be using reflink/rmap on 4.14 kernels, ever. :D

That said... does it change the minimum log size for (finobt, !reflink, !rmap) filesystems? That might be a bigger worry. I /think/ the

transaction reservation change is fine, though I defer to Amir on testing... :)

Quoted 56 lines · Darrick J. Wong

Quoted 54 lines · Amir Goldstein

Quoted 53 lines · Amir Goldstein

Quoted 41 lines · Amir Goldstein

From: Brian Foster <[email protected]>

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 <[email protected]>

Reviewed-by Dave Chinner <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Darrick J. Wong <[email protected]>

Cc <[email protected]> # v4.9+

Signed-off-by Amir Goldstein <[email protected]>

---

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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Thoughts?

Maybe you'd want to chalk it up to reflink/rmapbt being Experimental before kernel 4.16? so the change in "minimum log size" is an on-disk format change prior to removing the Experimental label??

TBH nobody should be using reflink/rmap on 4.14 kernels, ever. :D

I agree, as these error messages try to express:

[    4.982926] XFS (vdd): EXPERIMENTAL reverse mapping btree feature
enabled. Use at your own risk!
[    4.984843] XFS (vdd): EXPERIMENTAL reflink feature enabled. Use at
your own risk!
[    4.987259] XFS (vdd): Log size 3693 blocks too small, minimum size

is 4473 blocks

But it is still a regression, because as I understand some where using reflink back from
stable 4.9, while it was still maintained...

Quoted 3 lines

That said... does it change the minimum log size for (finobt, !reflink, !rmap) filesystems? That might be a bigger worry. I /think/ the

No problem on my systems mounting small fs with (finobt, !reflink,!rmap) formatted with mkfs.xfs 4.18.

Quoted 3 lines

transaction reservation change is fine, though I defer to Amir on testing... :)

Testing passed as I wrote, for configs 4k,1k,reflink,reflink+overlay.

Sasha, please consider the fix patch for 4.14.y,4.9.y.