Re: [PATCH 1/4] xfs: change xfs_isilocked() to always use lockdep()

[Christoph Hellwig <hch@infradead.org>]

On Thu, Jan 30, 2020 at 09:18:19AM +1100, Dave Chinner wrote:
> This captures both read and write locks on the rwsem, and doesn't
> discriminate at all. Now we don't have explicit writer lock checking
> in CONFIG_XFS_DEBUG=y kernels, I think we need to at least check
> that the rwsem is locked in all cases to catch cases where we are
> calling a function without the lock held. That will ctach most
> programming mistakes, and then lockdep will provide the
> read-vs-write discrimination to catch the "hold the wrong lock type"
> mistakes.
> 
> Hence I think this code should end up looking like this:
> 
> 	if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) {
> 		bool locked = false;
> 
> 		if (!rwsem_is_locked(&ip->i_lock))
> 			return false;
> 		if (!debug_locks)
> 			return true;
> 		if (lock_flags & XFS_ILOCK_EXCL)
> 			locked = lockdep_is_held_type(&ip->i_lock, 0);
> 		if (lock_flags & XFS_ILOCK_SHARED)
> 			locked |= lockdep_is_held_type(&ip->i_lock, 1);
> 		return locked;
> 	}
> 
> Thoughts?

I like the idea, but I really think that this does not belong into XFS,
but into the core rwsem code.  That means replacing the lock_flags with
a bool exclusive, picking a good name for it (can't think of one right
now, except for re-using rwsem_is_locked), and adding a kerneldoc
comment explaining the semantics and use cases in detail.