[PATCH 0/10] xfs: feature flag rework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi folks,

This series reworks how the kernel code handles feature bits. It
seemed to me to be a bit crazy to repeatedly have to check multiple
values in the superblock to check whether a feature is supported.
e.g. to check if reflink is available, we first mask the version
number to extract it from the superblock field, then we check it
against the v5 value, then we load and check the feature bit from
the superblock.

This could all just be a single flag check - a flag that is set at
mount time after doing all of the above, and so it's just a single
operation to check.

I'm also tired of typing "xfs_sb_version_hasfoo" everytime a check
needs to be done. Not to mention having a different mechanism to
check mount option based features.

And then there's the conflation of mount option feature flags and
runtime state in the one set of flags. The flags field is not
updated atomically, so there no guarantee that a change of a state
flag does race with anything else and we lose it.

So this patch set moves all the feature flags into a new m_features
field in the struct xfs_mount. It doesn't matter if they are mount
options or superblock feature bits - they all end up in the one
place whether they are [almost entirely] read only. These are now
checked by "xfs_has_foo(mp)" functions (following the ext4 feature
check syntax), and for the very few features that can be dynamically
added or removed there are also xfs_feat_add/remove_foo(mp) helpers.
IOWs, both superblock and mount option features are checked the same
way.

The state flags (clean mount, unmounting, shutdown, etc) are all
moved to a new m_state field, which is manipulated entirely by
atomic bitops to avoid update races. THere are a couple of wrappers
for the common checks - xfs_is_read_only(mp) and
xfs_is_shut_down(mp). The latter replaces the XFS_FORCED_SHUTDOWN()
macro.

There's a bit of extra work around the attr2 feature bit - we've got
the superblock bit, and then "attr2" and "noattr2" mount options and
somewhat convoluted interactions. This patchset changes the
behaviour of all these now - the attr2 bit on disk and mount option
now mean the same thing, and noattr2 will turn off the superblock
bit if it is set and prevent attr2 inodes from being created. This
simplifies the mount time checking and clearing of these features.

Also, it gives us a clean separation of "inode32 mount option" and
"inode32 allocation is active". The former is a feature bit (i.e.
SMALL_INUMS), and the latter is a state bit (32BITINODES) that is
set if the filesystem size requires the allocator to limit
allocation to 32 bit inode space.

This also allows us to get rid of all the xfs_sb_version_hasfoo()
inline functions. THere is a single function to populate m_features
from the superblock bits, and the low level checks that are purely
superblock based (i.e. on disk conversion and verifiers) directly
check the superblock fields rather than use wrappers.

Overall, this doesn't change the amount of code. If does reduce the
built binary size by about 3.2kB, mainly through replacing the
xfs_sb_version...() checks with a single m_features flag check. It
makes the code a bit simpler, easier to read, and less shouty and
separates runtime state from filesystem features.

-Dave.





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux