On Tue, 2024-07-02 at 08:12 -0700, Christoph Hellwig wrote: > On Tue, Jul 02, 2024 at 08:21:42AM -0400, Jeff Layton wrote: > > Many of the existing callers of inode_ctime_to_ts are in void > > return > > functions. They're just copying data from an internal > > representation to > > struct inode and assume it always succeeds. For those we'll > > probably > > have to catch bad ctime values earlier. > > > > So, I think I'll probably have to roll bespoke error handling in > > all of > > the relevant filesystems if we go this route. There are also > > differences between filesystems -- does it make sense to refuse to > > load > > an inode with a bogus ctime on NFS or AFS? Probably not. > > > > Hell, it may be simpler to just ditch this patch and reimplement > > mgtimes using the nanosecond fields like the earlier versions did. > > Thatdoes for sure sound simpler. What is the big advantage of the > ktime_t? Smaller size? > Yeah, mostly. We shrink struct inode by 8 bytes with that patch, and we (probably) get a better cache footprint, since i_version ends up in the same cacheline as the ctime. That's really a separate issue though, so I'm not too worked up about dropping that patch. As a bonus, leaving it split across separate fields means that we can use unused bits in the nsec field for the flag, so we don't need to sacrifice any timestamp granularity either. I've got a draft rework that does this that I'm testing now. Assuming it works OK, I'll resend in a few days. Thanks for the feedback! -- Jeff Layton <jlayton@xxxxxxxxxx>
