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.
