On Wed, Feb 08, 2012 at 01:08:47PM -0500, Ted Ts'o wrote: > So I've started looking at this patch series, and I'm wondering if it > might be better if we collapse these two feature flags: > > EXT4_FEATURE_RO_COMPAT_METADATA_CSUM > and > EXT4_FEATURE_INCOMPAT_BG_USE_META_CSUM > > To a single feature flag, EXT4_FEATURE_INCOMPAT_METADATA_CSUM, which > also implies EXT4_FEATURE_RO_COMPAT_GDT_CSUM. (So in the future, when > we enable INCOMPAT_CSUM_METADATA, the presense or absence of > EXT4_RO_COMPAT_GDT_CSUM won't matter, and in fact mke2fs will skip > setting that feature flag altogether. Tune2fs could also drop the > GDT_CSUM flag when adding the CSUM_METADATA flag.) I'm ok with this. > The reasoning behind this is that it simplifies the combinatorics we > need to test, and it also simplifies our code base. In addition, it's > really easy to make tune2fs recalculate the checksums when the feature > flag is set, and reculate the block group checksums using the old > algorithm when the metadata flag is unset. So if someone wants to > mount the file system on a downlevel kernel, it's really not that bad > that this feature is an INCOMPAT feature; we can easily downgrade it > using tune2fs. > > In fact, if we wanted to take this to extremes, we could call it > EXT4_FEATURE_INCOMPAT_NEW_METADATA, and then let it imply the > following feature flags as well: > > EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER > EXT4_FEATURE_RO_COMPAT_LARGE_FILE > EXT4_FEATURE_RO_COMPAT_HUGE_FILE > EXT4_FEATURE_RO_COMPAT_DIR_NLINK > EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE > EXT4_FEATURE_INCOMPAT_FILETYPE > EXT4_FEATURE_INCOMPAT_FLEX_BG > > We can add inline functions in fs/ext4/ext4.h and in > lib/ext2fs/ext2fs.h to make the source code look a bit simpler. > > This would help to reduce our testing load, and it would also make the > output of dumpe2fs easier to understand... Hm... does this make it impossible to add checksums to a fs that doesn't have flexbg set? I'm not 100% sure what happens if you enable the flexbg bit on a filesystem that wasn't mkfs'd with that turned on. Most of the other flags look like they've been mkfs default for years, but I could be wrong. I'm concerned that implementing this second idea would make it more difficult to add checksums to an older filesystem. --D > > - Ted > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
