[PATCH v4 0/3] minimal DAX support for XFS realtime device

[PATCH v4 0/3] minimal DAX support for XFS realtime device

[Dave Jiang]

Darrick,
After reading the comments from you, Dave Chinner, and Dan, it looks like
the dyanmic S_DAX flag support won't be coming or not any time soon at the
least. Here are the the collection of patches so far to address yours and
Dave C's comments for minimal support. Please let me know what else I am
missing. Thanks!

v3:
- Removed setting of error return in ext2 and ext4 per Ross's comments
- Rebased against 4.16-rc1 with updates

---

Darrick J. Wong (1):
      fs: allow per-device dax status checking for filesystems

Dave Jiang (2):
      dax: change bdev_dax_supported() to support boolean returns
      xfs: reject removal of realtime flag when datadev doesn't support DAX


 drivers/dax/super.c |   27 ++++++++++++++-------------
 fs/ext2/super.c     |    3 +--
 fs/ext4/super.c     |    3 +--
 fs/xfs/xfs_ioctl.c  |   18 +++++++++++++++++-
 fs/xfs/xfs_iops.c   |   30 +++++++++++++++++++++++++-----
 fs/xfs/xfs_super.c  |   11 +++++++++--
 include/linux/dax.h |   18 +++++++++++++-----
 7 files changed, 80 insertions(+), 30 deletions(-)

--
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[Dave Jiang]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor __bdev_dax_supported into a sb_dax_supported helper for
single-bdev filesystems and a regular bdev_dax_supported that takes a
bdev parameter.  This enables multi-device filesystems like xfs to check
that a dax device can work for the particular filesystem.  Once that's
in place, actually fix all the parts of XFS where we need to be able to
distinguish between datadev and rtdev.

This patch fixes the problem where we screw up the dax support checking
in xfs if the datadev and rtdev have different dax capabilities.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 drivers/dax/super.c |    9 +++++----
 fs/ext2/super.c     |    2 +-
 fs/ext4/super.c     |    2 +-
 fs/xfs/xfs_ioctl.c  |    3 ++-
 fs/xfs/xfs_iops.c   |   30 +++++++++++++++++++++++++-----
 fs/xfs/xfs_super.c  |   11 +++++++++--
 include/linux/dax.h |   16 ++++++++++++----
 7 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 473af694ad1c..8a3a168a267f 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -73,8 +73,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
 #endif
 
 /**
- * __bdev_dax_supported() - Check if the device supports dax for filesystem
+ * bdev_dax_supported() - Check if the device supports dax for filesystem
  * @sb: The superblock of the device
+ * @bdev: block device to check
  * @blocksize: The block size of the device
  *
  * This is a library function for filesystems to check if the block device
@@ -82,9 +83,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
  *
  * Return: negative errno if unsupported, 0 if supported.
  */
