On Sun, Jul 04, 2021 at 06:47:22PM +0800, Eryu Guan wrote: > > I think that depends on if we treat it as a feature or a bugfix. We > should let the test fail on old kernels if that's a bugfix, and skip the > test if it's a feature. Apparently, we take it as a feature here. Yes, it's clearly a feature. > > feature, and landed just a little bit after filename wipe feature, so > > use support for the journal checkpoint ioctl as a proxy for support > > for the filename wipe feature. > > This seems fragile, they're related features but not always bounded > together, I think it's possible for distros developers decide to > backport only one of the features (though quite unlikely). Given the discussion for the patches, I suspect that it will be only the distros that are meant for Cloud environments (e.g., Google's COS, maybe Amazon Linux, etc.) that will be interested at all in the first place --- and if they do, they'll want to take both of them. That's because it's only in a Cluod environment we can make specific guarantees about how discard works, and it's mostly in a Cloud environment where you have customers which appear to be the most concerned about being able to certify to their auditors that PII can be reliably wiped after they "lift and shift" from their private data centers to third-party operated hyperscale Cloud providers. > Is it worth to introduce features sysfs entry for journal checkpoint and > filename wipe in ext4? In fstests' point of view, it's great to have > such entries that could be checked. If it's not worth it, I think the > "feature proxy" way is acceptable, given that the two features are > closely related and aimed to the same security issue. Well, we didn't when the patch first landed, so even if we did add the features sysfs file, there's no guarantee that distros doing the backport will pick up the patch to add the features file. The sysfs files do take up a few hundred bytes each, and so there is some resistance in adding a lot of sysfs features, saving them for major features. For better or for worse, one of the principles of sysfs is to _not_ require any parsing, such that something like /proc/stat would have to be replaced by something like individual 1,000+ sysfs files (try running wc -w /proc/stat), depending on how cpu cores and interrupt channels the system has. This is why we are using separate sysfs files for each feature, instead of a /proc file containing a list of features which fstests could test by checking the exit status of "grep ^feature_name$ /proc/fs/ext4/feature-list". But in retrospect, maybe we should have gone down that path, since adding features to a feature-list pseudo-file is essentially free, and would make life much more convenient for fstests --- right now we do a lot of "create a scratch file system; try to mount it and see if it succeed or fails, which is a lot of needless church and extra write cycles on the test device, and takes more time than just running "grep" on a festure-list file. Alas, sysfs was considered the new hotness, and the superior approach, while /proc was considered the legacy, hacky approach. Ah, well... Cheers, - Ted
