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

Dave Chinner
2021-09-28T00:47:10Z

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