On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote: > The current representation of inode times in struct inode, struct iattr, > and struct kstat use struct timespec. timespec is not y2038 safe. > > Use scalar data types (seconds and nanoseconds stored separately) to > represent timestamps in struct inode in order to maintain same size for > times across 32 bit and 64 bit architectures. > In addition, lay them out such that they are packed on a naturally > aligned boundary on 64 bit arch as 4 bytes are used to store nsec. > This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes. > This will help save RAM space as inode structure is cached in memory. > The other structures are transient and do not benefit from these > changes. IMO, this decisions sends the patch series immediately down the wrong path. TO me, this is a severe case of premature optimisation because everything gets way more complex just to save those 8 bytes, especially as those holes can be filled simply by changing the variable declaration order in the structure and adding a simple comment. And, really, I don't like those VFS_INODE_[GS]ET_XTIME macros at all; you've got to touch lots of code(*), making it all shouty and harder to read. They seem only to exist because of the above structural change requires an abstract timestamp accessor while CONFIG_FS_USES_64BIT_TIME exists. Given that goes away at the end o the series, so should the macro - if we use a struct timespec64 in the first place, it isn't even necessary as a temporary construct. (*) I note you haven't touched XFS, which means you've probably broken lots of other filesystem code. e.g. in XFS, functions like xfs_vn_getattr() and xfs_vn_update_time() access inode->i_[acm]time directly and hence are not going to compile after this patch series. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
