Re: [PATCH RFC 6/8] xfs: fix imbalanced locking

[Christoph Hellwig <hch@infradead.org>]

On Fri, Jul 06, 2018 at 11:12:27AM +0800, Shan Hai wrote:
> The XFS_ILOCK_SHARED type lock is held in the xfs_ioc_fsgetxattr
> but does not released when the inode is in local format, fix it by
> eleasing the lock on the local format inode.
> 
> Signed-off-by: Shan Hai <shan.hai@oracle.com>
> ---
>  fs/xfs/xfs_ioctl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 0ef5ece5634c..246562615ccd 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -907,7 +907,10 @@ xfs_ioc_fsgetxattr(
>  	} else {
>  		if (ip->i_df.if_flags & XFS_IFEXTENTS)
>  			fa.fsx_nextents = xfs_iext_count(&ip->i_df);
> -		else
> +		else if (ip->i_df.if_flags & XFS_IFINLINE) {
> +			xfs_iunlock(ip, XFS_ILOCK_SHARED);
> +			return 0;
> +		} else
>  			fa.fsx_nextents = ip->i_d.di_nextents;
>  	}
>  	xfs_iunlock(ip, XFS_ILOCK_SHARED);

This looks odd, and the changelog confuses even more.

Please avoid the early return, which means we miss the copy_to_user
of the fa struct.  It seems to me like this should be something like:

		if (ip->i_df.if_flags & XFS_IFEXTENTS)
                        fa.fsx_nextents = xfs_iext_count(&ip->i_df);
		esle if (ip->i_df.if_flags & XFS_IFINLINE)
			fa.fsx_nextents = 0;
                else
			fa.fsx_nextents = ip->i_d.di_nextents;