Re: [PATCH 86/87] fs: switch timespec64 fields in inode to discrete integers

Jeff Layton
2023-09-28T17:09:31Z

On Thu, 2023-09-28 at 07:05 -0400, Jeff Layton wrote:
> This shaves 8 bytes off struct inode, according to pahole.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> include/linux/fs.h | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 831657011036..de902ff2938b 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -671,9 +671,12 @@ struct inode {
> };
> dev_t i_rdev;
> loff_t i_size;
> - struct timespec64 __i_atime; /* use inode_*_atime accessors */
> - struct timespec64 __i_mtime; /* use inode_*_mtime accessors */
> - struct timespec64 __i_ctime; /* use inode_*_ctime accessors */
> + time64_t i_atime_sec;
> + time64_t i_mtime_sec;
> + time64_t i_ctime_sec;
> + u32 i_atime_nsec;
> + u32 i_mtime_nsec;
> + u32 i_ctime_nsec;
> spinlock_t i_lock; /* i_blocks, i_bytes, maybe i_size */
> unsigned short i_bytes;
> u8 i_blkbits;
> @@ -1519,7 +1522,9 @@ struct timespec64 inode_set_ctime_current(struct inode *inode);
> */
> static inline struct timespec64 inode_get_ctime(const struct inode *inode)
> {
> - return inode->__i_ctime;
> + struct timespec64 ts = { .tv_sec = inode->i_ctime_sec,
> + .tv_nsec = inode->i_ctime_nsec };
> + return ts;
> }
>
>
>
> /**
> @@ -1532,7 +1537,8 @@ static inline struct timespec64 inode_get_ctime(const struct inode *inode)
> static inline struct timespec64 inode_set_ctime_to_ts(struct inode *inode,
> struct timespec64 ts)
> {
> - inode->__i_ctime = ts;
> + inode->i_ctime_sec = ts.tv_sec;
> + inode->i_ctime_nsec = ts.tv_sec;

Bug above and in the other inode_set_?time_to_ts() functions. This isn't
setting the nsec field correctly.

> return ts;
> }
>
>

>
> @@ -1555,13 +1561,17 @@ static inline struct timespec64 inode_set_ctime(struct inode *inode,
>
> static inline struct timespec64 inode_get_atime(const struct inode *inode)
> {
> - return inode->__i_atime;
> + struct timespec64 ts = { .tv_sec = inode->i_atime_sec,
> + .tv_nsec = inode->i_atime_nsec };
> +
> + return ts;
> }
>
> static inline struct timespec64 inode_set_atime_to_ts(struct inode *inode,
> struct timespec64 ts)
> {
> - inode->__i_atime = ts;
> + inode->i_atime_sec = ts.tv_sec;
> + inode->i_atime_nsec = ts.tv_sec;
> return ts;
> }
>
> @@ -1575,13 +1585,17 @@ static inline struct timespec64 inode_set_atime(struct inode *inode,
>
> static inline struct timespec64 inode_get_mtime(const struct inode *inode)
> {
> - return inode->__i_mtime;
> + struct timespec64 ts = { .tv_sec = inode->i_mtime_sec,
> + .tv_nsec = inode->i_mtime_nsec };
> +
> + return ts;
> }
>
> static inline struct timespec64 inode_set_mtime_to_ts(struct inode *inode,
> struct timespec64 ts)
> {
> - inode->__i_mtime = ts;
> + inode->i_atime_sec = ts.tv_sec;
> + inode->i_atime_nsec = ts.tv_sec;

Doh! s/atime/mtime/ in the above lines.

> return ts;
> }
>

Both bugs are fixed in my tree.
--
Jeff Layton <jlayton@kernel.org>