On Wed, Jul 14, 2021 at 03:56:03PM -0700, Darrick J. Wong wrote:
> On Wed, Jul 14, 2021 at 02:19:00PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Currently on-disk feature checks require decoding the superblock
> > fileds and so can be non-trivial. We have almost 400 hundred
> > individual feature checks in the XFS code, so this is a significant
> > amount of code. To reduce runtime check overhead, pre-process all
> > the version flags into a features field in the xfs_mount at mount
> > time so we can convert all the feature checks to a simple flag
> > check.
> >
> > There is also a need to convert the dynamic feature flags to update
> > the m_features field. This is required for attr, attr2 and quota
> > features. New xfs_mount based wrappers are added for this.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
....
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 5db3fb184fbe..488f472cedba 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -3315,6 +3315,7 @@ xlog_do_recover(
> > xfs_buf_relse(bp);
> >
> > /* re-initialise in-core superblock and geometry structures */
> > + mp->m_features |= xfs_sb_version_to_features(sbp);
>
> '|=' instead of '=' ?
Yes, that is intended.
> I would have expected assignment, but I guess the assumption here is
> that log recovery can process a sb update that adds a feature? And that
> log recovery won't be turning off features?
Right, we can add but we should never remove on-disk feature bits at
runtime. Getting rid of the noattr2 shenanigans dropped the only
case where we remove feature bits at runtime. Also, see below....
> > xfs_reinit_percpu_counters(mp);
> > error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
> > if (error) {
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 6be2a1c5b0f4..0ec463d91cce 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -225,6 +225,7 @@ xfs_readsb(
> > goto reread;
> > }
> >
> > + mp->m_features |= xfs_sb_version_to_features(sbp);
>
> Also, can't this be a plain assignment?
Nope, because a couple of patches further down the series,
mp->m_features will already contain all the mount features that have
been set and we do not want to overwrite them.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