-int __bdev_dax_supported(struct super_block *sb, int blocksize)
+int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
+		       int blocksize)
 {
-	struct block_device *bdev = sb->s_bdev;
 	struct dax_device *dax_dev;
 	pgoff_t pgoff;
 	int err, id;
@@ -135,7 +136,7 @@ int __bdev_dax_supported(struct super_block *sb, int blocksize)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(__bdev_dax_supported);
+EXPORT_SYMBOL_GPL(bdev_dax_supported);
 #endif
 
 enum dax_device_flags {
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 7666c065b96f..28ca694e9e92 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -961,7 +961,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
 
 	if (sbi->s_mount_opt & EXT2_MOUNT_DAX) {
-		err = bdev_dax_supported(sb, blocksize);
+		err = sb_dax_supported(sb, blocksize);
 		if (err) {
 			ext2_msg(sb, KERN_ERR,
 				"DAX unsupported by block device. Turning off DAX.");
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 39bf464c35f1..a09f832aad35 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3714,7 +3714,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 					" that may contain inline data");
 			sbi->s_mount_opt &= ~EXT4_MOUNT_DAX;
 		}
-		err = bdev_dax_supported(sb, blocksize);
+		err = sb_dax_supported(sb, blocksize);
 		if (err) {
 			ext4_msg(sb, KERN_ERR,
 				"DAX unsupported by block device. Turning off DAX.");
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 89fb1eb80aae..277355f5c084 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1103,7 +1103,8 @@ xfs_ioctl_setattr_dax_invalidate(
 	if (fa->fsx_xflags & FS_XFLAG_DAX) {
 		if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
 			return -EINVAL;
-		if (bdev_dax_supported(sb, sb->s_blocksize) < 0)
+		if (bdev_dax_supported(sb, xfs_find_bdev_for_inode(VFS_I(ip)),
+				sb->s_blocksize) < 0)
 			return -EINVAL;
 	}
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 56475fcd76f2..66cd61c172af 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1182,6 +1182,30 @@ static const struct inode_operations xfs_inline_symlink_inode_operations = {
 	.update_time		= xfs_vn_update_time,
 };
 
+/* Figure out if this file actually supports DAX. */
+static bool
+xfs_inode_supports_dax(
+	struct xfs_inode	*ip)
+{
+	struct xfs_mount	*mp = ip->i_mount;
+
+	/* Only supported on non-reflinked files. */
+	if (!S_ISREG(VFS_I(ip)->i_mode) || xfs_is_reflink_inode(ip))
+		return false;
+
+	/* DAX mount option or DAX iflag must be set. */
+	if (!(mp->m_flags & XFS_MOUNT_DAX) &&
+	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_DAX))
+		return false;
+
+	/* Block size must match page size */
+	if (mp->m_sb.sb_blocksize != PAGE_SIZE)
+		return false;
+
+	/* Device has to support DAX too. */
+	return xfs_find_daxdev_for_inode(VFS_I(ip)) != NULL;
+}
+
 STATIC void
 xfs_diflags_to_iflags(
 	struct inode		*inode,
@@ -1200,11 +1224,7 @@ xfs_diflags_to_iflags(
 		inode->i_flags |= S_SYNC;
 	if (flags & XFS_DIFLAG_NOATIME)
 		inode->i_flags |= S_NOATIME;
-	if (S_ISREG(inode->i_mode) &&
-	    ip->i_mount->m_sb.sb_blocksize == PAGE_SIZE &&
-	    !xfs_is_reflink_inode(ip) &&
-	    (ip->i_mount->m_flags & XFS_MOUNT_DAX ||
-	     ip->i_d.di_flags2 & XFS_DIFLAG2_DAX))
+	if (xfs_inode_supports_dax(ip))
 		inode->i_flags |= S_DAX;
 }
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7aba628dc527..f6c74ef9722c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1657,11 +1657,18 @@ xfs_fs_fill_super(
 		sb->s_flags |= SB_I_VERSION;
 
 	if (mp->m_flags & XFS_MOUNT_DAX) {
+		int	error2 = 0;
+
 		xfs_warn(mp,
 		"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
 
-		error = bdev_dax_supported(sb, sb->s_blocksize);
-		if (error) {
+		error = bdev_dax_supported(sb, mp->m_ddev_targp->bt_bdev,
+				sb->s_blocksize);
+		if (mp->m_rtdev_targp)
+			error2 = bdev_dax_supported(sb,
+					mp->m_rtdev_targp->bt_bdev,
+					sb->s_blocksize);
+		if (error && error2) {
 			xfs_alert(mp,
 			"DAX unsupported by block device. Turning off DAX.");
 			mp->m_flags &= ~XFS_MOUNT_DAX;
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 0185ecdae135..e640c8a19ec0 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -40,10 +40,11 @@ static inline void put_dax(struct dax_device *dax_dev)
 
 int bdev_dax_pgoff(struct block_device *, sector_t, size_t, pgoff_t *pgoff);
 #if IS_ENABLED(CONFIG_FS_DAX)
-int __bdev_dax_supported(struct super_block *sb, int blocksize);
-static inline int bdev_dax_supported(struct super_block *sb, int blocksize)
+int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
+		       int blocksize);
+static inline int sb_dax_supported(struct super_block *sb, int blocksize)
 {
-	return __bdev_dax_supported(sb, blocksize);
+	return bdev_dax_supported(sb, sb->s_bdev, blocksize);
 }
 
 static inline struct dax_device *fs_dax_get_by_host(const char *host)
@@ -58,7 +59,14 @@ static inline void fs_put_dax(struct dax_device *dax_dev)
 
 struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev);
 #else
-static inline int bdev_dax_supported(struct super_block *sb, int blocksize)
+static inline int bdev_dax_supported(struct super_block *sb,
+				     struct block_device *bdev,
+				     int blocksize)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int sb_dax_supported(struct super_block *sb, int blocksize)
 {
 	return -EOPNOTSUPP;
 }

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v4 2/3] dax: change bdev_dax_supported() to support boolean returns

[Dave Jiang]

The function return values are confusing with the way the function is
named. We expect a true or false return value but it actually returns
0/-errno.  This makes the code very confusing. Changing the return values
to return a bool where if DAX is supported then return true and no DAX
support returns false.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dax/super.c |   20 ++++++++++----------
 fs/ext2/super.c     |    3 +--
 fs/ext4/super.c     |    3 +--
 fs/xfs/xfs_ioctl.c  |    4 ++--
 fs/xfs/xfs_super.c  |   10 +++++-----
 include/linux/dax.h |   12 ++++++------
 6 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 8a3a168a267f..42e58eea305e 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -81,9 +81,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
  * This is a library function for filesystems to check if the block device
  * can be mounted with dax option.
  *
- * Return: negative errno if unsupported, 0 if supported.
+ * Return: true if supported, false if unsupported
  */
-int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
+bool bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
 		       int blocksize)
 {
 	struct dax_device *dax_dev;
@@ -96,21 +96,21 @@ int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
 	if (blocksize != PAGE_SIZE) {
 		pr_debug("VFS (%s): error: unsupported blocksize for dax\n",
 				sb->s_id);
-		return -EINVAL;
+		return false;
 	}
 
 	err = bdev_dax_pgoff(bdev, 0, PAGE_SIZE, &pgoff);
 	if (err) {
-		pr_debug("VFS (%s): error: unaligned partition for dax\n",
-				sb->s_id);
-		return err;
+		pr_debug("VFS (%s): error: unaligned partition for dax: %d\n",
+				sb->s_id, err);
+		return false;
 	}
 
 	dax_dev = dax_get_by_host(bdev->bd_disk->disk_name);
 	if (!dax_dev) {
 		pr_debug("VFS (%s): error: device does not support dax\n",
 				sb->s_id);
-		return -EOPNOTSUPP;
+		return false;
 	}
 
 	id = dax_read_lock();
@@ -122,7 +122,7 @@ int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
 	if (len < 1) {
 		pr_debug("VFS (%s): error: dax access failed (%ld)\n",
 				sb->s_id, len);
-		return len < 0 ? len : -EIO;
+		return false;
 	}
 
 	if ((IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(pfn))
@@ -131,10 +131,10 @@ int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
 	else {
 		pr_debug("VFS (%s): error: dax support not enabled\n",
 				sb->s_id);
-		return -EOPNOTSUPP;
+		return false;
 	}
 
-	return 0;
+	return true;
 }
 EXPORT_SYMBOL_GPL(bdev_dax_supported);
 #endif
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 28ca694e9e92..6901ba1d65d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -961,8 +961,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
 
 	if (sbi->s_mount_opt & EXT2_MOUNT_DAX) {
-		err = sb_dax_supported(sb, blocksize);
-		if (err) {
+		if(!sb_dax_supported(sb, blocksize)) {
 			ext2_msg(sb, KERN_ERR,
 				"DAX unsupported by block device. Turning off DAX.");
 			sbi->s_mount_opt &= ~EXT2_MOUNT_DAX;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a09f832aad35..8b4ee5f5c17b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3714,8 +3714,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 					" that may contain inline data");
 			sbi->s_mount_opt &= ~EXT4_MOUNT_DAX;
 		}
-		err = sb_dax_supported(sb, blocksize);
-		if (err) {
+		if (!sb_dax_supported(sb, blocksize)) {
 			ext4_msg(sb, KERN_ERR,
 				"DAX unsupported by block device. Turning off DAX.");
 			sbi->s_mount_opt &= ~EXT4_MOUNT_DAX;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 277355f5c084..a6a0efc35c76 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1103,8 +1103,8 @@ xfs_ioctl_setattr_dax_invalidate(
 	if (fa->fsx_xflags & FS_XFLAG_DAX) {
 		if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
 			return -EINVAL;
-		if (bdev_dax_supported(sb, xfs_find_bdev_for_inode(VFS_I(ip)),
-				sb->s_blocksize) < 0)
+		if (!bdev_dax_supported(sb, xfs_find_bdev_for_inode(VFS_I(ip)),
+				sb->s_blocksize))
 			return -EINVAL;
 	}
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f6c74ef9722c..80658e0f01c2 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1657,18 +1657,18 @@ xfs_fs_fill_super(
 		sb->s_flags |= SB_I_VERSION;
 
 	if (mp->m_flags & XFS_MOUNT_DAX) {
-		int	error2 = 0;
+		bool rtdev_is_dax, datadev_is_dax;
 
 		xfs_warn(mp,
 		"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
 
-		error = bdev_dax_supported(sb, mp->m_ddev_targp->bt_bdev,
-				sb->s_blocksize);
+		datadev_is_dax = bdev_dax_supported(sb,
+				mp->m_ddev_targp->bt_bdev, sb->s_blocksize);
 		if (mp->m_rtdev_targp)
-			error2 = bdev_dax_supported(sb,
+			rtdev_is_dax = bdev_dax_supported(sb,
 					mp->m_rtdev_targp->bt_bdev,
 					sb->s_blocksize);
-		if (error && error2) {
+		if (!rtdev_is_dax && !datadev_is_dax) {
 			xfs_alert(mp,
 			"DAX unsupported by block device. Turning off DAX.");
 			mp->m_flags &= ~XFS_MOUNT_DAX;
diff --git a/include/linux/dax.h b/include/linux/dax.h
index e640c8a19ec0..f733edd3a9dc 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -40,9 +40,9 @@ static inline void put_dax(struct dax_device *dax_dev)
 
 int bdev_dax_pgoff(struct block_device *, sector_t, size_t, pgoff_t *pgoff);
 #if IS_ENABLED(CONFIG_FS_DAX)
-int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
+bool bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
 		       int blocksize);
-static inline int sb_dax_supported(struct super_block *sb, int blocksize)
+static inline bool sb_dax_supported(struct super_block *sb, int blocksize)
 {
 	return bdev_dax_supported(sb, sb->s_bdev, blocksize);
 }
@@ -59,16 +59,16 @@ static inline void fs_put_dax(struct dax_device *dax_dev)
 
 struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev);
 #else
-static inline int bdev_dax_supported(struct super_block *sb,
+static inline bool bdev_dax_supported(struct super_block *sb,
 				     struct block_device *bdev,
 				     int blocksize)
 {
-	return -EOPNOTSUPP;
+	return false;
 }
 
-static inline int sb_dax_supported(struct super_block *sb, int blocksize)
+static inline bool sb_dax_supported(struct super_block *sb, int blocksize)
 {
-	return -EOPNOTSUPP;
+	return false;
 }
 
 static inline struct dax_device *fs_dax_get_by_host(const char *host)

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v4 3/3] xfs: reject removal of realtime flag when datadev doesn't support DAX

[Dave Jiang]

In a situation where the rt_dev is DAX and data_dev is not DAX, if the user
requests to remove the realtime flag via ioctl we can no longer support DAX
for that file. Dynamic changing of S_DAX on the inode is not supported due
to various complications in the existing implementation. Therefore until we
address the dynamic S_DAX change issues, we must disallow realtime flag
being removed.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 fs/xfs/xfs_ioctl.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index a6a0efc35c76..a2728f803e26 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1030,6 +1030,21 @@ xfs_ioctl_setattr_xflags(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	uint64_t		di_flags2;
+	struct inode		*inode = VFS_I(ip);
+	struct super_block	*sb = inode->i_sb;
+
+	/*
+	 * In the case that the inode is realtime, and we are trying to remove
+	 * the realtime flag, and the rtdev supports DAX but the datadev does
+	 * not support DAX, we can't allow the realtime flag to be removed
+	 * since we do not support dynamic S_DAX flag removal yet.
+	 */
+	if ((XFS_IS_REALTIME_INODE(ip)) &&
+	    !(fa->fsx_xflags & FS_XFLAG_REALTIME) &&
+	    bdev_dax_supported(sb, mp->m_rtdev_targp->bt_bdev,
+		    sb->s_blocksize) &&
+	    !bdev_dax_supported(sb, mp->m_ddev_targp->bt_bdev, sb->s_blocksize))
+		return -ENOTSUPP;
 
 	/* Can't change realtime flag if any extents are allocated. */
 	if ((ip->i_d.di_nextents || ip->i_delayed_blks) &&

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[Christoph Hellwig]

>  /**
> - * __bdev_dax_supported() - Check if the device supports dax for filesystem
> + * bdev_dax_supported() - Check if the device supports dax for filesystem
>   * @sb: The superblock of the device
> + * @bdev: block device to check
>   * @blocksize: The block size of the device
>   *
>   * This is a library function for filesystems to check if the block device
> @@ -82,9 +83,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
>   *
>   * Return: negative errno if unsupported, 0 if supported.
>   */
> -int __bdev_dax_supported(struct super_block *sb, int blocksize)
> +int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
> +		       int blocksize)
>  {
> -	struct block_device *bdev = sb->s_bdev;
>  	struct dax_device *dax_dev;
>  	pgoff_t pgoff;
>  	int err, id;

This now only uses sb for sb->s_id.  It might be better to use bdevname
to print the device name and don't bother passing a sb at all.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[Dave Jiang]

On 02/15/2018 10:52 AM, Christoph Hellwig wrote:
>>  /**
>> - * __bdev_dax_supported() - Check if the device supports dax for filesystem
>> + * bdev_dax_supported() - Check if the device supports dax for filesystem
>>   * @sb: The superblock of the device
>> + * @bdev: block device to check
>>   * @blocksize: The block size of the device
>>   *
>>   * This is a library function for filesystems to check if the block device
>> @@ -82,9 +83,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
>>   *
>>   * Return: negative errno if unsupported, 0 if supported.
>>   */
>> -int __bdev_dax_supported(struct super_block *sb, int blocksize)
>> +int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
>> +		       int blocksize)
>>  {
>> -	struct block_device *bdev = sb->s_bdev;
>>  	struct dax_device *dax_dev;
>>  	pgoff_t pgoff;
>>  	int err, id;
> 
> This now only uses sb for sb->s_id.  It might be better to use bdevname
> to print the device name and don't bother passing a sb at all.
> 

I think Darrick wanted to keep the API the same for non-XFS users. I'm
ok with dropping the passed in sb if Darrick has no objections.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[Christoph Hellwig]

On Thu, Feb 15, 2018 at 10:58:54AM -0700, Dave Jiang wrote:
> I think Darrick wanted to keep the API the same for non-XFS users. I'm
> ok with dropping the passed in sb if Darrick has no objections.

But the old API is gone - everyone either uses the new
sb_dax_supported, or the updated bdev_dax_supported.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[Darrick J. Wong]

On Thu, Feb 15, 2018 at 10:58:54AM -0700, Dave Jiang wrote:
> On 02/15/2018 10:52 AM, Christoph Hellwig wrote:
> >>  /**
> >> - * __bdev_dax_supported() - Check if the device supports dax for filesystem
> >> + * bdev_dax_supported() - Check if the device supports dax for filesystem
> >>   * @sb: The superblock of the device
> >> + * @bdev: block device to check
> >>   * @blocksize: The block size of the device
> >>   *
> >>   * This is a library function for filesystems to check if the block device
> >> @@ -82,9 +83,9 @@ EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
> >>   *
> >>   * Return: negative errno if unsupported, 0 if supported.
> >>   */
> >> -int __bdev_dax_supported(struct super_block *sb, int blocksize)
> >> +int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
> >> +		       int blocksize)
> >>  {
> >> -	struct block_device *bdev = sb->s_bdev;
> >>  	struct dax_device *dax_dev;
> >>  	pgoff_t pgoff;
> >>  	int err, id;
> > 
> > This now only uses sb for sb->s_id.  It might be better to use bdevname
> > to print the device name and don't bother passing a sb at all.
> > 
> 
> I think Darrick wanted to keep the API the same for non-XFS users. I'm
> ok with dropping the passed in sb if Darrick has no objections.

Fine with me to use bdevname.

--D

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 2/3] dax: change bdev_dax_supported() to support boolean returns

[kbuild test robot]

Hi Dave,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc1 next-20180216]
[cannot apply to dgc-xfs/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dave-Jiang/minimal-DAX-support-for-XFS-realtime-device/20180218-154220
config: i386-randconfig-x003-201807 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   fs//xfs/xfs_super.c: In function 'xfs_fs_fill_super':
>> fs//xfs/xfs_super.c:1660:8: warning: 'rtdev_is_dax' may be used uninitialized in this function [-Wmaybe-uninitialized]
      bool rtdev_is_dax, datadev_is_dax;
           ^~~~~~~~~~~~

vim +/rtdev_is_dax +1660 fs//xfs/xfs_super.c

  1566	
  1567	STATIC int
  1568	xfs_fs_fill_super(
  1569		struct super_block	*sb,
  1570		void			*data,
  1571		int			silent)
  1572	{
  1573		struct inode		*root;
  1574		struct xfs_mount	*mp = NULL;
  1575		int			flags = 0, error = -ENOMEM;
  1576	
  1577		mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
  1578		if (!mp)
  1579			goto out;
  1580	
  1581		spin_lock_init(&mp->m_sb_lock);
  1582		mutex_init(&mp->m_growlock);
  1583		atomic_set(&mp->m_active_trans, 0);
  1584		INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
  1585		INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
  1586		INIT_DELAYED_WORK(&mp->m_cowblocks_work, xfs_cowblocks_worker);
  1587		mp->m_kobj.kobject.kset = xfs_kset;
  1588	
  1589		mp->m_super = sb;
  1590		sb->s_fs_info = mp;
  1591	
  1592		error = xfs_parseargs(mp, (char *)data);
  1593		if (error)
  1594			goto out_free_fsname;
  1595	
  1596		sb_min_blocksize(sb, BBSIZE);
  1597		sb->s_xattr = xfs_xattr_handlers;
  1598		sb->s_export_op = &xfs_export_operations;
  1599	#ifdef CONFIG_XFS_QUOTA
  1600		sb->s_qcop = &xfs_quotactl_operations;
  1601		sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
  1602	#endif
  1603		sb->s_op = &xfs_super_operations;
  1604	
  1605		if (silent)
  1606			flags |= XFS_MFSI_QUIET;
  1607	
  1608		error = xfs_open_devices(mp);
  1609		if (error)
  1610			goto out_free_fsname;
  1611	
  1612		error = xfs_init_mount_workqueues(mp);
  1613		if (error)
  1614			goto out_close_devices;
  1615	
  1616		error = xfs_init_percpu_counters(mp);
  1617		if (error)
  1618			goto out_destroy_workqueues;
  1619	
  1620		/* Allocate stats memory before we do operations that might use it */
  1621		mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
  1622		if (!mp->m_stats.xs_stats) {
  1623			error = -ENOMEM;
  1624			goto out_destroy_counters;
  1625		}
  1626	
  1627		error = xfs_readsb(mp, flags);
  1628		if (error)
  1629			goto out_free_stats;
  1630	
  1631		error = xfs_finish_flags(mp);
  1632		if (error)
  1633			goto out_free_sb;
  1634	
  1635		error = xfs_setup_devices(mp);
  1636		if (error)
  1637			goto out_free_sb;
  1638	
  1639		error = xfs_filestream_mount(mp);
  1640		if (error)
  1641			goto out_free_sb;
  1642	
  1643		/*
  1644		 * we must configure the block size in the superblock before we run the
  1645		 * full mount process as the mount process can lookup and cache inodes.
  1646		 */
  1647		sb->s_magic = XFS_SB_MAGIC;
  1648		sb->s_blocksize = mp->m_sb.sb_blocksize;
  1649		sb->s_blocksize_bits = ffs(sb->s_blocksize) - 1;
  1650		sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
  1651		sb->s_max_links = XFS_MAXLINK;
  1652		sb->s_time_gran = 1;
  1653		set_posix_acl_flag(sb);
  1654	
  1655		/* version 5 superblocks support inode version counters. */
  1656		if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
  1657			sb->s_flags |= SB_I_VERSION;
  1658	
  1659		if (mp->m_flags & XFS_MOUNT_DAX) {
> 1660			bool rtdev_is_dax, datadev_is_dax;
  1661	
  1662			xfs_warn(mp,
  1663			"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
  1664	
  1665			datadev_is_dax = bdev_dax_supported(sb,
  1666					mp->m_ddev_targp->bt_bdev, sb->s_blocksize);
  1667			if (mp->m_rtdev_targp)
  1668				rtdev_is_dax = bdev_dax_supported(sb,
  1669						mp->m_rtdev_targp->bt_bdev,
  1670						sb->s_blocksize);
  1671			if (!rtdev_is_dax && !datadev_is_dax) {
  1672				xfs_alert(mp,
  1673				"DAX unsupported by block device. Turning off DAX.");
  1674				mp->m_flags &= ~XFS_MOUNT_DAX;
  1675			}
  1676			if (xfs_sb_version_hasreflink(&mp->m_sb)) {
  1677				xfs_alert(mp,
  1678			"DAX and reflink cannot be used together!");
  1679				error = -EINVAL;
  1680				goto out_filestream_unmount;
  1681			}
  1682		}
  1683	
  1684		if (mp->m_flags & XFS_MOUNT_DISCARD) {
  1685			struct request_queue *q = bdev_get_queue(sb->s_bdev);
  1686	
  1687			if (!blk_queue_discard(q)) {
  1688				xfs_warn(mp, "mounting with \"discard\" option, but "
  1689						"the device does not support discard");
  1690				mp->m_flags &= ~XFS_MOUNT_DISCARD;
  1691			}
  1692		}
  1693	
  1694		if (xfs_sb_version_hasreflink(&mp->m_sb) && mp->m_sb.sb_rblocks) {
  1695			xfs_alert(mp,
  1696		"reflink not compatible with realtime device!");
  1697			error = -EINVAL;
  1698			goto out_filestream_unmount;
  1699		}
  1700	
  1701		if (xfs_sb_version_hasrmapbt(&mp->m_sb) && mp->m_sb.sb_rblocks) {
  1702			xfs_alert(mp,
  1703		"reverse mapping btree not compatible with realtime device!");
  1704			error = -EINVAL;
  1705			goto out_filestream_unmount;
  1706		}
  1707	
  1708		error = xfs_mountfs(mp);
  1709		if (error)
  1710			goto out_filestream_unmount;
  1711	
  1712		root = igrab(VFS_I(mp->m_rootip));
  1713		if (!root) {
  1714			error = -ENOENT;
  1715			goto out_unmount;
  1716		}
  1717		sb->s_root = d_make_root(root);
  1718		if (!sb->s_root) {
  1719			error = -ENOMEM;
  1720			goto out_unmount;
  1721		}
  1722	
  1723		return 0;
  1724	
  1725	 out_filestream_unmount:
  1726		xfs_filestream_unmount(mp);
  1727	 out_free_sb:
  1728		xfs_freesb(mp);
  1729	 out_free_stats:
  1730		free_percpu(mp->m_stats.xs_stats);
  1731	 out_destroy_counters:
  1732		xfs_destroy_percpu_counters(mp);
  1733	 out_destroy_workqueues:
  1734		xfs_destroy_mount_workqueues(mp);
  1735	 out_close_devices:
  1736		xfs_close_devices(mp);
  1737	 out_free_fsname:
  1738		xfs_free_fsname(mp);
  1739		kfree(mp);
  1740	 out:
  1741		return error;
  1742	
  1743	 out_unmount:
  1744		xfs_filestream_unmount(mp);
  1745		xfs_unmountfs(mp);
  1746		goto out_free_sb;
  1747	}
  1748	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v4 1/3] fs: allow per-device dax status checking for filesystems

[kbuild test robot]

Hi Darrick,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc3 next-20180228]
[cannot apply to dgc-xfs/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dave-Jiang/minimal-DAX-support-for-XFS-realtime-device/20180218-154220
config: i386-randconfig-c0-03010621 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/dax/super.c:86:5: error: redefinition of 'bdev_dax_supported'
    int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
        ^~~~~~~~~~~~~~~~~~
   In file included from drivers/dax/super.c:23:0:
   include/linux/dax.h:62:19: note: previous definition of 'bdev_dax_supported' was here
    static inline int bdev_dax_supported(struct super_block *sb,
                      ^~~~~~~~~~~~~~~~~~

vim +/bdev_dax_supported +86 drivers/dax/super.c

    74	
    75	/**
    76	 * bdev_dax_supported() - Check if the device supports dax for filesystem
    77	 * @sb: The superblock of the device
    78	 * @bdev: block device to check
    79	 * @blocksize: The block size of the device
    80	 *
    81	 * This is a library function for filesystems to check if the block device
    82	 * can be mounted with dax option.
    83	 *
    84	 * Return: negative errno if unsupported, 0 if supported.
    85	 */
  > 86	int bdev_dax_supported(struct super_block *sb, struct block_device *bdev,
    87			       int blocksize)
    88	{
    89		struct dax_device *dax_dev;
    90		pgoff_t pgoff;
    91		int err, id;
    92		void *kaddr;
    93		pfn_t pfn;
    94		long len;
    95	
    96		if (blocksize != PAGE_SIZE) {
    97			pr_debug("VFS (%s): error: unsupported blocksize for dax\n",
    98					sb->s_id);
    99			return -EINVAL;
   100		}
   101	
   102		err = bdev_dax_pgoff(bdev, 0, PAGE_SIZE, &pgoff);
   103		if (err) {
   104			pr_debug("VFS (%s): error: unaligned partition for dax\n",
   105					sb->s_id);
   106			return err;
   107		}
   108	
   109		dax_dev = dax_get_by_host(bdev->bd_disk->disk_name);
   110		if (!dax_dev) {
   111			pr_debug("VFS (%s): error: device does not support dax\n",
   112					sb->s_id);
   113			return -EOPNOTSUPP;
   114		}
   115	
   116		id = dax_read_lock();
   117		len = dax_direct_access(dax_dev, pgoff, 1, &kaddr, &pfn);
   118		dax_read_unlock(id);
   119	
   120		put_dax(dax_dev);
   121	
   122		if (len < 1) {
   123			pr_debug("VFS (%s): error: dax access failed (%ld)\n",
   124					sb->s_id, len);
   125			return len < 0 ? len : -EIO;
   126		}
   127	
   128		if ((IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(pfn))
   129				|| pfn_t_devmap(pfn))
   130			/* pass */;
   131		else {
   132			pr_debug("VFS (%s): error: dax support not enabled\n",
   133					sb->s_id);
   134			return -EOPNOTSUPP;
   135		}
   136	
   137		return 0;
   138	}
   139	EXPORT_SYMBOL_GPL(bdev_dax_supported);
   140	#endif
   141	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm