On Thu, 2022-06-16 at 16:03 +1000, Dave Chinner wrote:
> On Sat, Jun 11, 2022 at 02:41:57AM -0700, Allison Henderson wrote:
> > [dchinner: forward ported and cleaned up]
> > [achender: rebased and added parent pointer attribute to
> > compatible attributes mask]
> >
> > Signed-off-by: Mark Tinguely <tinguely@xxxxxxx>
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_format.h | 14 +++++++++-----
> > fs/xfs/libxfs/xfs_fs.h | 1 +
> > fs/xfs/libxfs/xfs_sb.c | 2 ++
> > fs/xfs/xfs_super.c | 4 ++++
> > 4 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_format.h
> > b/fs/xfs/libxfs/xfs_format.h
> > index 96976497306c..e85d6b643622 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -83,6 +83,7 @@ struct xfs_ifork;
> > #define XFS_SB_VERSION2_OKBITS \
> > (XFS_SB_VERSION2_LAZYSBCOUNTBIT | \
> > XFS_SB_VERSION2_ATTR2BIT | \
> > + XFS_SB_VERSION2_PARENTBIT | \
> > XFS_SB_VERSION2_PROJID32BIT | \
> > XFS_SB_VERSION2_FTYPE)
>
> No need for a v4 filesystem format feature bit - this is v4 only.
Ok, I ended up having to add this in the rebase or we get an "SB
validate failed". I think it has to go over in
xfs_sb_validate_v5_features next to the manual crc bit check. Will
move
>
> >
> > @@ -353,11 +354,13 @@ xfs_sb_has_compat_feature(
> > #define XFS_SB_FEAT_RO_COMPAT_RMAPBT (1 << 1) /*
> > reverse map btree */
> > #define XFS_SB_FEAT_RO_COMPAT_REFLINK (1 << 2) /*
> > reflinked files */
> > #define XFS_SB_FEAT_RO_COMPAT_INOBTCNT (1 << 3) /*
> > inobt block counts */
> > +#define XFS_SB_FEAT_RO_COMPAT_PARENT (1 << 4) /*
> > parent inode ptr */
> > #define XFS_SB_FEAT_RO_COMPAT_ALL \
> > - (XFS_SB_FEAT_RO_COMPAT_FINOBT | \
> > - XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
> > - XFS_SB_FEAT_RO_COMPAT_REFLINK| \
> > - XFS_SB_FEAT_RO_COMPAT_INOBTCNT)
> > + (XFS_SB_FEAT_RO_COMPAT_FINOBT | \
> > + XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
> > + XFS_SB_FEAT_RO_COMPAT_REFLINK | \
> > + XFS_SB_FEAT_RO_COMPAT_INOBTCNT| \
> > + XFS_SB_FEAT_RO_COMPAT_PARENT)
>
> I'm not sure this is a RO Compat feature - we added an attribute
> namespace flag on disk, and the older kernels do not know about
> that (i.e. we changed XFS_ATTR_NSP_ONDISK_MASK). This may result in
> parent pointer attrs being exposed as user attrs rather than being
> hidden, or maybe parent pointer attrs being seen as corrupt because
> they have a flag that isn't defined set, etc.
>
> Hence I'm not sure that this classification is correct.
Gosh, I'm sure there was a reason we did this, but what ever it was
goes all the way back in the first re-appearance of the set back in
2018 and I just cant remember the discussion at the time. It may have
just been done to get mkfs working and we just never got to reviewing
it.
Should we drop it and just use XFS_SB_VERSION2_PARENTBIT?
>
> > #define XFS_SB_FEAT_RO_COMPAT_UNKNOWN ~XFS_SB_FEAT_RO_COMPAT_
> > ALL
> > static inline bool
> > xfs_sb_has_ro_compat_feature(
> > @@ -392,7 +395,8 @@ xfs_sb_has_incompat_feature(
> >
> > static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
> > {
> > - return false; /* We'll enable this at the end of the set */
> > + return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
> > + (sbp->sb_features_ro_compat &
> > XFS_SB_FEAT_RO_COMPAT_PARENT));
> > }
>
> This should go away and the feature bit in the mount get set by
> xfs_sb_version_to_features().
>
Alrighty
> > #define XFS_SB_FEAT_INCOMPAT_LOG_XATTRS (1 << 0) /* Delayed
> > Attributes */
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 1cfd5bc6520a..b0b4d7a3aa15 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -237,6 +237,7 @@ typedef struct xfs_fsop_resblks {
> > #define XFS_FSOP_GEOM_FLAGS_BIGTIME (1 << 21) /* 64-bit
> > nsec timestamps */
> > #define XFS_FSOP_GEOM_FLAGS_INOBTCNT (1 << 22) /* inobt
> > btree counter */
> > #define XFS_FSOP_GEOM_FLAGS_NREXT64 (1 << 23) /* large
> > extent counters */
> > +#define XFS_FSOP_GEOM_FLAGS_PARENT (1 << 24) /* parent pointers
> > */
> >
> > /*
> > * Minimum and maximum sizes need for growth checks.
> > diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> > index a20cade590e9..d90b05456dba 100644
> > --- a/fs/xfs/libxfs/xfs_sb.c
> > +++ b/fs/xfs/libxfs/xfs_sb.c
> > @@ -1187,6 +1187,8 @@ xfs_fs_geometry(
> > geo->flags |= XFS_FSOP_GEOM_FLAGS_BIGTIME;
> > if (xfs_has_inobtcounts(mp))
> > geo->flags |= XFS_FSOP_GEOM_FLAGS_INOBTCNT;
> > + if(xfs_sb_version_hasparent(sbp))
>
> if (xfs_has_parent_pointers(mp))
>
Will update
> > + geo->flags |= XFS_FSOP_GEOM_FLAGS_PARENT;
> > if (xfs_has_sector(mp)) {
> > geo->flags |= XFS_FSOP_GEOM_FLAGS_SECTOR;
> > geo->logsectsize = sbp->sb_logsectsize;
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index a6e7b4176faf..cbb492fea4a5 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1655,6 +1655,10 @@ xfs_fs_fill_super(
> > xfs_warn(mp,
> > "EXPERIMENTAL Large extent counts feature in use. Use at your
> > own risk!");
> >
> > + if (xfs_sb_version_hasparent(&mp->m_sb))
>
> if (xfs_has_parent_pointers(mp))
>
Will update
Thanks for the reviews!
Allison
> > + xfs_alert(mp,
> > + "EXPERIMENTAL parent pointer feature enabled. Use at your own
> > risk!");
> > +
> > error = xfs_mountfs(mp);
> > if (error)
> > goto out_filestream_unmount;
> > --
> > 2.25.1
> >
> >
