[PATCH 4/5] xfs: correct the parent pointer space reservation comment

[Javier Tia <javier@peridio.com>]

The comment on xfs_parent_calc_space_res() claims parent pointers are
"always the first attr in an attr tree".  They are not: a parent pointer
is recorded per dirent, so an inode with N hardlinks carries N of them,
and `xfs_io -c "parent -p"` on a 31-link file lists 31.  By the Nth link
the attr fork is in leaf or node format and the insert is not into a
fresh tree.

The reservation itself is fine, which is what makes the comment worth
fixing rather than the code.  XFS_DAENTER_SPACE_RES() reserves
XFS_DA_NODE_MAXDEPTH blocks plus a bmap allowance for each, i.e. enough
to split every level of a maximum-depth attr dabtree.  That depth is a
format ceiling, not a runtime property, so the result cannot depend on
the format the fork happens to be in.  Anyone auditing a reservation
shortfall here reads the comment, concludes the sizing rests on an
assumption that demonstrably does not hold, and goes looking for a bug
that is not there.

Record why no double split allowance is needed either, since that is one
of two visible differences from xfs_attr_calc_size() and is not obvious
from the expression: a parent pointer's name is a dirent name and its
value is a struct xfs_parent_rec, so the leaf entry is local and at most
round_up(3 + 255 + 12, 4) = 272 bytes.  Parent pointers require V5 and
therefore XFS_MIN_CRC_BLOCKSIZE, so the smallest half-block this can be
compared against is 512 and the double split branch is unreachable on
every mountable geometry.  Locality is decided against a different
threshold, xfs_attr_leaf_entsize_local_max() at three quarters of a
block, which the 272 bytes also clears.

Record the other difference too.  The second term hands a byte count to
XFS_NEXTENTADD_SPACE_RES(), whose parameter counts mappings, so it asks
for more extent-add allowance than the one mapping a parent pointer
adds.  The factor depends on the block size, because the macro divides
by XFS_MAX_CONTIG_EXTENTS_PER_BLOCK(), so the comment says only that it
over-reserves - a patch whose whole point is that the old comment stated
a geometry-dependent thing as invariant should not do the same.  That it
over-reserves is why it is not a bug and why this patch leaves it alone.

Signed-off-by: Javier Tia <floss@jetm.me>
---
 fs/xfs/libxfs/xfs_trans_space.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_trans_space.c b/fs/xfs/libxfs/xfs_trans_space.c
index 9b8f495c9049..c4cd547033e5 100644
--- a/fs/xfs/libxfs/xfs_trans_space.c
+++ b/fs/xfs/libxfs/xfs_trans_space.c
@@ -22,8 +22,23 @@ xfs_parent_calc_space_res(
 	unsigned int		namelen)
 {
 	/*
-	 * Parent pointers are always the first attr in an attr tree, and never
-	 * larger than a block
+	 * A parent pointer is recorded per dirent, so an inode with N links
+	 * carries N of them and the attr fork can already be in leaf or node
+	 * format when one is added.  That does not affect the reservation:
+	 * XFS_DAENTER_SPACE_RES covers a split at every level of a
+	 * maximum-depth attr dabtree, whatever format the fork is in now.
+	 *
+	 * The name is a dirent name and the value is a struct xfs_parent_rec,
+	 * so the leaf entry is always local and never exceeds 272 bytes.
+	 * Parent pointers require V5, hence a 1k minimum block size, so the
+	 * entry always stays under half a block and this needs none of the
+	 * double split allowance that xfs_attr_calc_size() makes.
+	 *
+	 * The second term hands a byte count to a macro whose parameter counts
+	 * mappings, so it asks for more extent-add allowance than the single
+	 * mapping a parent pointer adds - how much more depends on the block
+	 * size.  It over-reserves either way, which is why it is left alone:
+	 * correcting the unit would shrink a reservation that is only generous.
 	 */
 	return XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK) +
 	       XFS_NEXTENTADD_SPACE_RES(mp, namelen, XFS_ATTR_FORK);
-- 
Javier Tia