On Tue, 2022-08-23 at 10:05 +1000, Dave Chinner wrote: > On Thu, Aug 18, 2022 at 07:03:42AM -0400, Jeff Layton wrote: > > Dave, you keep talking about the xfs i_version counter as if there > > are > > other applications already relying on its behavior, but I don't see > > how > > that can be. There is no way for userland applications to fetch the > > counter currently. > > You miss the point entirely: the behaviour is defined by the on-disk > format (the di_changecount filed), not the applications that are > using the kernel internal iversion API. > > Just because there are no in-kernel users of the di_changecount > field in the XFS inode, it does not mean that it doesn't get used by > applications. Forensic analysis tools that walk filesystem images. > > Did you not notice that xfs_trans_log_inode() forces an iversion > update if the inode core is marked for update: > > inode_maybe_inc_iversion(inode, flags & XFS_ILOG_CORE)) > ^^^^^^^^^^^^^^^^^^^^^^ > this? > > So every modification to the inode core (which almost all inode > modifications will do, except for pure timestamp updates) will bump > the iversion regardless of whether it was queried or not. > > I use this information all the time for forensic analysis of broken > filesystem images. There are forensic tools that use expose this > information from filesystem images (e.g. xfs_db) so that we can use > it for forensic analysis. > > See the problem? On-disk format di_changecount != NFS change > attribute. We can implement the NFS change attribute with the > existing di_changecount field, but if you want to constrain the > definition of how the NFS change attribute is calculated, we can't > necessarily implement that in di_changecount without changing the > on-disk format definition. > > And, yes, this has implications for iversion being exposed to > userspace via statx(), as I've mentioned in reply to the v2 patch > you've posted. iversion is persistent information - you can't just > redefine it's behaviour without some fairly serious knock-on > effects for the subsystems that provide the persistent storage... > That still doesn't explain why a regular application would want to use that definition of a version counter. Your use case of a forensic tool is not a generic use case. It is very limited to something which needs access to detailed knowledge of the internals of your transactional filesystem. This isn't even something that needs to be exposed to the VFS layer through the i_version if the users are all going to be internal to XFS. What is being requested was initially driven (yes, over 20 years ago now, as you've pointed out) by the use case of caching applications, of which NFS is only one. There are other applications that can benefit from it. Pretty much anything that is currently using the ctime and/or mtime, wants a standard that overcomes the time resolution issues that increasingly affect storage as performance improves. That means applications such as build utilities, backup/restore utilities, search utilities, etc... Even 'git' could make use of it. However none of the above applications need the 'all metadata transactions' definition that you appear to need for your forensics use case. All will suffer a performance loss with such a definition. Worst of all, the application behaviour would vary wildly depending on the combination of mount options being used (noatime vs nodiratime vs...). So the point isn't about 'redefining the behaviour of i_version'. It is about delivering a statx() API that meets the requirements of a number of applications. If XFS is going to opt in to that API, then it needs to be able to deliver an attribute that meets the same requirements. Otherwise, it could choose to opt out. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
