Re: [PATCH 13/17 v2] xfs: convert inode lock flags to unsigned.

[Chandan Babu R <chandan.babu@oracle.com>]

On 21 Apr 2022 at 06:14, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 5.18 w/ std=gnu11 compiled with gcc-5 wants flags stored in unsigned
> fields to be unsigned.
>

Looks good.

Reviewed-by: Chandan Babu R <chandan.babu@oracle.com>

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> V2:
> - convert the missed ILOCK bit values and masks to unsigned.
>
>  fs/xfs/xfs_file.c  | 12 ++++++------
>  fs/xfs/xfs_inode.c | 21 ++++++++++++---------
>  fs/xfs/xfs_inode.h | 24 ++++++++++++------------
>  3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 5bddb1e9e0b3..f3e878408747 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -310,7 +310,7 @@ STATIC ssize_t
>  xfs_file_write_checks(
>  	struct kiocb		*iocb,
>  	struct iov_iter		*from,
> -	int			*iolock)
> +	unsigned int		*iolock)
>  {
>  	struct file		*file = iocb->ki_filp;
>  	struct inode		*inode = file->f_mapping->host;
> @@ -513,7 +513,7 @@ xfs_file_dio_write_aligned(
>  	struct kiocb		*iocb,
>  	struct iov_iter		*from)
>  {
> -	int			iolock = XFS_IOLOCK_SHARED;
> +	unsigned int		iolock = XFS_IOLOCK_SHARED;
>  	ssize_t			ret;
>  
>  	ret = xfs_ilock_iocb(iocb, iolock);
> @@ -566,7 +566,7 @@ xfs_file_dio_write_unaligned(
>  {
>  	size_t			isize = i_size_read(VFS_I(ip));
>  	size_t			count = iov_iter_count(from);
> -	int			iolock = XFS_IOLOCK_SHARED;
> +	unsigned int		iolock = XFS_IOLOCK_SHARED;
>  	unsigned int		flags = IOMAP_DIO_OVERWRITE_ONLY;
>  	ssize_t			ret;
>  
> @@ -655,7 +655,7 @@ xfs_file_dax_write(
>  {
>  	struct inode		*inode = iocb->ki_filp->f_mapping->host;
>  	struct xfs_inode	*ip = XFS_I(inode);
> -	int			iolock = XFS_IOLOCK_EXCL;
> +	unsigned int		iolock = XFS_IOLOCK_EXCL;
>  	ssize_t			ret, error = 0;
>  	loff_t			pos;
>  
> @@ -700,7 +700,7 @@ xfs_file_buffered_write(
>  	struct xfs_inode	*ip = XFS_I(inode);
>  	ssize_t			ret;
>  	bool			cleared_space = false;
> -	int			iolock;
> +	unsigned int		iolock;
>  
>  	if (iocb->ki_flags & IOCB_NOWAIT)
>  		return -EOPNOTSUPP;
> @@ -1181,7 +1181,7 @@ xfs_dir_open(
>  	struct file	*file)
>  {
>  	struct xfs_inode *ip = XFS_I(inode);
> -	int		mode;
> +	unsigned int	mode;
>  	int		error;
>  
>  	error = xfs_file_open(inode, file);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 9de6205fe134..5ea460f62201 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -416,10 +416,12 @@ xfs_lockdep_subclass_ok(
>   * parent locking. Care must be taken to ensure we don't overrun the subclass
>   * storage fields in the class mask we build.
>   */
> -static inline int
> -xfs_lock_inumorder(int lock_mode, int subclass)
> +static inline uint
> +xfs_lock_inumorder(
> +	uint	lock_mode,
> +	uint	subclass)
>  {
> -	int	class = 0;
> +	uint	class = 0;
>  
>  	ASSERT(!(lock_mode & (XFS_ILOCK_PARENT | XFS_ILOCK_RTBITMAP |
>  			      XFS_ILOCK_RTSUM)));
> @@ -464,7 +466,10 @@ xfs_lock_inodes(
>  	int			inodes,
>  	uint			lock_mode)
>  {
> -	int			attempts = 0, i, j, try_lock;
> +	int			attempts = 0;
> +	uint			i;
> +	int			j;
> +	bool			try_lock;
>  	struct xfs_log_item	*lp;
>  
>  	/*
> @@ -489,9 +494,9 @@ xfs_lock_inodes(
>  	} else if (lock_mode & XFS_MMAPLOCK_EXCL)
>  		ASSERT(!(lock_mode & XFS_ILOCK_EXCL));
>  
> -	try_lock = 0;
> -	i = 0;
>  again:
> +	try_lock = false;
> +	i = 0;
>  	for (; i < inodes; i++) {
>  		ASSERT(ips[i]);
>  
> @@ -506,7 +511,7 @@ xfs_lock_inodes(
>  			for (j = (i - 1); j >= 0 && !try_lock; j--) {
>  				lp = &ips[j]->i_itemp->ili_item;
>  				if (lp && test_bit(XFS_LI_IN_AIL, &lp->li_flags))
> -					try_lock++;
> +					try_lock = true;
>  			}
>  		}
>  
> @@ -546,8 +551,6 @@ xfs_lock_inodes(
>  		if ((attempts % 5) == 0) {
>  			delay(1); /* Don't just spin the CPU */
>  		}
> -		i = 0;
> -		try_lock = 0;
>  		goto again;
>  	}
>  }
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 740ab13d1aa2..b67ab9f10cf9 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -278,12 +278,12 @@ static inline bool xfs_inode_has_bigtime(struct xfs_inode *ip)
>   * Bit ranges:	1<<1  - 1<<16-1 -- iolock/ilock modes (bitfield)
>   *		1<<16 - 1<<32-1 -- lockdep annotation (integers)
>   */
> -#define	XFS_IOLOCK_EXCL		(1<<0)
> -#define	XFS_IOLOCK_SHARED	(1<<1)
> -#define	XFS_ILOCK_EXCL		(1<<2)
> -#define	XFS_ILOCK_SHARED	(1<<3)
> -#define	XFS_MMAPLOCK_EXCL	(1<<4)
> -#define	XFS_MMAPLOCK_SHARED	(1<<5)
> +#define	XFS_IOLOCK_EXCL		(1u << 0)
> +#define	XFS_IOLOCK_SHARED	(1u << 1)
> +#define	XFS_ILOCK_EXCL		(1u << 2)
> +#define	XFS_ILOCK_SHARED	(1u << 3)
> +#define	XFS_MMAPLOCK_EXCL	(1u << 4)
> +#define	XFS_MMAPLOCK_SHARED	(1u << 5)
>  
>  #define XFS_LOCK_MASK		(XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED \
>  				| XFS_ILOCK_EXCL | XFS_ILOCK_SHARED \
> @@ -350,19 +350,19 @@ static inline bool xfs_inode_has_bigtime(struct xfs_inode *ip)
>   */
>  #define XFS_IOLOCK_SHIFT		16
>  #define XFS_IOLOCK_MAX_SUBCLASS		3
> -#define XFS_IOLOCK_DEP_MASK		0x000f0000
> +#define XFS_IOLOCK_DEP_MASK		0x000f0000u
>  
>  #define XFS_MMAPLOCK_SHIFT		20
>  #define XFS_MMAPLOCK_NUMORDER		0
>  #define XFS_MMAPLOCK_MAX_SUBCLASS	3
> -#define XFS_MMAPLOCK_DEP_MASK		0x00f00000
> +#define XFS_MMAPLOCK_DEP_MASK		0x00f00000u
>  
>  #define XFS_ILOCK_SHIFT			24
> -#define XFS_ILOCK_PARENT_VAL		5
> +#define XFS_ILOCK_PARENT_VAL		5u
>  #define XFS_ILOCK_MAX_SUBCLASS		(XFS_ILOCK_PARENT_VAL - 1)
> -#define XFS_ILOCK_RTBITMAP_VAL		6
> -#define XFS_ILOCK_RTSUM_VAL		7
> -#define XFS_ILOCK_DEP_MASK		0xff000000
> +#define XFS_ILOCK_RTBITMAP_VAL		6u
> +#define XFS_ILOCK_RTSUM_VAL		7u
> +#define XFS_ILOCK_DEP_MASK		0xff000000u
>  #define	XFS_ILOCK_PARENT		(XFS_ILOCK_PARENT_VAL << XFS_ILOCK_SHIFT)
>  #define	XFS_ILOCK_RTBITMAP		(XFS_ILOCK_RTBITMAP_VAL << XFS_ILOCK_SHIFT)
>  #define	XFS_ILOCK_RTSUM			(XFS_ILOCK_RTSUM_VAL << XFS_ILOCK_SHIFT)


-- 
chandan