linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ira.weiny@intel.com
To: linux-kernel@vger.kernel.org
Cc: Ira Weiny <ira.weiny@intel.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>,
	"Theodore Y. Ts'o" <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: [PATCH v3 09/12] fs/xfs: Add write DAX lock to xfs layer
Date: Sat,  8 Feb 2020 11:34:42 -0800	[thread overview]
Message-ID: <20200208193445.27421-10-ira.weiny@intel.com> (raw)
In-Reply-To: <20200208193445.27421-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

XFS requires regular files to be locked for write while changing to/from
DAX state.

Take the DAX write lock while changing DAX state.

We define a new XFS_DAX_EXCL lock type to carry the lock through to
transaction completion.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes from V2:
	Change name of patch (WAS: fs/xfs: Add lock/unlock state to xfs)
	Remove the xfs specific lock and move to the vfs layer.
		We still use XFS_LOCK_DAX_EXCL to be able to pass this
		flag through to the transaction code.  But we no longer
		have a lock specific to xfs.  This removes a lot of code
		from the XFS layer, preps us for using this in ext4, and
		is actually more straight forward now that all the
		locking requirements are better known.

	Fix locking order comment
	Rework for new 'state' names
	(Other comments on the previous patch are not applicable with
	new patch as much of the code was removed in favor of the vfs
	level lock)

 fs/xfs/xfs_inode.c | 22 ++++++++++++++++++++--
 fs/xfs/xfs_inode.h |  7 +++++--
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 35df324875db..0c7b1855e0c8 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -142,12 +142,12 @@ xfs_ilock_attr_map_shared(
  *
  * Basic locking order:
  *
- * i_rwsem -> i_mmap_lock -> page_lock -> i_ilock
+ * s_dax_sem -> i_rwsem -> i_mmap_lock -> page_lock -> i_ilock
  *
  * mmap_sem locking order:
  *
  * i_rwsem -> page lock -> mmap_sem
- * mmap_sem -> i_mmap_lock -> page_lock
+ * s_dax_sem -> mmap_sem -> i_mmap_lock -> page_lock
  *
  * The difference in mmap_sem locking order mean that we cannot hold the
  * i_mmap_lock over syscall based read(2)/write(2) based IO. These IO paths can
@@ -182,6 +182,9 @@ xfs_ilock(
 	       (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
 	ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_SUBCLASS_MASK)) == 0);
 
