| Message ID | 20201005213852.233004-2-preichl@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: Remove wrappers for some semaphores | expand |
On Mon, Oct 05, 2020 at 11:38:49PM +0200, 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> > Suggested-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_inode.c | 62 ++++++++++++++++++++++++++++++++++------------ > fs/xfs/xfs_inode.h | 21 ++++++++++------ > 2 files changed, 60 insertions(+), 23 deletions(-) > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index c06129cffba9..1f39bce96656 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -345,32 +345,62 @@ xfs_ilock_demote( > } > > #if defined(DEBUG) || defined(XFS_WARN) > -int > +static inline bool > +__xfs_rwsem_islocked( > + struct rw_semaphore *rwsem, > + int lock_flags) > +{ > + int arg; This ought ^^^^ to be indented ^^ to there. Otherwise this patch looks ok to me from what I vaguely remember from the last revision. > + > + if (!debug_locks) > + return rwsem_is_locked(rwsem); > + > + if (lock_flags & (1 << XFS_SHARED_LOCK_SHIFT)) { > + /* > + * The caller could be asking if we have (shared | excl) > + * access to the lock. Ask lockdep if the rwsem is > + * locked either for read or write access. > + * > + * The caller could also be asking if we have only > + * shared access to the lock. Holding a rwsem > + * write-locked implies read access as well, so the > + * request to lockdep is the same for this case. > + */ > + arg = -1; > + } else { > + /* > + * The caller is asking if we have only exclusive access > + * to the lock. Ask lockdep if the rwsem is locked for > + * write access. Oh, also I appreciate the comment that documents why we're picking these seemingly magic numbers. --D > + */ > + arg = 0; > + } > + > + return lockdep_is_held_type(rwsem, arg); > +} > + > +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); > + if (lock_flags & (XFS_ILOCK_EXCL | XFS_ILOCK_SHARED)) { > + return __xfs_rwsem_islocked(&ip->i_lock, > + (lock_flags >> XFS_ILOCK_FLAG_SHIFT)); > } > > - 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); > + if (lock_flags & (XFS_MMAPLOCK_EXCL | XFS_MMAPLOCK_SHARED)) { > + return __xfs_rwsem_islocked(&ip->i_mmaplock, > + (lock_flags >> XFS_MMAPLOCK_FLAG_SHIFT)); > } > > - 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); > + if (lock_flags & (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED)) { > + return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem, > + (lock_flags >> XFS_IOLOCK_FLAG_SHIFT)); > } > > ASSERT(0); > - return 0; > + return false; > } > #endif > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index e9a8bb184d1f..77d5655191ab 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h > @@ -268,12 +268,19 @@ static inline void xfs_ifunlock(struct xfs_inode *ip) > * Bit ranges: 1<<1 - 1<<16-1 -- iolock/ilock modes (bitfield) > * 1<<16 - 1<<32-1 -- lockdep annotation (integers) > */ > -#define XFS_IOLOCK_EXCL (1<<0) > -#define XFS_IOLOCK_SHARED (1<<1) > -#define XFS_ILOCK_EXCL (1<<2) > -#define XFS_ILOCK_SHARED (1<<3) > -#define XFS_MMAPLOCK_EXCL (1<<4) > -#define XFS_MMAPLOCK_SHARED (1<<5) > + > +#define XFS_IOLOCK_FLAG_SHIFT 0 > +#define XFS_ILOCK_FLAG_SHIFT 2 > +#define XFS_MMAPLOCK_FLAG_SHIFT 4 > + > +#define XFS_SHARED_LOCK_SHIFT 1 > + > +#define XFS_IOLOCK_EXCL (1 << (XFS_IOLOCK_FLAG_SHIFT)) > +#define XFS_IOLOCK_SHARED (XFS_IOLOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) > +#define XFS_ILOCK_EXCL (1 << (XFS_ILOCK_FLAG_SHIFT)) > +#define XFS_ILOCK_SHARED (XFS_ILOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) > +#define XFS_MMAPLOCK_EXCL (1 << (XFS_MMAPLOCK_FLAG_SHIFT)) > +#define XFS_MMAPLOCK_SHARED (XFS_MMAPLOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) > > #define XFS_LOCK_MASK (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED \ > | XFS_ILOCK_EXCL | XFS_ILOCK_SHARED \ > @@ -412,7 +419,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 *); > > -- > 2.26.2 >
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index c06129cffba9..1f39bce96656 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -345,32 +345,62 @@ xfs_ilock_demote( } #if defined(DEBUG) || defined(XFS_WARN) -int +static inline bool +__xfs_rwsem_islocked( + struct rw_semaphore *rwsem, + int lock_flags) +{ + int arg; + + if (!debug_locks) + return rwsem_is_locked(rwsem); + + if (lock_flags & (1 << XFS_SHARED_LOCK_SHIFT)) { + /* + * The caller could be asking if we have (shared | excl) + * access to the lock. Ask lockdep if the rwsem is + * locked either for read or write access. + * + * The caller could also be asking if we have only + * shared access to the lock. Holding a rwsem + * write-locked implies read access as well, so the + * request to lockdep is the same for this case. + */ + arg = -1; + } else { + /* + * The caller is asking if we have only exclusive access + * to the lock. Ask lockdep if the rwsem is locked for + * write access. + */ + arg = 0; + } + + return lockdep_is_held_type(rwsem, arg); +} + +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); + if (lock_flags & (XFS_ILOCK_EXCL | XFS_ILOCK_SHARED)) { + return __xfs_rwsem_islocked(&ip->i_lock, + (lock_flags >> XFS_ILOCK_FLAG_SHIFT)); } - 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); + if (lock_flags & (XFS_MMAPLOCK_EXCL | XFS_MMAPLOCK_SHARED)) { + return __xfs_rwsem_islocked(&ip->i_mmaplock, + (lock_flags >> XFS_MMAPLOCK_FLAG_SHIFT)); } - 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); + if (lock_flags & (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED)) { + return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem, + (lock_flags >> XFS_IOLOCK_FLAG_SHIFT)); } ASSERT(0); - return 0; + return false; } #endif diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index e9a8bb184d1f..77d5655191ab 100644 --- a/fs/xfs/xfs_inode.h +++ b/fs/xfs/xfs_inode.h @@ -268,12 +268,19 @@ static inline void xfs_ifunlock(struct xfs_inode *ip) * Bit ranges: 1<<1 - 1<<16-1 -- iolock/ilock modes (bitfield) * 1<<16 - 1<<32-1 -- lockdep annotation (integers) */ -#define XFS_IOLOCK_EXCL (1<<0) -#define XFS_IOLOCK_SHARED (1<<1) -#define XFS_ILOCK_EXCL (1<<2) -#define XFS_ILOCK_SHARED (1<<3) -#define XFS_MMAPLOCK_EXCL (1<<4) -#define XFS_MMAPLOCK_SHARED (1<<5) + +#define XFS_IOLOCK_FLAG_SHIFT 0 +#define XFS_ILOCK_FLAG_SHIFT 2 +#define XFS_MMAPLOCK_FLAG_SHIFT 4 + +#define XFS_SHARED_LOCK_SHIFT 1 + +#define XFS_IOLOCK_EXCL (1 << (XFS_IOLOCK_FLAG_SHIFT)) +#define XFS_IOLOCK_SHARED (XFS_IOLOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) +#define XFS_ILOCK_EXCL (1 << (XFS_ILOCK_FLAG_SHIFT)) +#define XFS_ILOCK_SHARED (XFS_ILOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) +#define XFS_MMAPLOCK_EXCL (1 << (XFS_MMAPLOCK_FLAG_SHIFT)) +#define XFS_MMAPLOCK_SHARED (XFS_MMAPLOCK_EXCL << (XFS_SHARED_LOCK_SHIFT)) #define XFS_LOCK_MASK (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED \ | XFS_ILOCK_EXCL | XFS_ILOCK_SHARED \ @@ -412,7 +419,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> Suggested-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/xfs/xfs_inode.c | 62 ++++++++++++++++++++++++++++++++++------------ fs/xfs/xfs_inode.h | 21 ++++++++++------ 2 files changed, 60 insertions(+), 23 deletions(-)