All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH 0/2] xfs: random fixes for lock flags
@ 2022-06-17  2:30 xiakaixu1987
  2022-06-17  2:30 ` [RESEND PATCH 1/2] xfs: factor out the common lock flags assert xiakaixu1987
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: xiakaixu1987 @ 2022-06-17  2:30 UTC (permalink / raw)
  To: linux-xfs; +Cc: djwong, david, hch, Kaixu Xia

From: Kaixu Xia <kaixuxia@tencent.com>

Hi,

This patchset include some patches to factor out lock flags
and fix the mmap_lock state check.

Kaixu Xia (2):
  xfs: factor out the common lock flags assert
  xfs: use invalidate_lock to check the state of mmap_lock

 fs/xfs/xfs_inode.c | 64 ++++++++++++++++++----------------------------
 1 file changed, 25 insertions(+), 39 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RESEND PATCH 1/2] xfs: factor out the common lock flags assert
  2022-06-17  2:30 [RESEND PATCH 0/2] xfs: random fixes for lock flags xiakaixu1987
@ 2022-06-17  2:30 ` xiakaixu1987
  2022-06-17 22:04   ` Dave Chinner
  2022-06-17  2:30 ` [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock xiakaixu1987
  2022-06-22 23:28 ` [RESEND PATCH 0/2] xfs: random fixes for lock flags Darrick J. Wong
  2 siblings, 1 reply; 6+ messages in thread
From: xiakaixu1987 @ 2022-06-17  2:30 UTC (permalink / raw)
  To: linux-xfs; +Cc: djwong, david, hch, Kaixu Xia

From: Kaixu Xia <kaixuxia@tencent.com>

There are similar lock flags assert in xfs_ilock(), xfs_ilock_nowait(),
xfs_iunlock(), thus we can factor it out into a helper that is clear.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_inode.c | 60 ++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 37 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 52d6f2c7d58b..8b8bac7eba8c 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -131,6 +131,26 @@ xfs_ilock_attr_map_shared(
 	return lock_mode;
 }
 
+/*
+ * You can't set both SHARED and EXCL for the same lock,
+ * and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_MMAPLOCK_SHARED,
+ * XFS_MMAPLOCK_EXCL, XFS_ILOCK_SHARED, XFS_ILOCK_EXCL are valid values
+ * to set in lock_flags.
+ */
+static inline void
+xfs_lock_flags_assert(
+	uint		lock_flags)
+{
+	ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
+		(XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
+	ASSERT((lock_flags & (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL)) !=
+		(XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL));
+	ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
+		(XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
+	ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_SUBCLASS_MASK)) == 0);
+	ASSERT(lock_flags != 0);
+}
+
 /*
  * In addition to i_rwsem in the VFS inode, the xfs inode contains 2
  * multi-reader locks: invalidate_lock and the i_lock.  This routine allows
@@ -168,18 +188,7 @@ xfs_ilock(
 {
 	trace_xfs_ilock(ip, lock_flags, _RET_IP_);
 
-	/*
-	 * You can't set both SHARED and EXCL for the same lock,
-	 * and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED,
-	 * and XFS_ILOCK_EXCL are valid values to set in lock_flags.
-	 */
-	ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
-	       (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL)) !=
-	       (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
-	       (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
-	ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_SUBCLASS_MASK)) == 0);
+	xfs_lock_flags_assert(lock_flags);
 
 	if (lock_flags & XFS_IOLOCK_EXCL) {
 		down_write_nested(&VFS_I(ip)->i_rwsem,
@@ -222,18 +231,7 @@ xfs_ilock_nowait(
 {
 	trace_xfs_ilock_nowait(ip, lock_flags, _RET_IP_);
 
-	/*
-	 * You can't set both SHARED and EXCL for the same lock,
-	 * and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED,
-	 * and XFS_ILOCK_EXCL are valid values to set in lock_flags.
-	 */
-	ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
-	       (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL)) !=
-	       (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
-	       (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
-	ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_SUBCLASS_MASK)) == 0);
+	xfs_lock_flags_assert(lock_flags);
 
 	if (lock_flags & XFS_IOLOCK_EXCL) {
 		if (!down_write_trylock(&VFS_I(ip)->i_rwsem))
@@ -291,19 +289,7 @@ xfs_iunlock(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
 {
-	/*
-	 * You can't set both SHARED and EXCL for the same lock,
-	 * and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED,
-	 * and XFS_ILOCK_EXCL are valid values to set in lock_flags.
-	 */
-	ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
-	       (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL)) !=
-	       (XFS_MMAPLOCK_SHARED | XFS_MMAPLOCK_EXCL));
-	ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
-	       (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
-	ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_SUBCLASS_MASK)) == 0);
-	ASSERT(lock_flags != 0);
+	xfs_lock_flags_assert(lock_flags);
 
 	if (lock_flags & XFS_IOLOCK_EXCL)
 		up_write(&VFS_I(ip)->i_rwsem);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock
  2022-06-17  2:30 [RESEND PATCH 0/2] xfs: random fixes for lock flags xiakaixu1987
  2022-06-17  2:30 ` [RESEND PATCH 1/2] xfs: factor out the common lock flags assert xiakaixu1987
@ 2022-06-17  2:30 ` xiakaixu1987
  2022-06-17 22:06   ` Dave Chinner
  2022-06-22 23:28 ` [RESEND PATCH 0/2] xfs: random fixes for lock flags Darrick J. Wong
  2 siblings, 1 reply; 6+ messages in thread
From: xiakaixu1987 @ 2022-06-17  2:30 UTC (permalink / raw)
  To: linux-xfs; +Cc: djwong, david, hch, Kaixu Xia

From: Kaixu Xia <kaixuxia@tencent.com>

We should use invalidate_lock and XFS_MMAPLOCK_SHARED to check the state
of mmap_lock rw_semaphore in xfs_isilocked(), rather than i_rwsem and
XFS_IOLOCK_SHARED.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 8b8bac7eba8c..3e1c62ffa4f7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -365,8 +365,8 @@ xfs_isilocked(
 	}
 
 	if (lock_flags & (XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED)) {
-		return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem,
-				(lock_flags & XFS_IOLOCK_SHARED));
+		return __xfs_rwsem_islocked(&VFS_I(ip)->i_mapping->invalidate_lock,
+				(lock_flags & XFS_MMAPLOCK_SHARED));
 	}
 
 	if (lock_flags & (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED)) {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RESEND PATCH 1/2] xfs: factor out the common lock flags assert
  2022-06-17  2:30 ` [RESEND PATCH 1/2] xfs: factor out the common lock flags assert xiakaixu1987
@ 2022-06-17 22:04   ` Dave Chinner
  0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2022-06-17 22:04 UTC (permalink / raw)
  To: xiakaixu1987; +Cc: linux-xfs, djwong, hch, Kaixu Xia

On Fri, Jun 17, 2022 at 10:30:33AM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> There are similar lock flags assert in xfs_ilock(), xfs_ilock_nowait(),
> xfs_iunlock(), thus we can factor it out into a helper that is clear.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_inode.c | 60 ++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 37 deletions(-)

looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock
  2022-06-17  2:30 ` [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock xiakaixu1987
@ 2022-06-17 22:06   ` Dave Chinner
  0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2022-06-17 22:06 UTC (permalink / raw)
  To: xiakaixu1987; +Cc: linux-xfs, djwong, hch, Kaixu Xia

On Fri, Jun 17, 2022 at 10:30:34AM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> We should use invalidate_lock and XFS_MMAPLOCK_SHARED to check the state
> of mmap_lock rw_semaphore in xfs_isilocked(), rather than i_rwsem and
> XFS_IOLOCK_SHARED.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 8b8bac7eba8c..3e1c62ffa4f7 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -365,8 +365,8 @@ xfs_isilocked(
>  	}
>  
>  	if (lock_flags & (XFS_MMAPLOCK_EXCL|XFS_MMAPLOCK_SHARED)) {
> -		return __xfs_rwsem_islocked(&VFS_I(ip)->i_rwsem,
> -				(lock_flags & XFS_IOLOCK_SHARED));
> +		return __xfs_rwsem_islocked(&VFS_I(ip)->i_mapping->invalidate_lock,
> +				(lock_flags & XFS_MMAPLOCK_SHARED));

Looks good.

Fixes: 2433480a7e1d ("xfs: Convert to use invalidate_lock")

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RESEND PATCH 0/2] xfs: random fixes for lock flags
  2022-06-17  2:30 [RESEND PATCH 0/2] xfs: random fixes for lock flags xiakaixu1987
  2022-06-17  2:30 ` [RESEND PATCH 1/2] xfs: factor out the common lock flags assert xiakaixu1987
  2022-06-17  2:30 ` [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock xiakaixu1987
@ 2022-06-22 23:28 ` Darrick J. Wong
  2 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2022-06-22 23:28 UTC (permalink / raw)
  To: xiakaixu1987; +Cc: linux-xfs, david, hch, Kaixu Xia

On Fri, Jun 17, 2022 at 10:30:32AM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> Hi,
> 
> This patchset include some patches to factor out lock flags
> and fix the mmap_lock state check.

Funny that I didn't notice during review and nobody ever tripped
debugging assertions.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> Kaixu Xia (2):
>   xfs: factor out the common lock flags assert
>   xfs: use invalidate_lock to check the state of mmap_lock
> 
>  fs/xfs/xfs_inode.c | 64 ++++++++++++++++++----------------------------
>  1 file changed, 25 insertions(+), 39 deletions(-)
> 
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-22 23:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17  2:30 [RESEND PATCH 0/2] xfs: random fixes for lock flags xiakaixu1987
2022-06-17  2:30 ` [RESEND PATCH 1/2] xfs: factor out the common lock flags assert xiakaixu1987
2022-06-17 22:04   ` Dave Chinner
2022-06-17  2:30 ` [RESEND PATCH 2/2] xfs: use invalidate_lock to check the state of mmap_lock xiakaixu1987
2022-06-17 22:06   ` Dave Chinner
2022-06-22 23:28 ` [RESEND PATCH 0/2] xfs: random fixes for lock flags Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.