Re: [PATCH 04/16] xfs: reflect sb features in xfs_mount
From: Dave Chinner <david@fromorbit.com>
Date: 2021-07-14 23:07:34
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:quoted
From: Dave Chinner <redacted> 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 <redacted>
....
quoted
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....
quoted
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@fromorbit.com