Re: [PATCH V3 08/12] xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits respectively

[Dave Chinner <david@fromorbit.com>]

On Thu, Sep 16, 2021 at 03:36:43PM +0530, Chandan Babu R wrote:
> A future commit will introduce a 64-bit on-disk data extent counter and a
> 32-bit on-disk attr extent counter. This commit promotes xfs_extnum_t and
> xfs_aextnum_t to 64 and 32-bits in order to correctly handle in-core versions
> of these quantities.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>

So while I was auditing extent lengths w.r.t. the last patch f the
series, I noticed that xfs_extnum_t is used in the struct
xfs_log_dinode and so changing the size of these types changes the
layout of this structure:

/*
 * Define the format of the inode core that is logged. This structure must be
 * kept identical to struct xfs_dinode except for the endianness annotations.
 */
struct xfs_log_dinode {
....
        xfs_rfsblock_t  di_nblocks;     /* # of direct & btree blocks used */
        xfs_extlen_t    di_extsize;     /* basic/minimum extent size for file */
        xfs_extnum_t    di_nextents;    /* number of extents in data fork */
        xfs_aextnum_t   di_anextents;   /* number of extents in attribute fork*/
....

Which means this:

> -typedef int32_t		xfs_extnum_t;	/* # of extents in a file */
> -typedef int16_t		xfs_aextnum_t;	/* # extents in an attribute fork */
> +typedef uint64_t	xfs_extnum_t;	/* # of extents in a file */
> +typedef uint32_t	xfs_aextnum_t;	/* # extents in an attribute fork */

creates an incompatible log format change that will cause silent
inode corruption during log recovery if inodes logged with this
change are replayed on an older kernel without this change. It's not
just the type size change that matters here - it also changes the
implicit padding in this structure because xfs_extlen_t is a 32 bit
object and so:

Old					New
64 bit object (di_nblocks)		64 bit object (di_nblocks)
32 bit object (di_extsize)		32 bit object (di_extsize)
					32 bit pad (implicit)
32 bit object (di_nextents)		64 bit object (di_nextents)
16 bit object (di_anextents)		32 bit ojecct (di_anextents
8 bit object (di_forkoff)		8 bit object (di_forkoff)
8 bit object (di_aformat)		8 bit object (di_aformat)
					16 bit pad (implicit)
32 bit object (di_dmevmask)		32 bit object (di_dmevmask)


That's quite the layout change, and that's something we must not do
without a feature bit being set. hence I think we need to rev the
struct xfs_log_dinode version for large extent count support, too,
so that the struct xfs_log_dinode does not change size for
filesystems without the large extent count feature.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com