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.