Re: [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions

[Eric Sandeen <sandeen@sandeen.net>]

On 2/3/20 11:58 AM, Pavel Reichl wrote:
> Add xfs_is_ilocked(), xfs_is_iolocked() and xfs_is_mmaplocked()
> 
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_inode.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_inode.h |  3 +++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..80874c80df6d 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -372,6 +372,59 @@ xfs_isilocked(
>  	ASSERT(0);
>  	return 0;
>  }
> +
> +static inline bool
> +__xfs_is_ilocked(
> +	struct rw_semaphore	*rwsem,
> +	bool			shared,
> +	bool			excl)
> +{
> +	bool locked = false;
> +
> +	if (!rwsem_is_locked(rwsem))
> +		return false;
> +
> +	if (!debug_locks)
> +		return true;
> +
> +	if (shared)
> +		locked = lockdep_is_held_type(rwsem, 0);
> +
> +	if (excl)
> +		locked |= lockdep_is_held_type(rwsem, 1);
> +
> +	return locked;
> +}
> +
> +bool
> +xfs_is_ilocked(
> +	struct xfs_inode	*ip,
> +	int			lock_flags)
> +{
> +	return __xfs_is_ilocked(&ip->i_lock.mr_lock,
> +			(lock_flags & XFS_ILOCK_SHARED),
> +			(lock_flags & XFS_ILOCK_EXCL));
> +}

Apologies for not following or chiming in sooner on the prior discussion.

It seems a little odd that we must specify the lock type to test to
each of these 3 functions which are already named after a specific lock type.

i.e.:

xfs_is_mmaplocked(XFS_MMAPLOCK_EXCL);

seems a little redundant, but more problematic:

xfs_is_mmaplocked(XFS_ILOCK_SHARED);

would be accepted and silently return false.  I think that at least an
ASSERT() in each of the 3 functions to be sure we didn't pass in nonsense
flags would be wise.

-Eric