On Fri, 2022-09-09 at 08:11 -0400, Theodore Ts'o wrote: > On Thu, Sep 08, 2022 at 01:40:11PM -0400, Jeff Layton wrote: > > > > Ted, how would we access this? Maybe we could just add a new (generic) > > super_block field for this that ext4 (and other filesystems) could > > populate at mount time? > > Yeah, I was thinking about just adding it to struct super, with some > value (perhaps 0 or ~0) meaning that the file system didn't support > it. If people were concerned about struct super bloat, we could also > add some new function to struct super_ops that would return one or > more values that are used rarely by most of the kernel code, and so > doesn't need to be in the struct super data structure. I don't have > strong feelings one way or another. > Either would be fine, I think. > On another note, my personal opinion is that at least as far as ext4 > is concerned, i_version on disk's only use is for NFS's convenience, Technically, IMA uses it too, but it needs the same behavior as NFSv4. > and so I have absolutely no problem with changing how and when > i_version gets updated modulo concerns about impacting performance. > That's one of the reasons why being able to update i_version only > lazily, so that if we had, say, some workload that was doing O_DIRECT > writes followed by fdatasync(), there wouldn't be any obligation to > flush the inode out to disk just because we had bumped i_version > appeals to me. > i_version only changes now if someone has queried it since it was last changed. That makes a huge difference in performance. We can try to optimize it further, but it probably wouldn't move the needle much under real workloads. > But aside from that, I don't consider when i_version gets updated on > disk, especially what the semantics are after a crash, and if we need > to change things so that NFS can be more performant, I'm happy to > accomodate. One of the reasons why we implemented the ext4 fast > commit feature was to improve performance for NFS workloads. > > I know some XFS developers have some concerns here, but I just wanted > to make it be explicit that (a) I'm not aware of any users who are > depending on the i_version on-disk semantics, and (b) if they are > depending on something which as far as I'm concerned in an internal > implementation detail, we've made no promises to them, and they can > get to keep both pieces. :-) This is especially since up until now, > there is no supported, portable userspace interface to make i_version > available to userspace. > Great! That's what I was hoping for with ext4. Would you be willing to pick up these two patches for v6.1? https://lore.kernel.org/linux-ext4/20220908172448.208585-3-jlayton@xxxxxxxxxx/T/#u https://lore.kernel.org/linux-ext4/20220908172448.208585-4-jlayton@xxxxxxxxxx/T/#u They should be able to go in independently of the rest of the series and I don't forsee any big changes to them. Thanks, -- Jeff Layton <jlayton@xxxxxxxxxx>
