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

Jeff Layton <[email protected]>
Newsgroups gmane.comp.file-systems.btrfs,gmane.linux.file-systems,gmane.linux.kernel,gmane.linux.kernel.mm,gmane.linux.ports.ppc64.devel,gmane.linux.drivers.platform.x86.devel,gmane.linux.drivers.rdma,gmane.linux.serial,gmane.linux.usb.general,gmane.linux.kernel.autofs,gmane.comp.file-systems.ceph.devel,gmane.comp.file-systems.coda.general,gmane.linux.kernel.efi,gmane.comp.file-systems.ext4,gmane.linux.file-systems.f2fs,gmane.linux.uml.devel,gmane.linux.drivers.mtd,gmane.linux.nfs,gmane.comp.file-systems.nilfs.user,gmane.linux.file-systems.ntfs.devel,gmane.linux.hardware.karma.devel,gmane.linux.file-systems.union,gmane.comp.file-systems.reiserfs.general,gmane.linux.kernel.cifs,gmane.network.samba.internals,gmane.linux.kernel.bpf,gmane.linux.network,gmane.comp.security.apparmor,gmane.linux.kernel.lsm
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.