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

[Jeff Layton <jlayton@kernel.org>]

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>