On Apr 6, 2019, at 3:52 PM, Theodore Ts'o <tytso@xxxxxxx> wrote: > > On Fri, Mar 22, 2019 at 03:24:27AM -0600, Andreas Dilger wrote: >> From: Andreas Dilger <adilger@xxxxxxxxxxxxx> >> >> The 64bit feature should be allowed without extents to for 32-bit >> metadata_csum checksums to be stored in the group descriptor. >> Change the extents check to check for more than 2^32 blocks instead >> of the 64bit feature flag. This also avoids warnings later if the >> metadata_csum feature is enabled on a filesystem without 64bit. > > So what worries me about this change is if extents aren't enabled, and > we do an online (or off-line) resize such that we now have > 2^32 > blocks, things are going to get problematic. Even if the resize > operation sets the extent feature (which I don't think it will), the > problem is if you have an existing file which is using indirect > blocks, and you try to extend the file and there is simply no blocks > under the 2^32 cutoff, what then? Some files might get ENOSPC errors > while others will work just fine. > > With current versions of e2fsprogs we enable the 64-bit (and > metadata_csum) feature by default, which should avoid all of these > problems. Sure, that's not going to help older file systems --- but > this patch to mke2fs isn't going to help them, either. > > It simplifies our test matrix to simply require the extents feature if > the 64-bit feature is desired. I'm not sure I see the value in > relaxing this requirement. Is there a reason why you specifically > want 64bit && !extents && metadata_csum? Mostly we want to enable metadata_csum on Lustre metadata targets, which are typically always under 16TB in size, and since the MDT doesn't store much file data, it is more efficient to use block-mapped directories rather than extent-mapped, since directory blocks are typically allocated randomly, so extents are not really useful or needed. Currently the "64bit" feature is needed to get large group_descriptor entries to store the 32-bit metadata_csum fields to enable metadata_csum, which is the reason for this patch. If it were possible to enable 64-byte struct ext4_group_desc without the 64bit feature so that metadata_csum could store 32-bit checksums, that would be another solution instead of requiring 64bit to be enabled without extents. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
