Hi, maybe performance optimalizaton? If debug_locks is false then
there's no need to call lockdep_is_held_type()? But I'm not sure :-)
On Tue, Jan 28, 2020 at 5:42 PM Darrick J. Wong <darrick.wong@xxxxxxxxxx> 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).
>
> --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
> >
>
