On 1/28/20 10:42 AM, Darrick J. Wong wrote:
On Tue, Jan 28, 2020 at 03:55:25PM +0100, Pavel Reichl wrote:
mr_writer is obsolete and the information it contains is accesible
from mr_lock.
Signed-off-by: Pavel Reichl <preichl@xxxxxxxxxx>
---
fs/xfs/xfs_inode.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c5077e6326c7..32fac6152dc3 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -352,13 +352,17 @@ xfs_isilocked(
{
if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) {
if (!(lock_flags & XFS_ILOCK_SHARED))
- return !!ip->i_lock.mr_writer;
+ return !debug_locks ||
+ lockdep_is_held_type(&ip->i_lock.mr_lock, 0);
Why do we reference debug_locks here directly? It looks as though that
variable exists to shut up lockdep assertions WARN_ONs, but
xfs_isilocked is a predicate (and not itself an assertion), so why can't
we 'return lockdep_is_held_type(...);' directly?
(He says scowling at his own RVB in 6552321831dce).
yes that's the answer to why /this/ patch does that ;)
We seem to not be the only ones but who knows if this is cargo cult or?
8 i915/gem/i915_gem_object. i915_gem_object_lookup_rc 66
WARN_ON(debug_locks && !lock_is_held(&rcu_lock_map));
l include/net/sock.h sock_owned_by_me 1573
WARN_ON_ONCE(!lockdep_sock_is_held(sk) && debug_locks);
I'd be inclined to accept this as the current way xfs_isilocked handles
the lockdep tests, and if it's wrong or unnecessary that's another patch.
-Eric
--D
return rwsem_is_locked(&ip->i_lock.mr_lock);
}
if (lock_flags & (XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED)) {
if (!(lock_flags & XFS_MMAPLOCK_SHARED))
- return !!ip->i_mmaplock.mr_writer;
+ return !debug_locks ||
+ lockdep_is_held_type(
+ &ip->i_mmaplock.mr_lock,
+ 0);
return rwsem_is_locked(&ip->i_mmaplock.mr_lock);
}
--
2.24.1
