[PATCH V8 15/19] xfs: Directory's data fork extent counter can never overflow

Chandan Babu R
2022-03-21T05:19:06Z

The maximum file size that can be represented by the data fork extent counter
in the worst case occurs when all extents are 1 block in length and each block
is 1KB in size.

With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count and with
1KB sized blocks, a file can reach upto,
(2^31) * 1KB = 2TB

This is much larger than the theoretical maximum size of a directory
i.e. 32GB * 3 = 96GB.

Since a directory's inode can never overflow its data fork extent counter,
this commit replaces checking the return value of
xfs_iext_count_may_overflow() with calls to ASSERT(error == 0).

Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
---
fs/xfs/libxfs/xfs_bmap.c | 19 +++++++++----------
fs/xfs/xfs_inode.c | 32 ++++++++++++++++++++++++++++++++
fs/xfs/xfs_symlink.c | 8 ++++++++
3 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1254d4d4821e..5a089674c666 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5148,24 +5148,23 @@ xfs_bmap_del_extent_real(
*/

/*
- * For directories, -ENOSPC is returned since a directory entry
- * remove operation must not fail due to low extent count
- * availability. -ENOSPC will be handled by higher layers of XFS
+ * Extent count overflow during directory entry remove/rename
+ * operation ideally should result in -ENOSPC being returned.
+ * This error will eventually be handled by higher layers of XFS
* by letting the corresponding empty Data/Free blocks to linger
* until a future remove operation. Dabtree blocks would be
* swapped with the last block in the leaf space and then the
* new last block will be unmapped.
*
- * The above logic also applies to the source directory entry of
- * a rename operation.
+ * However, a directory inode can never overflow its data fork
+ * extent counter because of reasons provided in the definition
+ * of xfs_create().
*/
error = xfs_iext_count_may_overflow(ip, whichfork, 1);
- if (error) {
- ASSERT(S_ISDIR(VFS_I(ip)->i_mode) &&
- whichfork == XFS_DATA_FORK);
- error = -ENOSPC;
+ ASSERT(XFS_TEST_ERROR(false, ip->i_mount,
+ XFS_ERRTAG_REDUCE_MAX_IEXTENTS) || error == 0);
+ if (error)
goto done;
- }

old = got;

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 6810c4feaa45..6016013658ff 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1023,8 +1023,25 @@ xfs_create(
xfs_ilock(dp, XFS_ILOCK_EXCL | XFS_ILOCK_PARENT);
unlock_dp_on_error = true;

+ /*
+ * The maximum file size that can be represented by the data fork extent
+ * counter in the worst case occurs when all extents are 1 block in
+ * length and each block is 1KB in size.
+ *
+ * With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count
+ * and with 1KB sized blocks, a file can reach upto,
+ * 1KB * (2^31) = 2TB
+ *
+ * This is much larger than the theoretical maximum size of a directory
+ * i.e. 32GB * 3 = 96GB.
+ *
+ * Hence, a directory inode can never overflow its data fork extent
+ * counter.
+ */
error = xfs_iext_count_may_overflow(dp, XFS_DATA_FORK,
XFS_IEXT_DIR_MANIP_CNT(mp));
+ ASSERT(XFS_TEST_ERROR(false, dp->i_mount,
+ XFS_ERRTAG_REDUCE_MAX_IEXTENTS) || error == 0);
if (error)
goto out_trans_cancel;

@@ -1249,8 +1266,15 @@ xfs_link(
xfs_trans_ijoin(tp, sip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, tdp, XFS_ILOCK_EXCL);

+ /*
+ * Please refer to the comment in xfs_create() for an explaination of
+ * why a directory inode cannot have its data fork extent counter to
+ * overflow.
+ */
error = xfs_iext_count_may_overflow(tdp, XFS_DATA_FORK,
XFS_IEXT_DIR_MANIP_CNT(mp));
+ ASSERT(XFS_TEST_ERROR(false, tdp->i_mount,
+ XFS_ERRTAG_REDUCE_MAX_IEXTENTS) || error == 0);
if (error)
goto error_return;

@@ -3232,9 +3256,17 @@ xfs_rename(
if (error)
goto out_trans_cancel;
} else {
+ /*
+ * Please refer to the comment in xfs_create() for an
+ * explaination of why a directory inode cannot have its
+ * data fork extent counter to overflow.
+ */
error = xfs_iext_count_may_overflow(target_dp,
XFS_DATA_FORK,
XFS_IEXT_DIR_MANIP_CNT(mp));
+ ASSERT(XFS_TEST_ERROR(false, target_dp->i_mount,
+ XFS_ERRTAG_REDUCE_MAX_IEXTENTS) ||
+ error == 0);
if (error)
goto out_trans_cancel;
}
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index affbedf78160..0e5acb00c59f 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -23,6 +23,7 @@
#include "xfs_trans.h"
#include "xfs_ialloc.h"
#include "xfs_error.h"
+#include "xfs_errortag.h"

/* ----- Kernel only functions below ----- */
int
@@ -226,8 +227,15 @@ xfs_symlink(
goto out_trans_cancel;
}

+ /*
+ * Please refer to the comment in xfs_create() for an explaination of
+ * why a directory inode cannot have its data fork extent counter to
+ * overflow.
+ */
error = xfs_iext_count_may_overflow(dp, XFS_DATA_FORK,
XFS_IEXT_DIR_MANIP_CNT(mp));
+ ASSERT(XFS_TEST_ERROR(false, dp->i_mount,
+ XFS_ERRTAG_REDUCE_MAX_IEXTENTS) || error == 0);
if (error)
goto out_trans_cancel;

--
2.30.2