Re: [PATCH v1 03/23] ceph: Switch to use %ptSp

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Mon, 2025-11-10 at 19:40 +0100, Andy Shevchenko wrote:
> Use %ptSp instead of open coded variants to print content of
> struct timespec64 in human readable format.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  fs/ceph/dir.c   |  5 ++---
>  fs/ceph/inode.c | 47 +++++++++++++++--------------------------------
>  fs/ceph/xattr.c |  6 ++----
>  3 files changed, 19 insertions(+), 39 deletions(-)
> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index d18c0eaef9b7..bf50c6e7a029 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -2155,7 +2155,7 @@ static ssize_t ceph_read_dir(struct file *file, char __user *buf, size_t size,
>  				" rfiles:   %20lld\n"
>  				" rsubdirs: %20lld\n"
>  				"rbytes:    %20lld\n"
> -				"rctime:    %10lld.%09ld\n",
> +				"rctime:    %ptSp\n",
>  				ci->i_files + ci->i_subdirs,
>  				ci->i_files,
>  				ci->i_subdirs,
> @@ -2163,8 +2163,7 @@ static ssize_t ceph_read_dir(struct file *file, char __user *buf, size_t size,
>  				ci->i_rfiles,
>  				ci->i_rsubdirs,
>  				ci->i_rbytes,
> -				ci->i_rctime.tv_sec,
> -				ci->i_rctime.tv_nsec);
> +				&ci->i_rctime);
>  	}
>  
>  	if (*ppos >= dfi->dir_info_len)
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 37d3a2477c17..6db8951d79e0 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -879,7 +879,9 @@ void ceph_fill_file_time(struct inode *inode, int issued,
>  {
>  	struct ceph_client *cl = ceph_inode_to_client(inode);
>  	struct ceph_inode_info *ci = ceph_inode(inode);
> +	struct timespec64 iatime = inode_get_atime(inode);
>  	struct timespec64 ictime = inode_get_ctime(inode);
> +	struct timespec64 imtime = inode_get_mtime(inode);
>  	int warn = 0;
>  
>  	if (issued & (CEPH_CAP_FILE_EXCL|
> @@ -889,39 +891,26 @@ void ceph_fill_file_time(struct inode *inode, int issued,
>  		      CEPH_CAP_XATTR_EXCL)) {
>  		if (ci->i_version == 0 ||
>  		    timespec64_compare(ctime, &ictime) > 0) {
> -			doutc(cl, "ctime %lld.%09ld -> %lld.%09ld inc w/ cap\n",
> -			     ictime.tv_sec, ictime.tv_nsec,
> -			     ctime->tv_sec, ctime->tv_nsec);
> +			doutc(cl, "ctime %ptSp -> %ptSp inc w/ cap\n", &ictime, ctime);
>  			inode_set_ctime_to_ts(inode, *ctime);
>  		}
>  		if (ci->i_version == 0 ||
>  		    ceph_seq_cmp(time_warp_seq, ci->i_time_warp_seq) > 0) {
>  			/* the MDS did a utimes() */
> -			doutc(cl, "mtime %lld.%09ld -> %lld.%09ld tw %d -> %d\n",
> -			     inode_get_mtime_sec(inode),
> -			     inode_get_mtime_nsec(inode),
> -			     mtime->tv_sec, mtime->tv_nsec,
> +			doutc(cl, "mtime %ptSp -> %ptSp tw %d -> %d\n", &imtime, mtime,
>  			     ci->i_time_warp_seq, (int)time_warp_seq);
>  
>  			inode_set_mtime_to_ts(inode, *mtime);
>  			inode_set_atime_to_ts(inode, *atime);
>  			ci->i_time_warp_seq = time_warp_seq;
>  		} else if (time_warp_seq == ci->i_time_warp_seq) {
> -			struct timespec64	ts;
> -
>  			/* nobody did utimes(); take the max */
> -			ts = inode_get_mtime(inode);
> -			if (timespec64_compare(mtime, &ts) > 0) {
> -				doutc(cl, "mtime %lld.%09ld -> %lld.%09ld inc\n",
> -				     ts.tv_sec, ts.tv_nsec,
> -				     mtime->tv_sec, mtime->tv_nsec);
> +			if (timespec64_compare(mtime, &imtime) > 0) {
> +				doutc(cl, "mtime %ptSp -> %ptSp inc\n", &imtime, mtime);
>  				inode_set_mtime_to_ts(inode, *mtime);
>  			}
> -			ts = inode_get_atime(inode);
> -			if (timespec64_compare(atime, &ts) > 0) {
> -				doutc(cl, "atime %lld.%09ld -> %lld.%09ld inc\n",
> -				     ts.tv_sec, ts.tv_nsec,
> -				     atime->tv_sec, atime->tv_nsec);
> +			if (timespec64_compare(atime, &iatime) > 0) {
> +				doutc(cl, "atime %ptSp -> %ptSp inc\n", &iatime, atime);
>  				inode_set_atime_to_ts(inode, *atime);
>  			}
>  		} else if (issued & CEPH_CAP_FILE_EXCL) {
> @@ -2703,10 +2692,8 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode,
>  	if (ia_valid & ATTR_ATIME) {
>  		struct timespec64 atime = inode_get_atime(inode);
>  
> -		doutc(cl, "%p %llx.%llx atime %lld.%09ld -> %lld.%09ld\n",
> -		      inode, ceph_vinop(inode),
> -		      atime.tv_sec, atime.tv_nsec,
> -		      attr->ia_atime.tv_sec, attr->ia_atime.tv_nsec);
> +		doutc(cl, "%p %llx.%llx atime %ptSp -> %ptSp\n",
> +		      inode, ceph_vinop(inode), &atime, &attr->ia_atime);
>  		if (!do_sync && (issued & CEPH_CAP_FILE_EXCL)) {
>  			ci->i_time_warp_seq++;
>  			inode_set_atime_to_ts(inode, attr->ia_atime);
> @@ -2780,10 +2767,8 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode,
>  	if (ia_valid & ATTR_MTIME) {
>  		struct timespec64 mtime = inode_get_mtime(inode);
>  
> -		doutc(cl, "%p %llx.%llx mtime %lld.%09ld -> %lld.%09ld\n",
> -		      inode, ceph_vinop(inode),
> -		      mtime.tv_sec, mtime.tv_nsec,
> -		      attr->ia_mtime.tv_sec, attr->ia_mtime.tv_nsec);
> +		doutc(cl, "%p %llx.%llx mtime %ptSp -> %ptSp\n",
> +		      inode, ceph_vinop(inode), &mtime, &attr->ia_mtime);
>  		if (!do_sync && (issued & CEPH_CAP_FILE_EXCL)) {
>  			ci->i_time_warp_seq++;
>  			inode_set_mtime_to_ts(inode, attr->ia_mtime);
> @@ -2804,13 +2789,11 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode,
>  
>  	/* these do nothing */
>  	if (ia_valid & ATTR_CTIME) {
> +		struct timespec64 ictime = inode_get_ctime(inode);
>  		bool only = (ia_valid & (ATTR_SIZE|ATTR_MTIME|ATTR_ATIME|
>  					 ATTR_MODE|ATTR_UID|ATTR_GID)) == 0;
> -		doutc(cl, "%p %llx.%llx ctime %lld.%09ld -> %lld.%09ld (%s)\n",
> -		      inode, ceph_vinop(inode),
> -		      inode_get_ctime_sec(inode),
> -		      inode_get_ctime_nsec(inode),
> -		      attr->ia_ctime.tv_sec, attr->ia_ctime.tv_nsec,
> +		doutc(cl, "%p %llx.%llx ctime %ptSp -> %ptSp (%s)\n",
> +		      inode, ceph_vinop(inode), &ictime, &attr->ia_ctime,
>  		      only ? "ctime only" : "ignored");
>  		if (only) {
>  			/*
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 537165db4519..ad1f30bea175 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -249,8 +249,7 @@ static ssize_t ceph_vxattrcb_dir_rbytes(struct ceph_inode_info *ci, char *val,
>  static ssize_t ceph_vxattrcb_dir_rctime(struct ceph_inode_info *ci, char *val,
>  					size_t size)
>  {
> -	return ceph_fmt_xattr(val, size, "%lld.%09ld", ci->i_rctime.tv_sec,
> -				ci->i_rctime.tv_nsec);
> +	return ceph_fmt_xattr(val, size, "%ptSp", &ci->i_rctime);
>  }
>  
>  /* dir pin */
> @@ -307,8 +306,7 @@ static bool ceph_vxattrcb_snap_btime_exists(struct ceph_inode_info *ci)
>  static ssize_t ceph_vxattrcb_snap_btime(struct ceph_inode_info *ci, char *val,
>  					size_t size)
>  {
> -	return ceph_fmt_xattr(val, size, "%lld.%09ld", ci->i_snap_btime.tv_sec,
> -				ci->i_snap_btime.tv_nsec);
> +	return ceph_fmt_xattr(val, size, "%ptSp", &ci->i_snap_btime);
>  }
>  
>  static ssize_t ceph_vxattrcb_cluster_fsid(struct ceph_inode_info *ci,

Looks good. Nice cleanup.

Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

Thanks,
Slava.