+	if (lock_flags & XFS_DAX_EXCL)
+		inode_dax_state_down_write(VFS_I(ip));
+
 	if (lock_flags & XFS_IOLOCK_EXCL) {
 		down_write_nested(&VFS_I(ip)->i_rwsem,
 				  XFS_IOLOCK_DEP(lock_flags));
@@ -224,6 +227,8 @@ xfs_ilock_nowait(
 	 * 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.
+	 *
+	 * XFS_DAX_* is not allowed
 	 */
 	ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
 	       (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
@@ -232,6 +237,7 @@ xfs_ilock_nowait(
 	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 & XFS_DAX_EXCL) == 0);
 
 	if (lock_flags & XFS_IOLOCK_EXCL) {
 		if (!down_write_trylock(&VFS_I(ip)->i_rwsem))
@@ -318,6 +324,9 @@ xfs_iunlock(
 	else if (lock_flags & XFS_ILOCK_SHARED)
 		mrunlock_shared(&ip->i_lock);
 
+	if (lock_flags & XFS_DAX_EXCL)
+		inode_dax_state_up_write(VFS_I(ip));
+
 	trace_xfs_iunlock(ip, lock_flags, _RET_IP_);
 }
 
@@ -333,6 +342,8 @@ xfs_ilock_demote(
 	ASSERT(lock_flags & (XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL|XFS_ILOCK_EXCL));
 	ASSERT((lock_flags &
 		~(XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL|XFS_ILOCK_EXCL)) == 0);
+	/* XFS_DAX_* is not allowed */
+	ASSERT((lock_flags & XFS_DAX_EXCL) == 0);
 
 	if (lock_flags & XFS_ILOCK_EXCL)
 		mrdemote(&ip->i_lock);
@@ -465,6 +476,9 @@ xfs_lock_inodes(
 	ASSERT(!(lock_mode & XFS_ILOCK_EXCL) ||
 		inodes <= XFS_ILOCK_MAX_SUBCLASS + 1);
 
+	/* XFS_DAX_* is not allowed */
+	ASSERT((lock_mode & XFS_DAX_EXCL) == 0);
+
 	if (lock_mode & XFS_IOLOCK_EXCL) {
 		ASSERT(!(lock_mode & (XFS_MMAPLOCK_EXCL | XFS_ILOCK_EXCL)));
 	} else if (lock_mode & XFS_MMAPLOCK_EXCL)
@@ -566,6 +580,10 @@ xfs_lock_two_inodes(
 	ASSERT(!(ip0_mode & (XFS_MMAPLOCK_SHARED|XFS_MMAPLOCK_EXCL)) ||
 	       !(ip1_mode & (XFS_ILOCK_SHARED|XFS_ILOCK_EXCL)));
 
+	/* XFS_DAX_* is not allowed */
+	ASSERT((ip0_mode & XFS_DAX_EXCL) == 0);
+	ASSERT((ip1_mode & XFS_DAX_EXCL) == 0);
+
 	ASSERT(ip0->i_ino != ip1->i_ino);
 
 	if (ip0->i_ino > ip1->i_ino) {
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 492e53992fa9..25fe20740bf7 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -278,10 +278,12 @@ static inline void xfs_ifunlock(struct xfs_inode *ip)
 #define	XFS_ILOCK_SHARED	(1<<3)
 #define	XFS_MMAPLOCK_EXCL	(1<<4)
 #define	XFS_MMAPLOCK_SHARED	(1<<5)
+#define	XFS_DAX_EXCL		(1<<6)
 
 #define XFS_LOCK_MASK		(XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED \
 				| XFS_ILOCK_EXCL | XFS_ILOCK_SHARED \
-				| XFS_MMAPLOCK_EXCL | XFS_MMAPLOCK_SHARED)
+				| XFS_MMAPLOCK_EXCL | XFS_MMAPLOCK_SHARED \
+				| XFS_DAX_EXCL)
 
 #define XFS_LOCK_FLAGS \
 	{ XFS_IOLOCK_EXCL,	"IOLOCK_EXCL" }, \
@@ -289,7 +291,8 @@ static inline void xfs_ifunlock(struct xfs_inode *ip)
 	{ XFS_ILOCK_EXCL,	"ILOCK_EXCL" }, \
 	{ XFS_ILOCK_SHARED,	"ILOCK_SHARED" }, \
 	{ XFS_MMAPLOCK_EXCL,	"MMAPLOCK_EXCL" }, \
-	{ XFS_MMAPLOCK_SHARED,	"MMAPLOCK_SHARED" }
+	{ XFS_MMAPLOCK_SHARED,	"MMAPLOCK_SHARED" }, \
+	{ XFS_DAX_EXCL,		"DAX_EXCL" }
 
 
 /*
-- 
2.21.0


  parent reply	other threads:[~2020-02-08 19:35 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-08 19:34 [PATCH v3 00/12] Enable per-file/directory DAX operations V3 ira.weiny
2020-02-08 19:34 ` [PATCH v3 01/12] fs/stat: Define DAX statx attribute ira.weiny
2020-02-08 19:34 ` [PATCH v3 02/12] fs/xfs: Isolate the physical DAX flag from effective ira.weiny
2020-02-08 19:34 ` [PATCH v3 03/12] fs/xfs: Separate functionality of xfs_inode_supports_dax() ira.weiny
2020-02-11  5:47   ` Dave Chinner
2020-02-11 16:13     ` Ira Weiny
2020-02-08 19:34 ` [PATCH v3 04/12] fs/xfs: Clean up DAX support check ira.weiny
2020-02-11  5:57   ` Dave Chinner
2020-02-11 16:28     ` Ira Weiny
2020-02-11 20:38       ` Dave Chinner
2020-02-08 19:34 ` [PATCH v3 05/12] fs: remove unneeded IS_DAX() check ira.weiny
2020-02-11  5:34   ` Dave Chinner
2020-02-11 16:38     ` Ira Weiny
2020-02-11 20:41       ` Dave Chinner
2020-02-12 16:04         ` Ira Weiny
2020-02-08 19:34 ` [PATCH v3 06/12] fs/xfs: Check if the inode supports DAX under lock ira.weiny
2020-02-11  6:16   ` Dave Chinner
2020-02-11 17:55     ` Ira Weiny
2020-02-11 20:42       ` Dave Chinner
2020-02-12 16:10         ` Ira Weiny
2020-02-08 19:34 ` [PATCH v3 07/12] fs: Add locking for a dynamic DAX state ira.weiny
2020-02-11  8:00   ` Dave Chinner
2020-02-11 20:14     ` Ira Weiny
2020-02-11 20:59       ` Dan Williams
2020-02-11 21:49       ` Dave Chinner
2020-02-12  6:31         ` Darrick J. Wong
2020-02-08 19:34 ` [PATCH v3 08/12] fs/xfs: Clarify lockdep dependency for xfs_isilocked() ira.weiny
2020-02-08 19:34 ` ira.weiny [this message]
2020-02-08 19:34 ` [PATCH v3 10/12] fs: Prevent DAX state change if file is mmap'ed ira.weiny
2020-02-08 19:34 ` [PATCH v3 11/12] fs/xfs: Clean up locking in dax invalidate ira.weiny
2020-02-08 19:34 ` [PATCH v3 12/12] fs/xfs: Allow toggle of effective DAX flag ira.weiny
2020-02-10 15:15 ` [PATCH v3 00/12] Enable per-file/directory DAX operations V3 Jeff Moyer
2020-02-11 20:17   ` Ira Weiny
2020-02-12 19:49     ` Jeff Moyer
2020-02-13 19:01       ` Ira Weiny
2020-02-13 19:05         ` Ira Weiny
2020-02-13 19:58           ` Darrick J. Wong
2020-02-13 23:29             ` Ira Weiny
2020-02-14  0:16               ` Dan Williams
2020-02-14 20:06                 ` Ira Weiny
2020-02-14 21:23                   ` Jeff Moyer
2020-02-14 21:58                     ` Ira Weiny
2020-02-14 22:06                       ` Jeff Moyer
2020-02-14 22:58                         ` Jeff Moyer
2020-02-14 23:03                           ` Jeff Moyer
2020-02-18  2:35                           ` Ira Weiny
2020-02-18 14:22                             ` Jeff Moyer
2020-02-18 23:54                               ` Ira Weiny
2020-02-20 16:20                                 ` Ira Weiny
2020-02-20 16:30                                   ` Darrick J. Wong
2020-02-20 16:49                                     ` Ira Weiny
2020-02-20 17:00                                       ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200208193445.27421-10-ira.weiny@intel.com \
    --to=ira.weiny@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).