Re: [PATCH v1 1/3] Expand the type of nfs_fattr->valid

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2025-05-15 at 10:40 -0400, Benjamin Coddington wrote:
> From: Trond Myklebust <trond.myklebust@primarydata.com>
> 
> We need to be able to track more than 32 attributes per inode.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/nfs/inode.c            |  5 ++--
>  include/linux/nfs_fs_sb.h |  2 +-
>  include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
>  3 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 1aa67fca69b2..d4e449fa076e 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -2164,10 +2164,11 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  	bool attr_changed = false;
>  	bool have_delegation;
>  
> -	dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n",
> +	dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%lx)\n",
>  			__func__, inode->i_sb->s_id, inode->i_ino,
>  			nfs_display_fhandle_hash(NFS_FH(inode)),
> -			atomic_read(&inode->i_count), fattr->valid);
> +			atomic_read(&inode->i_count),
> +			(unsigned long)fattr->valid);

Why the cast? You could just set the format to %llx and pass fattr-
>valid as-is?

>  
>  	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
>  		/* Only a mounted-on-fileid? Just exit */
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index f00bfcee7120..056d0ad38756 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -168,8 +168,8 @@ struct nfs_server {
>  #define NFS_MOUNT_SHUTDOWN			0x08000000
>  #define NFS_MOUNT_NO_ALIGNWRITE		0x10000000
>  
> -	unsigned int		fattr_valid;	/* Valid attributes */
>  	unsigned int		caps;		/* server capabilities */
> +	__u64			fattr_valid;	/* Valid attributes */
>  	unsigned int		rsize;		/* read size */
>  	unsigned int		rpages;		/* read size (in pages) */
>  	unsigned int		wsize;		/* write size */
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 9155a6ffc370..b7b06f0d2fb9 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -45,7 +45,7 @@ struct nfs4_threshold {
>  };
>  
>  struct nfs_fattr {
> -	unsigned int		valid;		/* which fields are valid */
> +	__u64			valid;		/* which fields are valid */
>  	umode_t			mode;
>  	__u32			nlink;
>  	kuid_t			uid;
> @@ -80,32 +80,32 @@ struct nfs_fattr {
>  	struct nfs4_label	*label;
>  };
>  
> -#define NFS_ATTR_FATTR_TYPE		(1U << 0)
> -#define NFS_ATTR_FATTR_MODE		(1U << 1)
> -#define NFS_ATTR_FATTR_NLINK		(1U << 2)
> -#define NFS_ATTR_FATTR_OWNER		(1U << 3)
> -#define NFS_ATTR_FATTR_GROUP		(1U << 4)
> -#define NFS_ATTR_FATTR_RDEV		(1U << 5)
> -#define NFS_ATTR_FATTR_SIZE		(1U << 6)
> -#define NFS_ATTR_FATTR_PRESIZE		(1U << 7)
> -#define NFS_ATTR_FATTR_BLOCKS_USED	(1U << 8)
> -#define NFS_ATTR_FATTR_SPACE_USED	(1U << 9)
> -#define NFS_ATTR_FATTR_FSID		(1U << 10)
> -#define NFS_ATTR_FATTR_FILEID		(1U << 11)
> -#define NFS_ATTR_FATTR_ATIME		(1U << 12)
> -#define NFS_ATTR_FATTR_MTIME		(1U << 13)
> -#define NFS_ATTR_FATTR_CTIME		(1U << 14)
> -#define NFS_ATTR_FATTR_PREMTIME		(1U << 15)
> -#define NFS_ATTR_FATTR_PRECTIME		(1U << 16)
> -#define NFS_ATTR_FATTR_CHANGE		(1U << 17)
> -#define NFS_ATTR_FATTR_PRECHANGE	(1U << 18)
> -#define NFS_ATTR_FATTR_V4_LOCATIONS	(1U << 19)
> -#define NFS_ATTR_FATTR_V4_REFERRAL	(1U << 20)
> -#define NFS_ATTR_FATTR_MOUNTPOINT	(1U << 21)
> -#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID (1U << 22)
> -#define NFS_ATTR_FATTR_OWNER_NAME	(1U << 23)
> -#define NFS_ATTR_FATTR_GROUP_NAME	(1U << 24)
> -#define NFS_ATTR_FATTR_V4_SECURITY_LABEL (1U << 25)
> +#define NFS_ATTR_FATTR_TYPE		BIT_ULL(0)
> +#define NFS_ATTR_FATTR_MODE		BIT_ULL(1)
> +#define NFS_ATTR_FATTR_NLINK		BIT_ULL(2)
> +#define NFS_ATTR_FATTR_OWNER		BIT_ULL(3)
> +#define NFS_ATTR_FATTR_GROUP		BIT_ULL(4)
> +#define NFS_ATTR_FATTR_RDEV		BIT_ULL(5)
> +#define NFS_ATTR_FATTR_SIZE		BIT_ULL(6)
> +#define NFS_ATTR_FATTR_PRESIZE		BIT_ULL(7)
> +#define NFS_ATTR_FATTR_BLOCKS_USED	BIT_ULL(8)
> +#define NFS_ATTR_FATTR_SPACE_USED	BIT_ULL(9)
> +#define NFS_ATTR_FATTR_FSID		BIT_ULL(10)
> +#define NFS_ATTR_FATTR_FILEID		BIT_ULL(11)
> +#define NFS_ATTR_FATTR_ATIME		BIT_ULL(12)
> +#define NFS_ATTR_FATTR_MTIME		BIT_ULL(13)
> +#define NFS_ATTR_FATTR_CTIME		BIT_ULL(14)
> +#define NFS_ATTR_FATTR_PREMTIME		BIT_ULL(15)
> +#define NFS_ATTR_FATTR_PRECTIME		BIT_ULL(16)
> +#define NFS_ATTR_FATTR_CHANGE		BIT_ULL(17)
> +#define NFS_ATTR_FATTR_PRECHANGE	BIT_ULL(18)
> +#define NFS_ATTR_FATTR_V4_LOCATIONS	BIT_ULL(19)
> +#define NFS_ATTR_FATTR_V4_REFERRAL	BIT_ULL(20)
> +#define NFS_ATTR_FATTR_MOUNTPOINT	BIT_ULL(21)
> +#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID BIT_ULL(22)
> +#define NFS_ATTR_FATTR_OWNER_NAME	BIT_ULL(23)
> +#define NFS_ATTR_FATTR_GROUP_NAME	BIT_ULL(24)
> +#define NFS_ATTR_FATTR_V4_SECURITY_LABEL BIT_ULL(25)
>  
>  #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
>  		| NFS_ATTR_FATTR_MODE \

-- 
Jeff Layton <jlayton@kernel.org>