| Message ID | 20200211221018.709125-1-preichl@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v4,1/4] xfs: Refactor xfs_isilocked() | expand |
On 2/11/20 4:10 PM, Pavel Reichl wrote: > Refactor xfs_isilocked() to use newly introduced __xfs_rwsem_islocked(). > __xfs_rwsem_islocked() is a helper function which encapsulates checking > state of rw_semaphores hold by inode. > > Signed-off-by: Pavel Reichl <preichl@redhat.com> > Suggested-by: Dave Chinner <dchinner@redhat.com> > Suggested-by: Eric Sandeen <sandeen@redhat.com> > --- > Changelog from V3: > Added ASSERTS() to isilocked() to make sure that only flags for a single > type of lock are passed So while I like the ASSERTs going forward, the problem here is that a bisect will now be broken between this and patch three. Sorry to do this to you, but I think you should probably add the asserts in patch 3 so that you fix the wrong calls and add the protective asserts at the same time. Also, since your next patch fixes whitespace, I guess this: + ASSERT(!(lock_flags & ~(XFS_ILOCK_EXCL|XFS_ILOCK_SHARED))); should observe the same rules around " | " -Eric > > fs/xfs/xfs_inode.c | 51 ++++++++++++++++++++++++++++++++++------------ > fs/xfs/xfs_inode.h | 2 +- > 2 files changed, 39 insertions(+), 14 deletions(-) > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index c5077e6326c7..cfefa7543b37 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -345,32 +345,57 @@ xfs_ilock_demote( > } > > #if defined(DEBUG) || defined(XFS_WARN) > -int > +static inline bool > +__xfs_rwsem_islocked( > + 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_isilocked( > - xfs_inode_t *ip, > + struct xfs_inode *ip, > uint lock_flags) > { > if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) { > - if (!(lock_flags & XFS_ILOCK_SHARED)) > - return !!ip->i_lock.mr_writer; > - return rwsem_is_locked(&ip->i_lock.mr_lock); > + ASSERT(!(lock_flags & ~(XFS_ILOCK_EXCL|XFS_ILOCK_SHARED))); > + return __xfs_rwsem_islocked(&ip->i_lock.mr_lock, > + (lock_flags & XFS_ILOCK_SHARED), > + (lock_flags & XFS_ILOCK_EXCL)); > } > > if (lock_flags & (XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED)) { > - if (!(lock_flags & XFS_MMAPLOCK_SHARED)) > - return !!ip->i_mmaplock.mr_writer; > - return rwsem_is_locked(&ip->i_mmaplock.mr_lock); > + ASSERT(!(lock_flags & ~(XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED))); > + return __xfs_rwsem_islocked(&ip->i_mmaplock.mr_lock, > + (lock_flags & XFS_MMAPLOCK_SHARED), > + (lock_flags & XFS_MMAPLOCK_EXCL)); > } > > if (lock_flags & (XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED)) { > - if (!(lock_flags & XFS_IOLOCK_SHARED)) > - return !debug_locks || > - lockdep_is_held_type(&VFS_I(ip)->i_rwsem, 0); > - return rwsem_is_locked(&VFS_I(ip)->i_rwsem); > + ASSERT(!(lock_flags & ~(XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED))); > + return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem, > + (lock_flags & XFS_IOLOCK_SHARED), > + (lock_flags & XFS_IOLOCK_EXCL)); > } > > ASSERT(0); > - return 0; > + return false; > } > #endif > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index 492e53992fa9..3d7ce355407d 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h > @@ -416,7 +416,7 @@ void xfs_ilock(xfs_inode_t *, uint); > int xfs_ilock_nowait(xfs_inode_t *, uint); > void xfs_iunlock(xfs_inode_t *, uint); > void xfs_ilock_demote(xfs_inode_t *, uint); > -int xfs_isilocked(xfs_inode_t *, uint); > +bool xfs_isilocked(xfs_inode_t *, uint); > uint xfs_ilock_data_map_shared(struct xfs_inode *); > uint xfs_ilock_attr_map_shared(struct xfs_inode *); > >
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index c5077e6326c7..cfefa7543b37 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -345,32 +345,57 @@ xfs_ilock_demote( } #if defined(DEBUG) || defined(XFS_WARN) -int +static inline bool +__xfs_rwsem_islocked( + 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_isilocked( - xfs_inode_t *ip, + struct xfs_inode *ip, uint lock_flags) { if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) { - if (!(lock_flags & XFS_ILOCK_SHARED)) - return !!ip->i_lock.mr_writer; - return rwsem_is_locked(&ip->i_lock.mr_lock); + ASSERT(!(lock_flags & ~(XFS_ILOCK_EXCL|XFS_ILOCK_SHARED))); + return __xfs_rwsem_islocked(&ip->i_lock.mr_lock, + (lock_flags & XFS_ILOCK_SHARED), + (lock_flags & XFS_ILOCK_EXCL)); } if (lock_flags & (XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED)) { - if (!(lock_flags & XFS_MMAPLOCK_SHARED)) - return !!ip->i_mmaplock.mr_writer; - return rwsem_is_locked(&ip->i_mmaplock.mr_lock); + ASSERT(!(lock_flags & ~(XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED))); + return __xfs_rwsem_islocked(&ip->i_mmaplock.mr_lock, + (lock_flags & XFS_MMAPLOCK_SHARED), + (lock_flags & XFS_MMAPLOCK_EXCL)); } if (lock_flags & (XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED)) { - if (!(lock_flags & XFS_IOLOCK_SHARED)) - return !debug_locks || - lockdep_is_held_type(&VFS_I(ip)->i_rwsem, 0); - return rwsem_is_locked(&VFS_I(ip)->i_rwsem); + ASSERT(!(lock_flags & ~(XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED))); + return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem, + (lock_flags & XFS_IOLOCK_SHARED), + (lock_flags & XFS_IOLOCK_EXCL)); } ASSERT(0); - return 0; + return false; } #endif diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index 492e53992fa9..3d7ce355407d 100644 --- a/fs/xfs/xfs_inode.h +++ b/fs/xfs/xfs_inode.h @@ -416,7 +416,7 @@ void xfs_ilock(xfs_inode_t *, uint); int xfs_ilock_nowait(xfs_inode_t *, uint); void xfs_iunlock(xfs_inode_t *, uint); void xfs_ilock_demote(xfs_inode_t *, uint); -int xfs_isilocked(xfs_inode_t *, uint); +bool xfs_isilocked(xfs_inode_t *, uint); uint xfs_ilock_data_map_shared(struct xfs_inode *); uint xfs_ilock_attr_map_shared(struct xfs_inode *);
Refactor xfs_isilocked() to use newly introduced __xfs_rwsem_islocked(). __xfs_rwsem_islocked() is a helper function which encapsulates checking state of rw_semaphores hold by inode. Signed-off-by: Pavel Reichl <preichl@redhat.com> Suggested-by: Dave Chinner <dchinner@redhat.com> Suggested-by: Eric Sandeen <sandeen@redhat.com> --- Changelog from V3: Added ASSERTS() to isilocked() to make sure that only flags for a single type of lock are passed fs/xfs/xfs_inode.c | 51 ++++++++++++++++++++++++++++++++++------------ fs/xfs/xfs_inode.h | 2 +- 2 files changed, 39 insertions(+), 14 deletions(-)