* [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 21:00 ` Chaitanya Kulkarni
` (3 more replies)
2020-02-03 17:58 ` [PATCH v2 2/7] xfs: Update checking excl. locks for ilock Pavel Reichl
` (5 subsequent siblings)
6 siblings, 4 replies; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Add xfs_is_ilocked(), xfs_is_iolocked() and xfs_is_mmaplocked()
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_inode.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_inode.h | 3 +++
2 files changed, 56 insertions(+)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c5077e6326c7..80874c80df6d 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -372,6 +372,59 @@ xfs_isilocked(
ASSERT(0);
return 0;
}
+
+static inline bool
+__xfs_is_ilocked(
+ 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_is_ilocked(
+ struct xfs_inode *ip,
+ int lock_flags)
+{
+ return __xfs_is_ilocked(&ip->i_lock.mr_lock,
+ (lock_flags & XFS_ILOCK_SHARED),
+ (lock_flags & XFS_ILOCK_EXCL));
+}
+
+bool
+xfs_is_mmaplocked(
+ struct xfs_inode *ip,
+ int lock_flags)
+{
+ return __xfs_is_ilocked(&ip->i_mmaplock.mr_lock,
+ (lock_flags & XFS_MMAPLOCK_SHARED),
+ (lock_flags & XFS_MMAPLOCK_EXCL));
+}
+
+bool
+xfs_is_iolocked(
+ struct xfs_inode *ip,
+ int lock_flags)
+{
+ return __xfs_is_ilocked(&VFS_I(ip)->i_rwsem,
+ (lock_flags & XFS_IOLOCK_SHARED),
+ (lock_flags & XFS_IOLOCK_EXCL));
+}
#endif
/*
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 492e53992fa9..6ba575f35c1f 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -417,6 +417,9 @@ 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_is_ilocked(struct xfs_inode *, int);
+bool xfs_is_mmaplocked(struct xfs_inode *, int);
+bool xfs_is_iolocked(struct xfs_inode *, int);
uint xfs_ilock_data_map_shared(struct xfs_inode *);
uint xfs_ilock_attr_map_shared(struct xfs_inode *);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
@ 2020-02-03 21:00 ` Chaitanya Kulkarni
2020-02-03 23:15 ` Dave Chinner
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-03 21:00 UTC (permalink / raw)
To: Pavel Reichl, linux-xfs; +Cc: Dave Chinner
Hi Pavel,
Thanks for the patch. Please see inline-comment.
On 02/03/2020 09:59 AM, Pavel Reichl wrote:
> Add xfs_is_ilocked(), xfs_is_iolocked() and xfs_is_mmaplocked()
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_inode.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_inode.h | 3 +++
> 2 files changed, 56 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..80874c80df6d 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -372,6 +372,59 @@ xfs_isilocked(
> ASSERT(0);
> return 0;
> }
> +
> +static inline bool
> +__xfs_is_ilocked(
> + 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_is_ilocked(
> + struct xfs_inode *ip,
> + int lock_flags)
nit:- isn't above lock_flag variable should be uint ?
Is there a particular reason that it has int type ?
> +{
> + return __xfs_is_ilocked(&ip->i_lock.mr_lock,
> + (lock_flags & XFS_ILOCK_SHARED),
> + (lock_flags & XFS_ILOCK_EXCL));
> +}
> +
> +bool
> +xfs_is_mmaplocked(
> + struct xfs_inode *ip,
> + int lock_flags)
Same here.
> +{
> + return __xfs_is_ilocked(&ip->i_mmaplock.mr_lock,
> + (lock_flags & XFS_MMAPLOCK_SHARED),
> + (lock_flags & XFS_MMAPLOCK_EXCL));
> +}
> +
> +bool
> +xfs_is_iolocked(
> + struct xfs_inode *ip,
> + int lock_flags)
Same here.
> +{
> + return __xfs_is_ilocked(&VFS_I(ip)->i_rwsem,
> + (lock_flags & XFS_IOLOCK_SHARED),
> + (lock_flags & XFS_IOLOCK_EXCL));
> +}
> #endif
>
> /*
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 492e53992fa9..6ba575f35c1f 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -417,6 +417,9 @@ 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_is_ilocked(struct xfs_inode *, int);
> +bool xfs_is_mmaplocked(struct xfs_inode *, int);
> +bool xfs_is_iolocked(struct xfs_inode *, int);
> uint xfs_ilock_data_map_shared(struct xfs_inode *);
> uint xfs_ilock_attr_map_shared(struct xfs_inode *);
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
2020-02-03 21:00 ` Chaitanya Kulkarni
@ 2020-02-03 23:15 ` Dave Chinner
2020-02-03 23:45 ` Eric Sandeen
2020-02-04 6:19 ` Christoph Hellwig
3 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:15 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:44PM +0100, Pavel Reichl wrote:
> Add xfs_is_ilocked(), xfs_is_iolocked() and xfs_is_mmaplocked()
The commit description is supposed to explain "Why?" rather than
describe what the code does.
So why are we adding these interfaces?
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_inode.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_inode.h | 3 +++
> 2 files changed, 56 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..80874c80df6d 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -372,6 +372,59 @@ xfs_isilocked(
> ASSERT(0);
> return 0;
> }
> +
> +static inline bool
> +__xfs_is_ilocked(
> + 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;
> +}
This needs a comment explaining the reason why it is structured this
way. I can see quite clearly what it is doing, but why it is done
this way is not immediately apparent from the code.
In a few months, I'm not going to remember the reasons for this
code, and if the neither the code nor the commit description
explains the reasons why the code is like this, then it's really
quite difficult and time consuming to try to discover the reason for
the code being this way.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
2020-02-03 21:00 ` Chaitanya Kulkarni
2020-02-03 23:15 ` Dave Chinner
@ 2020-02-03 23:45 ` Eric Sandeen
2020-02-04 6:19 ` Christoph Hellwig
3 siblings, 0 replies; 21+ messages in thread
From: Eric Sandeen @ 2020-02-03 23:45 UTC (permalink / raw)
To: Pavel Reichl, linux-xfs; +Cc: Dave Chinner
On 2/3/20 11:58 AM, Pavel Reichl wrote:
> Add xfs_is_ilocked(), xfs_is_iolocked() and xfs_is_mmaplocked()
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_inode.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_inode.h | 3 +++
> 2 files changed, 56 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..80874c80df6d 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -372,6 +372,59 @@ xfs_isilocked(
> ASSERT(0);
> return 0;
> }
> +
> +static inline bool
> +__xfs_is_ilocked(
> + 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_is_ilocked(
> + struct xfs_inode *ip,
> + int lock_flags)
> +{
> + return __xfs_is_ilocked(&ip->i_lock.mr_lock,
> + (lock_flags & XFS_ILOCK_SHARED),
> + (lock_flags & XFS_ILOCK_EXCL));
> +}
Apologies for not following or chiming in sooner on the prior discussion.
It seems a little odd that we must specify the lock type to test to
each of these 3 functions which are already named after a specific lock type.
i.e.:
xfs_is_mmaplocked(XFS_MMAPLOCK_EXCL);
seems a little redundant, but more problematic:
xfs_is_mmaplocked(XFS_ILOCK_SHARED);
would be accepted and silently return false. I think that at least an
ASSERT() in each of the 3 functions to be sure we didn't pass in nonsense
flags would be wise.
-Eric
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
` (2 preceding siblings ...)
2020-02-03 23:45 ` Eric Sandeen
@ 2020-02-04 6:19 ` Christoph Hellwig
3 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2020-02-04 6:19 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
> +static inline bool
> +__xfs_is_ilocked(
> + struct rw_semaphore *rwsem,
> + bool shared,
> + bool excl)
This calling conventions seems odd. In other places like
lockdep we just have a bool excl. This means we might get a false
positive when the lock is held exclusive but only shared is asserted,
but given that the low-level helpers can't give better information
that isn't going to hurt.
Also I'd name this function xfs_rwsem_is_locked, as there is nothing
inode specific here.
I also agree that this function needs good comments explaining the
rationale.
> +bool
> +xfs_is_ilocked(
> + struct xfs_inode *ip,
> + int lock_flags)
> +{
> + return __xfs_is_ilocked(&ip->i_lock.mr_lock,
> + (lock_flags & XFS_ILOCK_SHARED),
> + (lock_flags & XFS_ILOCK_EXCL));
> +}
> +
> +bool
> +xfs_is_mmaplocked(
> + struct xfs_inode *ip,
> + int lock_flags)
> +{
> + return __xfs_is_ilocked(&ip->i_mmaplock.mr_lock,
> + (lock_flags & XFS_MMAPLOCK_SHARED),
> + (lock_flags & XFS_MMAPLOCK_EXCL));
> +}
> +
> +bool
> +xfs_is_iolocked(
> + struct xfs_inode *ip,
> + int lock_flags)
> +{
> + return __xfs_is_ilocked(&VFS_I(ip)->i_rwsem,
> + (lock_flags & XFS_IOLOCK_SHARED),
> + (lock_flags & XFS_IOLOCK_EXCL));
> +}
> #endif
What is the benefit of these helpers? xfs_isilocked can just
call __xfs_is_ilocked / xfs_rwsem_is_locked directly.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 23:16 ` Dave Chinner
2020-02-04 6:21 ` Christoph Hellwig
2020-02-03 17:58 ` [PATCH v2 3/7] xfs: Update checking read or write " Pavel Reichl
` (4 subsequent siblings)
6 siblings, 2 replies; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_attr_remote.c | 2 +-
fs/xfs/libxfs/xfs_bmap.c | 10 +++++-----
fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
fs/xfs/libxfs/xfs_trans_inode.c | 6 +++---
fs/xfs/xfs_dquot.c | 4 ++--
fs/xfs/xfs_inode.c | 4 ++--
fs/xfs/xfs_inode_item.c | 4 ++--
fs/xfs/xfs_iops.c | 4 ++--
fs/xfs/xfs_qm.c | 10 +++++-----
fs/xfs/xfs_reflink.c | 2 +-
fs/xfs/xfs_rtalloc.c | 4 ++--
fs/xfs/xfs_trans_dquot.c | 2 +-
12 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 8b7f74b3bea2..5607c5551095 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -577,7 +577,7 @@ xfs_attr_rmtval_stale(
struct xfs_mount *mp = ip->i_mount;
struct xfs_buf *bp;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (XFS_IS_CORRUPT(mp, map->br_startblock == DELAYSTARTBLOCK) ||
XFS_IS_CORRUPT(mp, map->br_startblock == HOLESTARTBLOCK))
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9a6d7a84689a..318c006b4b50 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1238,7 +1238,7 @@ xfs_iread_extents(
struct xfs_btree_cur *cur;
int error;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (XFS_IS_CORRUPT(mp,
XFS_IFORK_FORMAT(ip, whichfork) !=
@@ -4402,7 +4402,7 @@ xfs_bmapi_write(
ASSERT(tp != NULL);
ASSERT(len > 0);
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(!(flags & XFS_BMAPI_REMAP));
/* zeroing is for currently only for data extents, not metadata */
@@ -4678,7 +4678,7 @@ xfs_bmapi_remap(
ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT(len > 0);
ASSERT(len <= (xfs_filblks_t)MAXEXTLEN);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(!(flags & ~(XFS_BMAPI_ATTRFORK | XFS_BMAPI_PREALLOC |
XFS_BMAPI_NORMAP)));
ASSERT((flags & (XFS_BMAPI_ATTRFORK | XFS_BMAPI_PREALLOC)) !=
@@ -5329,7 +5329,7 @@ __xfs_bunmapi(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(len > 0);
ASSERT(nexts >= 0);
@@ -5700,7 +5700,7 @@ xfs_bmse_merge(
blockcount = left->br_blockcount + got->br_blockcount;
ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(xfs_bmse_can_merge(left, got, shift));
new = *left;
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index f42c74cb8be5..4ba9dea9bdf0 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -974,7 +974,7 @@ xfs_rtfree_extent(
mp = tp->t_mountp;
ASSERT(mp->m_rbmip->i_itemp != NULL);
- ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
error = xfs_rtcheck_alloc_range(mp, tp, bno, len);
if (error)
diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
index 2b8ccb5b975d..bac474d97574 100644
--- a/fs/xfs/libxfs/xfs_trans_inode.c
+++ b/fs/xfs/libxfs/xfs_trans_inode.c
@@ -29,7 +29,7 @@ xfs_trans_ijoin(
{
xfs_inode_log_item_t *iip;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (ip->i_itemp == NULL)
xfs_inode_item_init(ip, ip->i_mount);
iip = ip->i_itemp;
@@ -58,7 +58,7 @@ xfs_trans_ichgtime(
struct timespec64 tv;
ASSERT(tp);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
tv = current_time(inode);
@@ -88,7 +88,7 @@ xfs_trans_log_inode(
struct inode *inode = VFS_I(ip);
ASSERT(ip->i_itemp != NULL);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
/*
* Don't bother with i_lock for the I_DIRTY_TIME check here, as races
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index d223e1ae90a6..74d9d00d45ef 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -862,7 +862,7 @@ xfs_qm_dqget_inode(
if (error)
return error;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(xfs_inode_dquot(ip, type) == NULL);
id = xfs_qm_id_for_quotatype(ip, type);
@@ -919,7 +919,7 @@ xfs_qm_dqget_inode(
}
dqret:
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
trace_xfs_dqget_miss(dqp);
*O_dqpp = dqp;
return 0;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 80874c80df6d..a19c6ddaea6f 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1574,7 +1574,7 @@ xfs_itruncate_extents_flags(
xfs_filblks_t unmap_len;
int error = 0;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
xfs_isilocked(ip, XFS_IOLOCK_EXCL));
ASSERT(new_size <= XFS_ISIZE(ip));
@@ -2805,7 +2805,7 @@ xfs_ifree(
int error;
struct xfs_icluster xic = { 0 };
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(VFS_I(ip)->i_nlink == 0);
ASSERT(ip->i_d.di_nextents == 0);
ASSERT(ip->i_d.di_anextents == 0);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 8bd5d0de6321..6396d7b2038c 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -440,7 +440,7 @@ xfs_inode_item_pin(
{
struct xfs_inode *ip = INODE_ITEM(lip)->ili_inode;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
trace_xfs_inode_pin(ip, _RET_IP_);
atomic_inc(&ip->i_pincount);
@@ -574,7 +574,7 @@ xfs_inode_item_release(
unsigned short lock_flags;
ASSERT(ip->i_itemp != NULL);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
lock_flags = iip->ili_lock_flags;
iip->ili_lock_flags = 0;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 81f2f93caec0..eba2ec2a59f1 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -598,7 +598,7 @@ xfs_setattr_mode(
struct inode *inode = VFS_I(ip);
umode_t mode = iattr->ia_mode;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
inode->i_mode &= S_IFMT;
inode->i_mode |= mode & ~S_IFMT;
@@ -611,7 +611,7 @@ xfs_setattr_time(
{
struct inode *inode = VFS_I(ip);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (iattr->ia_valid & ATTR_ATIME)
inode->i_atime = iattr->ia_atime;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 0b0909657bad..dfe155cbaa55 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -253,7 +253,7 @@ xfs_qm_dqattach_one(
struct xfs_dquot *dqp;
int error;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
error = 0;
/*
@@ -323,7 +323,7 @@ xfs_qm_dqattach_locked(
if (!xfs_qm_need_dqattach(ip))
return 0;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (XFS_IS_UQUOTA_ON(mp) && !ip->i_udquot) {
error = xfs_qm_dqattach_one(ip, ip->i_d.di_uid, XFS_DQ_USER,
@@ -354,7 +354,7 @@ xfs_qm_dqattach_locked(
* Don't worry about the dquots that we may have attached before any
* error - they'll get detached later if it has not already been done.
*/
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
return error;
}
@@ -1754,7 +1754,7 @@ xfs_qm_vop_chown(
XFS_TRANS_DQ_RTBCOUNT : XFS_TRANS_DQ_BCOUNT;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(XFS_IS_QUOTA_RUNNING(ip->i_mount));
/* old dquot */
@@ -1915,7 +1915,7 @@ xfs_qm_vop_create_dqattach(
if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
return;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(XFS_IS_QUOTA_RUNNING(mp));
if (udqp && XFS_IS_UQUOTA_ON(mp)) {
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index b0ce04ffd3cd..1d570ad1cfc9 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -359,7 +359,7 @@ xfs_reflink_allocate_cow(
xfs_filblks_t resaligned;
xfs_extlen_t resblks = 0;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (!ip->i_cowfp) {
ASSERT(!xfs_is_reflink_inode(ip));
xfs_ifork_init_cow(ip);
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 6209e7b6b895..35906b3c2825 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1121,7 +1121,7 @@ xfs_rtallocate_extent(
xfs_fsblock_t sb; /* summary file block number */
xfs_buf_t *sumbp; /* summary file block buffer */
- ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
ASSERT(minlen > 0 && minlen <= maxlen);
/*
@@ -1278,7 +1278,7 @@ xfs_rtpick_extent(
uint64_t seq; /* sequence number of file creation */
uint64_t *seqp; /* pointer to seqno in inode */
- ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
seqp = (uint64_t *)&VFS_I(mp->m_rbmip)->i_atime;
if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM)) {
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index d1b9869bc5fa..ed3a78e6a295 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -808,7 +808,7 @@ xfs_trans_reserve_quota_nblks(
ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
XFS_TRANS_DQ_RES_RTBLKS ||
(flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-03 17:58 ` [PATCH v2 2/7] xfs: Update checking excl. locks for ilock Pavel Reichl
@ 2020-02-03 23:16 ` Dave Chinner
2020-02-04 6:21 ` Christoph Hellwig
1 sibling, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:16 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:45PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_attr_remote.c | 2 +-
> fs/xfs/libxfs/xfs_bmap.c | 10 +++++-----
> fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
> fs/xfs/libxfs/xfs_trans_inode.c | 6 +++---
> fs/xfs/xfs_dquot.c | 4 ++--
> fs/xfs/xfs_inode.c | 4 ++--
> fs/xfs/xfs_inode_item.c | 4 ++--
> fs/xfs/xfs_iops.c | 4 ++--
> fs/xfs/xfs_qm.c | 10 +++++-----
> fs/xfs/xfs_reflink.c | 2 +-
> fs/xfs/xfs_rtalloc.c | 4 ++--
> fs/xfs/xfs_trans_dquot.c | 2 +-
> 12 files changed, 27 insertions(+), 27 deletions(-)
Looks fine, though you could combine this with the next patch.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-03 17:58 ` [PATCH v2 2/7] xfs: Update checking excl. locks for ilock Pavel Reichl
2020-02-03 23:16 ` Dave Chinner
@ 2020-02-04 6:21 ` Christoph Hellwig
2020-02-04 16:08 ` Eric Sandeen
1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-02-04 6:21 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> + ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
I think this is a very bad interface. Either we keep our good old
xfs_isilocked that just operates on the inode and lock flags, or
we use something that gets the actual lock passed. But an interface
that encodes the lock in both the function called and the flags, and
one that doesn't follow neither the XFS lock flags conventions nor
the core kernel convention is just not very useful.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-04 6:21 ` Christoph Hellwig
@ 2020-02-04 16:08 ` Eric Sandeen
2020-02-04 21:06 ` Dave Chinner
0 siblings, 1 reply; 21+ messages in thread
From: Eric Sandeen @ 2020-02-04 16:08 UTC (permalink / raw)
To: Christoph Hellwig, Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On 2/4/20 12:21 AM, Christoph Hellwig wrote:
>> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>> + ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
>
> I think this is a very bad interface. Either we keep our good old
> xfs_isilocked that just operates on the inode and lock flags, or
> we use something that gets the actual lock passed. But an interface
> that encodes the lock in both the function called and the flags, and
> one that doesn't follow neither the XFS lock flags conventions nor
> the core kernel convention is just not very useful.
I think this came out of Dave's suggestion on the previous patchset,
but I agree with you Chrisoph. Even if there is a future reason to
split it out into a function for each type, I don't see a reason to
do it now, and this interface is awkward.
I'd prefer to keep xfs_isilocked() with the current calling convention and
just change its internals to use lockdep. Dave spotted a bug in the
current implementation, but I think that can be fixed.
Splitting out the 3 lock testing functions seems to me like complexity
creep that doesn't need to be in this series.
Dave, thoughts?
-Eric
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-04 16:08 ` Eric Sandeen
@ 2020-02-04 21:06 ` Dave Chinner
2020-02-04 22:20 ` Eric Sandeen
0 siblings, 1 reply; 21+ messages in thread
From: Dave Chinner @ 2020-02-04 21:06 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, Pavel Reichl, linux-xfs, Dave Chinner
On Tue, Feb 04, 2020 at 10:08:45AM -0600, Eric Sandeen wrote:
> On 2/4/20 12:21 AM, Christoph Hellwig wrote:
> >> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> >> + ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
> >
> > I think this is a very bad interface. Either we keep our good old
> > xfs_isilocked that just operates on the inode and lock flags, or
> > we use something that gets the actual lock passed. But an interface
> > that encodes the lock in both the function called and the flags, and
> > one that doesn't follow neither the XFS lock flags conventions nor
> > the core kernel convention is just not very useful.
>
> I think this came out of Dave's suggestion on the previous patchset,
> but I agree with you Chrisoph. Even if there is a future reason to
> split it out into a function for each type, I don't see a reason to
> do it now, and this interface is awkward.
>
> I'd prefer to keep xfs_isilocked() with the current calling convention and
> just change its internals to use lockdep. Dave spotted a bug in the
> current implementation, but I think that can be fixed.
>
> Splitting out the 3 lock testing functions seems to me like complexity
> creep that doesn't need to be in this series.
>
> Dave, thoughts?
All I care about is that we get rid of the mrlock_t. I'm not
interested in bikeshedding the details to death. I've put my 2c
worth in, if you don't like it, then that's fine and I'm not going
to get upset about that.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] xfs: Update checking excl. locks for ilock
2020-02-04 21:06 ` Dave Chinner
@ 2020-02-04 22:20 ` Eric Sandeen
0 siblings, 0 replies; 21+ messages in thread
From: Eric Sandeen @ 2020-02-04 22:20 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, Pavel Reichl, linux-xfs, Dave Chinner
On 2/4/20 3:06 PM, Dave Chinner wrote:
> On Tue, Feb 04, 2020 at 10:08:45AM -0600, Eric Sandeen wrote:
>> On 2/4/20 12:21 AM, Christoph Hellwig wrote:
>>>> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>>>> + ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
>>>
>>> I think this is a very bad interface. Either we keep our good old
>>> xfs_isilocked that just operates on the inode and lock flags, or
>>> we use something that gets the actual lock passed. But an interface
>>> that encodes the lock in both the function called and the flags, and
>>> one that doesn't follow neither the XFS lock flags conventions nor
>>> the core kernel convention is just not very useful.
>>
>> I think this came out of Dave's suggestion on the previous patchset,
>> but I agree with you Chrisoph. Even if there is a future reason to
>> split it out into a function for each type, I don't see a reason to
>> do it now, and this interface is awkward.
>>
>> I'd prefer to keep xfs_isilocked() with the current calling convention and
>> just change its internals to use lockdep. Dave spotted a bug in the
>> current implementation, but I think that can be fixed.
>>
>> Splitting out the 3 lock testing functions seems to me like complexity
>> creep that doesn't need to be in this series.
>>
>> Dave, thoughts?
>
> All I care about is that we get rid of the mrlock_t. I'm not
> interested in bikeshedding the details to death. I've put my 2c
> worth in, if you don't like it, then that's fine and I'm not going
> to get upset about that.
In the short term I'd suggest something like this. It keeps the helper,
but we don't have to change the callsites, and breaking out separate types
etc can be done after the mrlock removal patchset is done - in a separate
series if/when needed.
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(
struct xfs_inode *ip,
uint lock_flags)
{
if (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)) {
return __xfs_rwsem_islocked(&ip->i_mmlock.mr_lock,
(lock_flags & XFS_MMAPLOCK_SHARED),
(lock_flags & XFS_MMAPLOCK_EXCL));
}
if (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));
}
}
That still has the problem/bug with the existing
ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
callsite since it doesn't actually test both types but again that is a separate
bugfix - it could be changed to accumulate a true/false state for all the flags
in the lock_flags argument in another bugfix patch.
-Eric
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/7] xfs: Update checking read or write locks for ilock
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
2020-02-03 17:58 ` [PATCH v2 1/7] xfs: Add xfs_is_{i,io,mmap}locked functions Pavel Reichl
2020-02-03 17:58 ` [PATCH v2 2/7] xfs: Update checking excl. locks for ilock Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 23:18 ` Dave Chinner
2020-02-03 17:58 ` [PATCH v2 4/7] xfs: Update checking for iolock Pavel Reichl
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 2 +-
fs/xfs/libxfs/xfs_bmap.c | 2 +-
fs/xfs/libxfs/xfs_inode_fork.c | 2 +-
fs/xfs/xfs_attr_list.c | 2 +-
fs/xfs/xfs_inode.c | 6 +++---
fs/xfs/xfs_qm.c | 2 +-
fs/xfs/xfs_symlink.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index e6149720ce02..e692225d2e64 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -107,7 +107,7 @@ xfs_attr_get_ilocked(
struct xfs_inode *ip,
struct xfs_da_args *args)
{
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
if (!xfs_inode_hasattr(ip))
return -ENOATTR;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 318c006b4b50..86a9fe2a7629 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3906,7 +3906,7 @@ xfs_bmapi_read(
ASSERT(*nmap >= 1);
ASSERT(!(flags & ~(XFS_BMAPI_ATTRFORK|XFS_BMAPI_ENTIRE|
XFS_BMAPI_COWFORK)));
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index ad2b9c313fd2..ffb5731a73da 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -560,7 +560,7 @@ xfs_iextents_copy(
struct xfs_bmbt_irec rec;
int64_t copied = 0;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT(ifp->if_bytes > 0);
for_each_xfs_iext(ifp, &icur, &rec) {
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index d37743bdf274..a52539a0fd63 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -511,7 +511,7 @@ xfs_attr_list_int_ilocked(
{
struct xfs_inode *dp = context->dp;
- ASSERT(xfs_isilocked(dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
/*
* Decide on what work routines to call based on the inode size.
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index a19c6ddaea6f..d7cb2886ca81 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2859,7 +2859,7 @@ static void
xfs_iunpin(
struct xfs_inode *ip)
{
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
trace_xfs_inode_unpin_nowait(ip, _RET_IP_);
@@ -3723,7 +3723,7 @@ xfs_iflush(
XFS_STATS_INC(mp, xs_iflush_count);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT(xfs_isiflocked(ip));
ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
ip->i_d.di_nextents > XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK));
@@ -3855,7 +3855,7 @@ xfs_iflush_int(
struct xfs_dinode *dip;
struct xfs_mount *mp = ip->i_mount;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT(xfs_isiflocked(ip));
ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
ip->i_d.di_nextents > XFS_IFORK_MAXEXT(ip, XFS_DATA_FORK));
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index dfe155cbaa55..757c8cc00e39 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1802,7 +1802,7 @@ xfs_qm_vop_chown_reserve(
int error;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT(XFS_IS_QUOTA_RUNNING(mp));
delblks = ip->i_delayed_blks;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index d762d42ed0ff..20179f173688 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -41,7 +41,7 @@ xfs_readlink_bmap_ilocked(
int fsblocks = 0;
int offset;
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
fsblocks = xfs_symlink_blocks(mp, pathlen);
error = xfs_bmapi_read(ip, 0, fsblocks, mval, &nmaps, 0);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 3/7] xfs: Update checking read or write locks for ilock
2020-02-03 17:58 ` [PATCH v2 3/7] xfs: Update checking read or write " Pavel Reichl
@ 2020-02-03 23:18 ` Dave Chinner
0 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:18 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:46PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
BTW, for these aPI conversions, you don't really need a suggested-by
tag on them. The first patch taht introduces the API, yes, but the
mechanical conversions don't really need it.
Otherwise, looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/7] xfs: Update checking for iolock
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
` (2 preceding siblings ...)
2020-02-03 17:58 ` [PATCH v2 3/7] xfs: Update checking read or write " Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 23:24 ` Dave Chinner
2020-02-03 17:58 ` [PATCH v2 5/7] xfs: Update checking for mmaplock Pavel Reichl
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 4 ++--
fs/xfs/xfs_bmap_util.c | 4 ++--
fs/xfs/xfs_file.c | 3 ++-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_iops.c | 2 +-
5 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 86a9fe2a7629..c3638552b3c0 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5699,7 +5699,7 @@ xfs_bmse_merge(
blockcount = left->br_blockcount + got->br_blockcount;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(xfs_bmse_can_merge(left, got, shift));
@@ -5904,7 +5904,7 @@ xfs_bmap_can_insert_extents(
int is_empty;
int error = 0;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
if (XFS_FORCED_SHUTDOWN(ip->i_mount))
return -EIO;
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index e62fb5216341..ae0bccb2288f 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1065,7 +1065,7 @@ xfs_collapse_file_space(
uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
bool done = false;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
trace_xfs_collapse_file_space(ip);
@@ -1133,7 +1133,7 @@ xfs_insert_file_space(
xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
bool done = false;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
trace_xfs_insert_file_space(ip);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index b8a4a3f29b36..9b3958ca73d9 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -770,7 +770,8 @@ xfs_break_layouts(
bool retry;
int error;
- ASSERT(xfs_isilocked(XFS_I(inode), XFS_IOLOCK_SHARED|XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(XFS_I(inode),
+ XFS_IOLOCK_SHARED|XFS_IOLOCK_EXCL));
do {
retry = false;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d7cb2886ca81..328a3b4ffbd2 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1576,7 +1576,7 @@ xfs_itruncate_extents_flags(
ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
- xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(new_size <= XFS_ISIZE(ip));
ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
ASSERT(ip->i_itemp != NULL);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index eba2ec2a59f1..aad255521514 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -865,7 +865,7 @@ xfs_setattr_size(
uint lock_flags = 0;
bool did_zeroing = false;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
ASSERT(S_ISREG(inode->i_mode));
ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 4/7] xfs: Update checking for iolock
2020-02-03 17:58 ` [PATCH v2 4/7] xfs: Update checking for iolock Pavel Reichl
@ 2020-02-03 23:24 ` Dave Chinner
0 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:24 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:47PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 4 ++--
> fs/xfs/xfs_bmap_util.c | 4 ++--
> fs/xfs/xfs_file.c | 3 ++-
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_iops.c | 2 +-
> 5 files changed, 8 insertions(+), 7 deletions(-)
....
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index b8a4a3f29b36..9b3958ca73d9 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -770,7 +770,8 @@ xfs_break_layouts(
> bool retry;
> int error;
>
> - ASSERT(xfs_isilocked(XFS_I(inode), XFS_IOLOCK_SHARED|XFS_IOLOCK_EXCL));
> + ASSERT(xfs_is_iolocked(XFS_I(inode),
> + XFS_IOLOCK_SHARED|XFS_IOLOCK_EXCL));
Whitespace: XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL
Otherwise looks ok.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 5/7] xfs: Update checking for mmaplock
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
` (3 preceding siblings ...)
2020-02-03 17:58 ` [PATCH v2 4/7] xfs: Update checking for iolock Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 23:25 ` Dave Chinner
2020-02-03 17:58 ` [PATCH v2 6/7] xfs: update excl. lock check for IOLOCK and ILOCK Pavel Reichl
2020-02-03 17:58 ` [PATCH v2 7/7] xfs: Replace mrlock_t by rw_semaphore Pavel Reichl
6 siblings, 1 reply; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_bmap_util.c | 4 ++--
fs/xfs/xfs_file.c | 2 +-
fs/xfs/xfs_iops.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index ae0bccb2288f..377389fadf5a 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1066,7 +1066,7 @@ xfs_collapse_file_space(
bool done = false;
ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
- ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
+ ASSERT(xfs_is_mmaplocked(ip, XFS_MMAPLOCK_EXCL));
trace_xfs_collapse_file_space(ip);
@@ -1134,7 +1134,7 @@ xfs_insert_file_space(
bool done = false;
ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
- ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
+ ASSERT(xfs_is_mmaplocked(ip, XFS_MMAPLOCK_EXCL));
trace_xfs_insert_file_space(ip);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 9b3958ca73d9..a4dbd9a6f45a 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -749,7 +749,7 @@ xfs_break_dax_layouts(
{
struct page *page;
- ASSERT(xfs_isilocked(XFS_I(inode), XFS_MMAPLOCK_EXCL));
+ ASSERT(xfs_is_mmaplocked(XFS_I(inode), XFS_MMAPLOCK_EXCL));
page = dax_layout_busy_page(inode->i_mapping);
if (!page)
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index aad255521514..67a0f940b30e 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -866,7 +866,7 @@ xfs_setattr_size(
bool did_zeroing = false;
ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
- ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
+ ASSERT(xfs_is_mmaplocked(ip, XFS_MMAPLOCK_EXCL));
ASSERT(S_ISREG(inode->i_mode));
ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 5/7] xfs: Update checking for mmaplock
2020-02-03 17:58 ` [PATCH v2 5/7] xfs: Update checking for mmaplock Pavel Reichl
@ 2020-02-03 23:25 ` Dave Chinner
0 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:25 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:48PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_bmap_util.c | 4 ++--
> fs/xfs/xfs_file.c | 2 +-
> fs/xfs/xfs_iops.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 6/7] xfs: update excl. lock check for IOLOCK and ILOCK
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
` (4 preceding siblings ...)
2020-02-03 17:58 ` [PATCH v2 5/7] xfs: Update checking for mmaplock Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
2020-02-03 23:35 ` Dave Chinner
2020-02-03 17:58 ` [PATCH v2 7/7] xfs: Replace mrlock_t by rw_semaphore Pavel Reichl
6 siblings, 1 reply; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index c3638552b3c0..2d371f87e890 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5829,7 +5829,8 @@ xfs_bmap_collapse_extents(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) ||
+ xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (!(ifp->if_flags & XFS_IFEXTENTS)) {
error = xfs_iread_extents(tp, ip, whichfork);
@@ -5946,7 +5947,8 @@ xfs_bmap_insert_extents(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
+ ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) ||
+ xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
if (!(ifp->if_flags & XFS_IFEXTENTS)) {
error = xfs_iread_extents(tp, ip, whichfork);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 6/7] xfs: update excl. lock check for IOLOCK and ILOCK
2020-02-03 17:58 ` [PATCH v2 6/7] xfs: update excl. lock check for IOLOCK and ILOCK Pavel Reichl
@ 2020-02-03 23:35 ` Dave Chinner
0 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2020-02-03 23:35 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs, Dave Chinner
On Mon, Feb 03, 2020 at 06:58:49PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index c3638552b3c0..2d371f87e890 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5829,7 +5829,8 @@ xfs_bmap_collapse_extents(
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
> + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) ||
> + xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
Hmmm. I think this is incorrect - a bug in the original code in that
xfs_isilocked() will only check one lock type and so this never
worked as intended.
That is, we should have both the IOLOCK and the ILOCK held here.
The IOLOCK is taken by the high level xfs_file_fallocate() code to
lock out IO, while ILOCK is taken cwinside the transaction to make
the metadata changes atomic.
Hence I think this should actually end up as:
ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL));
ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
>
> if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> error = xfs_iread_extents(tp, ip, whichfork);
> @@ -5946,7 +5947,8 @@ xfs_bmap_insert_extents(
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
> + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) ||
> + xfs_is_ilocked(ip, XFS_ILOCK_EXCL));
Same here.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 7/7] xfs: Replace mrlock_t by rw_semaphore
2020-02-03 17:58 [PATCH v2 0/7] xfs: Remove wrappers for some semaphores Pavel Reichl
` (5 preceding siblings ...)
2020-02-03 17:58 ` [PATCH v2 6/7] xfs: update excl. lock check for IOLOCK and ILOCK Pavel Reichl
@ 2020-02-03 17:58 ` Pavel Reichl
6 siblings, 0 replies; 21+ messages in thread
From: Pavel Reichl @ 2020-02-03 17:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Pavel Reichl, Dave Chinner
Remove mrlock_t as it does not provide any extra value over rw_semaphores.
Make i_lock and i_mmaplock native rw_semaphores and replace mr*() functions
with native rwsem calls.
Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/mrlock.h | 78 ----------------------------------------------
fs/xfs/xfs_inode.c | 65 +++++++++++---------------------------
fs/xfs/xfs_inode.h | 7 +++--
fs/xfs/xfs_iops.c | 4 +--
fs/xfs/xfs_linux.h | 1 -
fs/xfs/xfs_super.c | 6 ++--
6 files changed, 27 insertions(+), 134 deletions(-)
delete mode 100644 fs/xfs/mrlock.h
diff --git a/fs/xfs/mrlock.h b/fs/xfs/mrlock.h
deleted file mode 100644
index 79155eec341b..000000000000
--- a/fs/xfs/mrlock.h
+++ /dev/null
@@ -1,78 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2000-2006 Silicon Graphics, Inc.
- * All Rights Reserved.
- */
-#ifndef __XFS_SUPPORT_MRLOCK_H__
-#define __XFS_SUPPORT_MRLOCK_H__
-
-#include <linux/rwsem.h>
-
-typedef struct {
- struct rw_semaphore mr_lock;
-#if defined(DEBUG) || defined(XFS_WARN)
- int mr_writer;
-#endif
-} mrlock_t;
-
-#if defined(DEBUG) || defined(XFS_WARN)
-#define mrinit(mrp, name) \
- do { (mrp)->mr_writer = 0; init_rwsem(&(mrp)->mr_lock); } while (0)
-#else
-#define mrinit(mrp, name) \
- do { init_rwsem(&(mrp)->mr_lock); } while (0)
-#endif
-
-#define mrlock_init(mrp, t,n,s) mrinit(mrp, n)
-#define mrfree(mrp) do { } while (0)
-
-static inline void mraccess_nested(mrlock_t *mrp, int subclass)
-{
- down_read_nested(&mrp->mr_lock, subclass);
-}
-
-static inline void mrupdate_nested(mrlock_t *mrp, int subclass)
-{
- down_write_nested(&mrp->mr_lock, subclass);
-#if defined(DEBUG) || defined(XFS_WARN)
- mrp->mr_writer = 1;
-#endif
-}
-
-static inline int mrtryaccess(mrlock_t *mrp)
-{
- return down_read_trylock(&mrp->mr_lock);
-}
-
-static inline int mrtryupdate(mrlock_t *mrp)
-{
- if (!down_write_trylock(&mrp->mr_lock))
- return 0;
-#if defined(DEBUG) || defined(XFS_WARN)
- mrp->mr_writer = 1;
-#endif
- return 1;
-}
-
-static inline void mrunlock_excl(mrlock_t *mrp)
-{
-#if defined(DEBUG) || defined(XFS_WARN)
- mrp->mr_writer = 0;
-#endif
- up_write(&mrp->mr_lock);
-}
-
-static inline void mrunlock_shared(mrlock_t *mrp)
-{
- up_read(&mrp->mr_lock);
-}
-
-static inline void mrdemote(mrlock_t *mrp)
-{
-#if defined(DEBUG) || defined(XFS_WARN)
- mrp->mr_writer = 0;
-#endif
- downgrade_write(&mrp->mr_lock);
-}
-
-#endif /* __XFS_SUPPORT_MRLOCK_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 328a3b4ffbd2..ca54e2e151c0 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -191,14 +191,15 @@ xfs_ilock(
}
if (lock_flags & XFS_MMAPLOCK_EXCL)
- mrupdate_nested(&ip->i_mmaplock, XFS_MMAPLOCK_DEP(lock_flags));
+ down_write_nested(&ip->i_mmaplock,
+ XFS_MMAPLOCK_DEP(lock_flags));
else if (lock_flags & XFS_MMAPLOCK_SHARED)
- mraccess_nested(&ip->i_mmaplock, XFS_MMAPLOCK_DEP(lock_flags));
+ down_read_nested(&ip->i_mmaplock, XFS_MMAPLOCK_DEP(lock_flags));
if (lock_flags & XFS_ILOCK_EXCL)
- mrupdate_nested(&ip->i_lock, XFS_ILOCK_DEP(lock_flags));
+ down_write_nested(&ip->i_lock, XFS_ILOCK_DEP(lock_flags));
else if (lock_flags & XFS_ILOCK_SHARED)
- mraccess_nested(&ip->i_lock, XFS_ILOCK_DEP(lock_flags));
+ down_read_nested(&ip->i_lock, XFS_ILOCK_DEP(lock_flags));
}
/*
@@ -242,27 +243,27 @@ xfs_ilock_nowait(
}
if (lock_flags & XFS_MMAPLOCK_EXCL) {
- if (!mrtryupdate(&ip->i_mmaplock))
+ if (!down_write_trylock(&ip->i_mmaplock))
goto out_undo_iolock;
} else if (lock_flags & XFS_MMAPLOCK_SHARED) {
- if (!mrtryaccess(&ip->i_mmaplock))
+ if (!down_read_trylock(&ip->i_mmaplock))
goto out_undo_iolock;
}
if (lock_flags & XFS_ILOCK_EXCL) {
- if (!mrtryupdate(&ip->i_lock))
+ if (!down_write_trylock(&ip->i_lock))
goto out_undo_mmaplock;
} else if (lock_flags & XFS_ILOCK_SHARED) {
- if (!mrtryaccess(&ip->i_lock))
+ if (!down_read_trylock(&ip->i_lock))
goto out_undo_mmaplock;
}
return 1;
out_undo_mmaplock:
if (lock_flags & XFS_MMAPLOCK_EXCL)
- mrunlock_excl(&ip->i_mmaplock);
+ up_write(&ip->i_mmaplock);
else if (lock_flags & XFS_MMAPLOCK_SHARED)
- mrunlock_shared(&ip->i_mmaplock);
+ up_read(&ip->i_mmaplock);
out_undo_iolock:
if (lock_flags & XFS_IOLOCK_EXCL)
up_write(&VFS_I(ip)->i_rwsem);
@@ -309,14 +310,14 @@ xfs_iunlock(
up_read(&VFS_I(ip)->i_rwsem);
if (lock_flags & XFS_MMAPLOCK_EXCL)
- mrunlock_excl(&ip->i_mmaplock);
+ up_write(&ip->i_mmaplock);
else if (lock_flags & XFS_MMAPLOCK_SHARED)
- mrunlock_shared(&ip->i_mmaplock);
+ up_read(&ip->i_mmaplock);
if (lock_flags & XFS_ILOCK_EXCL)
- mrunlock_excl(&ip->i_lock);
+ up_write(&ip->i_lock);
else if (lock_flags & XFS_ILOCK_SHARED)
- mrunlock_shared(&ip->i_lock);
+ up_read(&ip->i_lock);
trace_xfs_iunlock(ip, lock_flags, _RET_IP_);
}
@@ -335,9 +336,9 @@ xfs_ilock_demote(
~(XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL|XFS_ILOCK_EXCL)) == 0);
if (lock_flags & XFS_ILOCK_EXCL)
- mrdemote(&ip->i_lock);
+ downgrade_write(&ip->i_lock);
if (lock_flags & XFS_MMAPLOCK_EXCL)
- mrdemote(&ip->i_mmaplock);
+ downgrade_write(&ip->i_mmaplock);
if (lock_flags & XFS_IOLOCK_EXCL)
downgrade_write(&VFS_I(ip)->i_rwsem);
@@ -345,34 +346,6 @@ xfs_ilock_demote(
}
#if defined(DEBUG) || defined(XFS_WARN)
-int
-xfs_isilocked(
- xfs_inode_t *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_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_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(0);
- return 0;
-}
-
static inline bool
__xfs_is_ilocked(
struct rw_semaphore *rwsem,
@@ -401,7 +374,7 @@ xfs_is_ilocked(
struct xfs_inode *ip,
int lock_flags)
{
- return __xfs_is_ilocked(&ip->i_lock.mr_lock,
+ return __xfs_is_ilocked(&ip->i_lock,
(lock_flags & XFS_ILOCK_SHARED),
(lock_flags & XFS_ILOCK_EXCL));
}
@@ -411,7 +384,7 @@ xfs_is_mmaplocked(
struct xfs_inode *ip,
int lock_flags)
{
- return __xfs_is_ilocked(&ip->i_mmaplock.mr_lock,
+ return __xfs_is_ilocked(&ip->i_mmaplock,
(lock_flags & XFS_MMAPLOCK_SHARED),
(lock_flags & XFS_MMAPLOCK_EXCL));
}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 6ba575f35c1f..d7c77d1d7930 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -9,6 +9,8 @@
#include "xfs_inode_buf.h"
#include "xfs_inode_fork.h"
+#include <linux/rwsem.h>
+
/*
* Kernel only inode definitions
*/
@@ -39,8 +41,8 @@ typedef struct xfs_inode {
/* Transaction and locking information. */
struct xfs_inode_log_item *i_itemp; /* logging information */
- mrlock_t i_lock; /* inode lock */
- mrlock_t i_mmaplock; /* inode mmap IO lock */
+ struct rw_semaphore i_lock; /* inode lock */
+ struct rw_semaphore i_mmaplock; /* inode mmap IO lock */
atomic_t i_pincount; /* inode pin count */
/*
@@ -416,7 +418,6 @@ 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_is_ilocked(struct xfs_inode *, int);
bool xfs_is_mmaplocked(struct xfs_inode *, int);
bool xfs_is_iolocked(struct xfs_inode *, int);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 67a0f940b30e..bb3a2a57f04a 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1319,9 +1319,9 @@ xfs_setup_inode(
*/
lockdep_set_class(&inode->i_rwsem,
&inode->i_sb->s_type->i_mutex_dir_key);
- lockdep_set_class(&ip->i_lock.mr_lock, &xfs_dir_ilock_class);
+ lockdep_set_class(&ip->i_lock, &xfs_dir_ilock_class);
} else {
- lockdep_set_class(&ip->i_lock.mr_lock, &xfs_nondir_ilock_class);
+ lockdep_set_class(&ip->i_lock, &xfs_nondir_ilock_class);
}
/*
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 8738bb03f253..921a3eb093ed 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -22,7 +22,6 @@ typedef __u32 xfs_nlink_t;
#include "xfs_types.h"
#include "kmem.h"
-#include "mrlock.h"
#include <linux/semaphore.h>
#include <linux/mm.h>
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 760901783944..1289ce1f4e9e 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -661,10 +661,8 @@ xfs_fs_inode_init_once(
atomic_set(&ip->i_pincount, 0);
spin_lock_init(&ip->i_flags_lock);
- mrlock_init(&ip->i_mmaplock, MRLOCK_ALLOW_EQUAL_PRI|MRLOCK_BARRIER,
- "xfsino", ip->i_ino);
- mrlock_init(&ip->i_lock, MRLOCK_ALLOW_EQUAL_PRI|MRLOCK_BARRIER,
- "xfsino", ip->i_ino);
+ init_rwsem(&ip->i_mmaplock);
+ init_rwsem(&ip->i_lock);
}
/*
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread