On Tue, Dec 13, 2022 at 05:55:22PM +0000, Matthew Wilcox wrote: > On Tue, Dec 13, 2022 at 06:29:26PM +0100, Andrey Albershteyn wrote: > > Add wrapper to clear mapping's large folio flag. This is handy for > > disabling large folios on already existing inodes (e.g. future XFS > > integration of fs-verity). > > I have two problems with this. One is your use of __clear_bit(). > We can use __set_bit() because it's done as part of initialisation. > As far as I can tell from your patches, mapping_clear_large_folios() is > called on a live inode, so you'd have to use clear_bit() to avoid races. I think we can do without mapping_clear_large_folios() - we already have precedence for this sort of mapping state change with the DAX inode feature flag. That is, we change the on-disk state in the ioctl context, but we don't change the in-memory inode state. Instead, we mark it I_DONTCACHEi to get it turfed from memory with expediency. Then when it is re-instantiated, we see the on-disk state and then don't enable large mappings on that inode. That will work just fine here, I think. > The second is that verity should obviously be enhanced to support > large folios (and for that matter, block sizes smaller than PAGE_SIZE). > Without that, this is just a toy or a prototype. Disabling large folios > is not an option. Disabling large folios is very much an option. Filesystems must opt in to large mapping support, so they can also choose to opt out. i.e. large mappings is a filesystem policy decision, not a core infrastructure decision. Hence how we disable large mappings for fsverity enabled inodes is open to discussion, but saying we can't opt out of an optional feature is entirely non-sensical. > I'm happy to work with you to add support for large folios to verity. > It hasn't been high priority for me, but I'm now working on folio support > for bufferhead filesystems and this would probably fit in. Yes, we need fsverity to support multipage folios, but modifying fsverity is outside the scope of initially enabling fsverity support on XFS. This patch set is about sorting out the on-disk format changes and interfacing with the fsverity infrastructure to enable the feature to be tested and verified. Stuff like large mapping support in fsverity is a future concern, not a show-stopper for initial feature support. We don't need every bell and whistle in the initial merge.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
