linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: linux-kernel@vger.kernel.org
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.cz>,
	linux-nvdimm@lists.01.org, linux-xfs@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH 1/2] xfs: always use DAX if mount option is used
Date: Thu,  7 Sep 2017 15:08:31 -0600	[thread overview]
Message-ID: <20170907210832.17969-2-ross.zwisler@linux.intel.com> (raw)
In-Reply-To: <20170907210832.17969-1-ross.zwisler@linux.intel.com>

Before support for the per-inode DAX flag was disabled the XFS the code had
an issue where the user couldn't reliably tell whether or not DAX was being
used to service page faults and I/O when the DAX mount option was used.  In
this case each inode within the mounted filesystem started with S_DAX set
due to the mount option, but it could be cleared if someone touched the
individual inode flag.

For example (v4.13 and before):

  # mount | grep dax
  /dev/pmem0 on /mnt type xfs
  (rw,relatime,seclabel,attr2,dax,inode64,sunit=4096,swidth=4096,noquota)

  # touch /mnt/a /mnt/b   # both files currently use DAX

  # xfs_io -c "lsattr" /mnt/*  # neither has the DAX inode option set
  ----------e----- /mnt/a
  ----------e----- /mnt/b

  # xfs_io -c "chattr -x" /mnt/a  # this clears S_DAX for /mnt/a

  # xfs_io -c "lsattr" /mnt/*
  ----------e----- /mnt/a
  ----------e----- /mnt/b

We end up with both /mnt/a and /mnt/b looking identical from the point of
view of the mount option and from lsattr, but one is using DAX and the
other is not.

Fix this by always doing DAX I/O when either the mount option is set or
when the DAX inode flag is set.  This means that DAX will always be used
for all inodes on a filesystem mounted with -o dax, making the usage
reliable and detectable.

This does not fix the race issues that caused the XFS DAX inode option to
be disabled, so that option will still be disabled.  If/when we re-enable
it, though, I think we will want this issue to have been fixed.  I also do
think that we want to fix this in stable kernels.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
CC: stable@vger.kernel.org
---
 fs/xfs/xfs_ioctl.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 5049e8a..26faeb9 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1013,7 +1013,7 @@ xfs_diflags_to_linux(
 	else
 		inode->i_flags &= ~S_NOATIME;
 #if 0	/* disabled until the flag switching races are sorted out */
-	if (xflags & FS_XFLAG_DAX)
+	if ((xflags & FS_XFLAG_DAX) || (ip->i_mount->m_flags & XFS_MOUNT_DAX))
 		inode->i_flags |= S_DAX;
 	else
 		inode->i_flags &= ~S_DAX;
@@ -1104,7 +1104,14 @@ xfs_ioctl_setattr_dax_invalidate(
 			return -EINVAL;
 	}
 
-	/* If the DAX state is not changing, we have nothing to do here. */
+	/*
+	 * If the DAX state is not changing, we have nothing to do here.  If
+	 * the DAX mount option was used we will update the DAX inode flag as
+	 * the user requested but we will continue to use DAX for I/O and page
+	 * faults regardless of how the inode flag is set.
+	 */
+	if (ip->i_mount->m_flags & XFS_MOUNT_DAX)
+		return 0;
 	if ((fa->fsx_xflags & FS_XFLAG_DAX) && IS_DAX(inode))
 		return 0;
 	if (!(fa->fsx_xflags & FS_XFLAG_DAX) && !IS_DAX(inode))
-- 
2.9.5


  reply	other threads:[~2017-09-07 21:08 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 21:08 [PATCH 0/2] xfs: some DAX fixes Ross Zwisler
2017-09-07 21:08 ` Ross Zwisler [this message]
2017-09-08  7:20   ` [PATCH 1/2] xfs: always use DAX if mount option is used Christoph Hellwig
2017-09-08 15:28     ` Ross Zwisler
2017-09-08 21:21       ` [PATCH] xfs: add regression test for DAX mount option usage Ross Zwisler
2017-09-11 15:16         ` Ross Zwisler
2017-09-11 15:37           ` Dan Williams
2017-09-11 20:01             ` [fstests PATCH v2] " Ross Zwisler
2017-09-14  6:57               ` Eryu Guan
2017-09-15 22:42                 ` Ross Zwisler
2017-09-16 22:26                   ` Dave Chinner
2017-09-12  6:44         ` [PATCH] " Dave Chinner
2017-09-12 15:38           ` Ross Zwisler
2017-09-12 23:47             ` Dave Chinner
2017-09-13 14:42               ` Ross Zwisler
2017-09-13 22:01                 ` Dave Chinner
2017-09-13 22:23                   ` Dan Williams
2017-09-13 23:34                     ` Dave Chinner
2017-09-14  0:28                       ` Dan Williams
2017-09-14  0:40                         ` Dave Chinner
2017-09-14  1:24                           ` Dan Williams
2017-09-14 12:19                             ` Jeff Moyer
2017-09-14 13:16                               ` Johannes Thumshirn
2017-09-14 14:10                                 ` Dan Williams
2017-09-15  9:18                                   ` Johannes Thumshirn
2017-09-15 17:39                                     ` Dan Williams
2017-09-18  7:47                                       ` Johannes Thumshirn
2017-09-07 21:08 ` [PATCH 2/2] xfs: validate bdev support for DAX inode flag Ross Zwisler
2017-09-08  7:21   ` Christoph Hellwig

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=20170907210832.17969-2-ross.zwisler@linux.intel.com \
    --to=ross.zwisler@linux.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-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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).