[PATCH v3 0/5] Add alignment check for DAX mount

[PATCH v3 0/5] Add alignment check for DAX mount

[Toshi Kani]

When a partition is not aligned by 4KB, mount -o dax succeeds,
but any read/write access to the filesystem fails, except for
metadata update.  Add alignment check to ext4, ext2, and xfs.

- Patch 1-2 add bdev_supports_dax() which performs all the checks
  necessary for dax mount.
- Patch 3-5 change fillesystems to call bdev_supports_dax().

v3:
 - Remove boilerplate code from filesytems (Christoph Hellwig)
 - Add a helper function to perform all checks (Dave Chinner)

v2:
 - Use a helper function via ->direct_access for the check.
   (Christoph Hellwig)
 - Call bdev_direct_access() with sector 0 for the check.
   (Boaz Harrosh)

---
Toshi Kani (5):
 1/5 block: Add vfs_msg() interface
 2/5 block: Add bdev_supports_dax() for dax mount checks
 3/5 ext4: Add alignment check for DAX mount
 4/5 ext2: Add alignment check for DAX mount
 5/5 xfs: Add alignment check for DAX mount

---
 fs/block_dev.c         | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ext2/super.c        | 11 ++--------
 fs/ext4/super.c        | 11 ++--------
 fs/xfs/xfs_super.c     | 12 +++++------
 include/linux/blkdev.h | 12 +++++++++++
 5 files changed, 75 insertions(+), 25 deletions(-)

[PATCH v3 1/5] block: Add vfs_msg() interface

[Toshi Kani]

In preparation of moving DAX capability checks to the block layer
from filesystem code, add a VFS message interface that aligns with
filesystem's message format.

For instance, a vfs_msg() message followed by XFS messages in case
of a dax mount error may look like:

  VFS (pmem0p1): error: unaligned partition for dax
  XFS (pmem0p1): DAX unsupported by block device. Turning off DAX.
  XFS (pmem0p1): Mounting V5 Filesystem
   :

vfs_msg() is largely based on ext4_msg().

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Jens Axboe <axboe@fb.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Jan Kara <jack@suse.cz>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Boaz Harrosh <boaz@plexistor.com>
---
 fs/block_dev.c         |   12 ++++++++++++
 include/linux/blkdev.h |   11 +++++++++++
 2 files changed, 23 insertions(+)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 20a2c02..7be17c4 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -50,6 +50,18 @@ struct block_device *I_BDEV(struct inode *inode)
 }
 EXPORT_SYMBOL(I_BDEV);
 
+void __vfs_msg(struct super_block *sb, const char *prefix, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	printk_ratelimited("%sVFS (%s): %pV\n", prefix, sb->s_id, &vaf);
+	va_end(args);
+}
+
 static void bdev_write_inode(struct block_device *bdev)
 {
 	struct inode *inode = bdev->bd_inode;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 669e419..78c48ab 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -767,6 +767,17 @@ static inline void rq_flush_dcache_pages(struct request *rq)
 }
 #endif
 
+#ifdef CONFIG_PRINTK
+#define vfs_msg(sb, level, fmt, ...)				\
+	__vfs_msg(sb, level, fmt, ##__VA_ARGS__)
+#else
+#define vfs_msg(sb, level, fmt, ...)				\
+do {								\
+	no_printk(fmt, ##__VA_ARGS__);				\
+	__vfs_msg(sb, "", " ");					\
+} while (0)
+#endif
+
 extern int blk_register_queue(struct gendisk *disk);
 extern void blk_unregister_queue(struct gendisk *disk);
 extern blk_qc_t generic_make_request(struct bio *bio);

[PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Toshi Kani]

DAX imposes additional requirements to a device.  Add
bdev_supports_dax() which performs all the precondition checks
necessary for filesystem to mount the device with dax option.

Also add a new check to verify if a partition is aligned by 4KB.
When a partition is unaligned, any dax read/write access fails,
except for metadata update.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Jens Axboe <axboe@fb.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Jan Kara <jack@suse.cz>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Boaz Harrosh <boaz@plexistor.com>
---
 fs/block_dev.c         |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |    1 +
 2 files changed, 43 insertions(+)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7be17c4..e51a2c3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -509,6 +509,48 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
 }
 EXPORT_SYMBOL_GPL(bdev_direct_access);
 
+/**
+ * bdev_supports_dax() - Check if the block device supports DAX
+ * @sb: The superblock of the device
+ * @blocksize: The block size of the device
+ *
+ * Return: negative errno if unsupported, 0 if supported.
+ */
+int bdev_supports_dax(struct super_block *sb, int blocksize)
+{
+	struct blk_dax_ctl dax = {
+		.sector = 0,
+		.size = PAGE_SIZE,
+	};
+	int err;
+
+	if (blocksize != PAGE_SIZE) {
+		vfs_msg(sb, KERN_ERR, "error: unsupported blocksize for dax");
+		return -EINVAL;
+	}
+
+	err = bdev_direct_access(sb->s_bdev, &dax);
+	if (err < 0) {
+		switch (err) {
+		case -EOPNOTSUPP:
+			vfs_msg(sb, KERN_ERR,
+				"error: device does not support dax");
+			break;
+		case -EINVAL:
+			vfs_msg(sb, KERN_ERR,
+				"error: unaligned partition for dax");
+			break;
+		default:
+			vfs_msg(sb, KERN_ERR,
+				"error: dax access failed (%d)", err);
+		}
+		return err;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(bdev_supports_dax);
+
 /*
  * pseudo-fs
  */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 78c48ab..6a792aa 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1688,6 +1688,7 @@ extern int bdev_read_page(struct block_device *, sector_t, struct page *);
 extern int bdev_write_page(struct block_device *, sector_t, struct page *,
 						struct writeback_control *);
 extern long bdev_direct_access(struct block_device *, struct blk_dax_ctl *);
+extern int bdev_supports_dax(struct super_block *, int);
 #else /* CONFIG_BLOCK */
 
 struct block_device;

[PATCH v3 3/5] ext4: Add alignment check for DAX mount

[Toshi Kani]

When a partition is not aligned by 4KB, mount -o dax succeeds,
but any read/write access to the filesystem fails, except for
metadata update.

Call bdev_supports_dax() to perform proper precondition checks
which includes this partition alignment check.

Reported-by: Micah Parrish <micah.parrish@hpe.com>
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Jan Kara <jack@suse.cz>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Boaz Harrosh <boaz@plexistor.com>
---
 fs/ext4/super.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 304c712..e7ca71c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3416,16 +3416,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 	if (sbi->s_mount_opt & EXT4_MOUNT_DAX) {
-		if (blocksize != PAGE_SIZE) {
-			ext4_msg(sb, KERN_ERR,
-					"error: unsupported blocksize for dax");
-			goto failed_mount;
-		}
-		if (!sb->s_bdev->bd_disk->fops->direct_access) {
-			ext4_msg(sb, KERN_ERR,
-					"error: device does not support dax");
+		err = bdev_supports_dax(sb, blocksize);
+		if (err)
 			goto failed_mount;
-		}
 	}
 
 	if (ext4_has_feature_encrypt(sb) && es->s_encryption_level) {

[PATCH v3 4/5] ext2: Add alignment check for DAX mount

[Toshi Kani]

When a partition is not aligned by 4KB, mount -o dax succeeds,
but any read/write access to the filesystem fails, except for
metadata update.

Call bdev_supports_dax() to perform proper precondition checks
which includes this partition alignment check.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Boaz Harrosh <boaz@plexistor.com>
---
 fs/ext2/super.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index b78caf2..7d63de4 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -922,16 +922,9 @@ 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) {
-		if (blocksize != PAGE_SIZE) {
-			ext2_msg(sb, KERN_ERR,
-					"error: unsupported blocksize for dax");
+		err = bdev_supports_dax(sb, blocksize);
+		if (err)
 			goto failed_mount;
-		}
-		if (!sb->s_bdev->bd_disk->fops->direct_access) {
-			ext2_msg(sb, KERN_ERR,
-					"error: device does not support dax");
-			goto failed_mount;
-		}
 	}
 
 	/* If the blocksize doesn't match, re-read the thing.. */

[PATCH v3 5/5] xfs: Add alignment check for DAX mount

[Toshi Kani]

When a partition is not aligned by 4KB, mount -o dax succeeds,
but any read/write access to the filesystem fails, except for
metadata update.

Call bdev_supports_dax() to perform proper precondition checks
which includes this partition alignment check.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Boaz Harrosh <boaz@plexistor.com>
---
 fs/xfs/xfs_super.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 187e14b..cc177da 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1558,14 +1558,12 @@ xfs_fs_fill_super(
 
 	if (mp->m_flags & XFS_MOUNT_DAX) {
 		xfs_warn(mp,
-	"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
-		if (sb->s_blocksize != PAGE_SIZE) {
-			xfs_alert(mp,
-		"Filesystem block size invalid for DAX Turning DAX off.");
-			mp->m_flags &= ~XFS_MOUNT_DAX;
-		} else if (!sb->s_bdev->bd_disk->fops->direct_access) {
+		"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
+
+		error = bdev_supports_dax(sb, sb->s_blocksize);
+		if (error) {
 			xfs_alert(mp,
-		"Block device does not support DAX Turning DAX off.");
+			"DAX unsupported by block device. Turning off DAX.");
 			mp->m_flags &= ~XFS_MOUNT_DAX;
 		}
 	}

Re: [PATCH v3 1/5] block: Add vfs_msg() interface

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3 5/5] xfs: Add alignment check for DAX mount

[Christoph Hellwig]

Looks fine for now.  In the long run we'll need to check it for the
RT subvolume as well, or prohibit DAX if there is an active RT
subvolume.

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3 4/5] ext2: Add alignment check for DAX mount

[Christoph Hellwig]

Not really for the patch, but given that we have the right people
on CC:

Do we really want to keep DAX support in ext2 in the long run?  ext2
is missing a lot of the useful features for a modern FS, shouldn't
we direct people to use ext4 (in non-journal mode if needed) if they
want to use DAX?  ext4 will even support the unmodified ext2 fs, so it
shouldn't be a big hurdle, and it would avoid a lot of churn in ext2.

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Dan Williams]

On Thu, May 5, 2016 at 5:29 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
> DAX imposes additional requirements to a device.  Add
> bdev_supports_dax() which performs all the precondition checks
> necessary for filesystem to mount the device with dax option.
>
> Also add a new check to verify if a partition is aligned by 4KB.
> When a partition is unaligned, any dax read/write access fails,
> except for metadata update.
>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Jens Axboe <axboe@fb.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dave Chinner <david@fromorbit.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Boaz Harrosh <boaz@plexistor.com>
> ---
>  fs/block_dev.c         |   42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/blkdev.h |    1 +
>  2 files changed, 43 insertions(+)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7be17c4..e51a2c3 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -509,6 +509,48 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
>  }
>  EXPORT_SYMBOL_GPL(bdev_direct_access);
>
> +/**
> + * bdev_supports_dax() - Check if the block device supports DAX
> + * @sb: The superblock of the device
> + * @blocksize: The block size of the device
> + *
> + * Return: negative errno if unsupported, 0 if supported.
> + */
> +int bdev_supports_dax(struct super_block *sb, int blocksize)
> +{
> +       struct blk_dax_ctl dax = {
> +               .sector = 0,
> +               .size = PAGE_SIZE,
> +       };
> +       int err;
> +
> +       if (blocksize != PAGE_SIZE) {
> +               vfs_msg(sb, KERN_ERR, "error: unsupported blocksize for dax");
> +               return -EINVAL;
> +       }
> +
> +       err = bdev_direct_access(sb->s_bdev, &dax);
> +       if (err < 0) {
> +               switch (err) {
> +               case -EOPNOTSUPP:
> +                       vfs_msg(sb, KERN_ERR,
> +                               "error: device does not support dax");
> +                       break;
> +               case -EINVAL:
> +                       vfs_msg(sb, KERN_ERR,
> +                               "error: unaligned partition for dax");
> +                       break;
> +               default:
> +                       vfs_msg(sb, KERN_ERR,
> +                               "error: dax access failed (%d)", err);
> +               }
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(bdev_supports_dax);

This patch should replace blkdev_dax_capable(), or just reuse that
existing routine, or am I missing something?

Re: [PATCH v3 3/5] ext4: Add alignment check for DAX mount

[Jan Kara]

On Thu 05-05-16 18:29:54, Toshi Kani wrote:
> When a partition is not aligned by 4KB, mount -o dax succeeds,
> but any read/write access to the filesystem fails, except for
> metadata update.
> 
> Call bdev_supports_dax() to perform proper precondition checks
> which includes this partition alignment check.
> 
> Reported-by: Micah Parrish <micah.parrish@hpe.com>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Boaz Harrosh <boaz@plexistor.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 4/5] ext2: Add alignment check for DAX mount

[Jan Kara]

On Sun 08-05-16 01:59:37, Christoph Hellwig wrote:
> Not really for the patch, but given that we have the right people
> on CC:
> 
> Do we really want to keep DAX support in ext2 in the long run?  ext2
> is missing a lot of the useful features for a modern FS, shouldn't
> we direct people to use ext4 (in non-journal mode if needed) if they
> want to use DAX?  ext4 will even support the unmodified ext2 fs, so it
> shouldn't be a big hurdle, and it would avoid a lot of churn in ext2.

I've heard concerns that embedded people use ext2 driver (due to smaller
code size than ext4 - 1.7 MB object for ext2 vs over 7 MB object for ext4
in my build) including old XIP support. DAX has replaced the old XIP code
so removing DAX from ext2 would be a regression for them - either they'd
have to go with significantly larger module or without XIP. So for now I'd
leave DAX in ext2.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 4/5] ext2: Add alignment check for DAX mount

[Jan Kara]

On Thu 05-05-16 18:29:55, Toshi Kani wrote:
> When a partition is not aligned by 4KB, mount -o dax succeeds,
> but any read/write access to the filesystem fails, except for
> metadata update.
> 
> Call bdev_supports_dax() to perform proper precondition checks
> which includes this partition alignment check.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Boaz Harrosh <boaz@plexistor.com>

Looks good to me. Since this patch depends on the first two in your series,
just feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

and merge it along with other patches. I've removed the ext2 patch from
the previous version from my tree.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Toshi Kani]

On Sun, 2016-05-08 at 12:14 -0700, Dan Williams wrote:
> On Thu, May 5, 2016 at 5:29 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
 :
> > +int bdev_supports_dax(struct super_block *sb, int blocksize)
> > +{
> > +       struct blk_dax_ctl dax = {
> > +               .sector = 0,
> > +               .size = PAGE_SIZE,
> > +       };
> > +       int err;
> > +
> > +       if (blocksize != PAGE_SIZE) {
> > +               vfs_msg(sb, KERN_ERR, "error: unsupported blocksize for
> > dax");
> > +               return -EINVAL;
> > +       }
> > +
> > +       err = bdev_direct_access(sb->s_bdev, &dax);
> > +       if (err < 0) {
> > +               switch (err) {
> > +               case -EOPNOTSUPP:
> > +                       vfs_msg(sb, KERN_ERR,
> > +                               "error: device does not support dax");
> > +                       break;
> > +               case -EINVAL:
> > +                       vfs_msg(sb, KERN_ERR,
> > +                               "error: unaligned partition for dax");
> > +                       break;
> > +               default:
> > +                       vfs_msg(sb, KERN_ERR,
> > +                               "error: dax access failed (%d)", err);
> > +               }
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(bdev_supports_dax);
>
> This patch should replace blkdev_dax_capable(), or just reuse that
> existing routine, or am I missing something?

Good question.  bdev_supports_dax() is a helper function tailored for the
filesystem's mount -o dax case.  While blkdev_dax_capable() is similar, it
does not need error messages like "device does not support dax" since it
implicitly enables dax when capable.  So, I think we can keep
blkdev_dax_capable(), but change it to call bdev_direct_access() so that
actual check is performed in a single place.

Thanks,
-Toshi

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Dan Williams]

On Mon, May 9, 2016 at 11:12 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> On Sun, 2016-05-08 at 12:14 -0700, Dan Williams wrote:
>> On Thu, May 5, 2016 at 5:29 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
>  :
>> > +int bdev_supports_dax(struct super_block *sb, int blocksize)
>> > +{
>> > +       struct blk_dax_ctl dax = {
>> > +               .sector = 0,
>> > +               .size = PAGE_SIZE,
>> > +       };
>> > +       int err;
>> > +
>> > +       if (blocksize != PAGE_SIZE) {
>> > +               vfs_msg(sb, KERN_ERR, "error: unsupported blocksize for
>> > dax");
>> > +               return -EINVAL;
>> > +       }
>> > +
>> > +       err = bdev_direct_access(sb->s_bdev, &dax);
>> > +       if (err < 0) {
>> > +               switch (err) {
>> > +               case -EOPNOTSUPP:
>> > +                       vfs_msg(sb, KERN_ERR,
>> > +                               "error: device does not support dax");
>> > +                       break;
>> > +               case -EINVAL:
>> > +                       vfs_msg(sb, KERN_ERR,
>> > +                               "error: unaligned partition for dax");
>> > +                       break;
>> > +               default:
>> > +                       vfs_msg(sb, KERN_ERR,
>> > +                               "error: dax access failed (%d)", err);
>> > +               }
>> > +               return err;
>> > +       }
>> > +
>> > +       return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(bdev_supports_dax);
>>
>> This patch should replace blkdev_dax_capable(), or just reuse that
>> existing routine, or am I missing something?
>
> Good question.  bdev_supports_dax() is a helper function tailored for the
> filesystem's mount -o dax case.  While blkdev_dax_capable() is similar, it
> does not need error messages like "device does not support dax" since it
> implicitly enables dax when capable.  So, I think we can keep
> blkdev_dax_capable(), but change it to call bdev_direct_access() so that
> actual check is performed in a single place.

Sounds good to me.

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Dave Chinner]

On Mon, May 09, 2016 at 11:23:03AM -0700, Dan Williams wrote:
> On Mon, May 9, 2016 at 11:12 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > On Sun, 2016-05-08 at 12:14 -0700, Dan Williams wrote:
> >> On Thu, May 5, 2016 at 5:29 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
> >  :
> >> > +int bdev_supports_dax(struct super_block *sb, int blocksize)
> >> > +{
> >> > +       struct blk_dax_ctl dax = {
> >> > +               .sector = 0,
> >> > +               .size = PAGE_SIZE,
> >> > +       };
> >> > +       int err;
> >> > +
> >> > +       if (blocksize != PAGE_SIZE) {
> >> > +               vfs_msg(sb, KERN_ERR, "error: unsupported blocksize for
> >> > dax");
> >> > +               return -EINVAL;
> >> > +       }
> >> > +
> >> > +       err = bdev_direct_access(sb->s_bdev, &dax);
> >> > +       if (err < 0) {
> >> > +               switch (err) {
> >> > +               case -EOPNOTSUPP:
> >> > +                       vfs_msg(sb, KERN_ERR,
> >> > +                               "error: device does not support dax");
> >> > +                       break;
> >> > +               case -EINVAL:
> >> > +                       vfs_msg(sb, KERN_ERR,
> >> > +                               "error: unaligned partition for dax");
> >> > +                       break;
> >> > +               default:
> >> > +                       vfs_msg(sb, KERN_ERR,
> >> > +                               "error: dax access failed (%d)", err);
> >> > +               }
> >> > +               return err;
> >> > +       }
> >> > +
> >> > +       return 0;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(bdev_supports_dax);
> >>
> >> This patch should replace blkdev_dax_capable(), or just reuse that
> >> existing routine, or am I missing something?
> >
> > Good question.  bdev_supports_dax() is a helper function tailored for the
> > filesystem's mount -o dax case.  While blkdev_dax_capable() is similar, it
> > does not need error messages like "device does not support dax" since it
> > implicitly enables dax when capable.  So, I think we can keep
> > blkdev_dax_capable(), but change it to call bdev_direct_access() so that
> > actual check is performed in a single place.
> 
> Sounds good to me.

Can you name them consistently then? i.e. blkdev_dax_supported() and
blkdev_dax_capable()?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Toshi Kani]

On Tue, 2016-05-10 at 07:19 +1000, Dave Chinner wrote:
> On Mon, May 09, 2016 at 11:23:03AM -0700, Dan Williams wrote:
> > 
> > On Mon, May 9, 2016 at 11:12 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > > 
> > > On Sun, 2016-05-08 at 12:14 -0700, Dan Williams wrote:
> > > > 
> > > > On Thu, May 5, 2016 at 5:29 PM, Toshi Kani <toshi.kani@hpe.com>
> > > > wrote:
 :
> > > > This patch should replace blkdev_dax_capable(), or just reuse that
> > > > existing routine, or am I missing something?
> > >
> > > Good question.  bdev_supports_dax() is a helper function tailored for
> > > the filesystem's mount -o dax case.  While blkdev_dax_capable() is
> > > similar, it does not need error messages like "device does not
> > > support dax" since it implicitly enables dax when capable.  So, I
> > > think we can keep blkdev_dax_capable(), but change it to call
> > > bdev_direct_access() so that actual check is performed in a single
> > > place.
> >
> > Sounds good to me.
>
> Can you name them consistently then? i.e. blkdev_dax_supported() and
> blkdev_dax_capable()?

Sure.  Will do.

Thanks,
-Toshi

Re: [PATCH v3 2/5] block: Add bdev_supports_dax() for dax mount checks

[Toshi Kani]

On Mon, 2016-05-09 at 16:34 -0600, Toshi Kani wrote:
> On Tue, 2016-05-10 at 07:19 +1000, Dave Chinner wrote:
 :
> > > > > 
> > > > > This patch should replace blkdev_dax_capable(), or just reuse
> > > > > that existing routine, or am I missing something?
> > > >
> > > > Good question.  bdev_supports_dax() is a helper function tailored
> > > > for the filesystem's mount -o dax case.  While blkdev_dax_capable()
> > > > is similar, it does not need error messages like "device does not
> > > > support dax" since it implicitly enables dax when capable.  So, I
> > > > think we can keep blkdev_dax_capable(), but change it to call
> > > > bdev_direct_access() so that actual check is performed in a single
> > > > place.
> > >
> > > Sounds good to me.
> >
> > Can you name them consistently then? i.e. blkdev_dax_supported() and
> > blkdev_dax_capable()?
>
> Sure.  Will do.

I will keep the "bdev_" prefix to be consistent with bdev_direct_access(),
i.e. bdev_dax_supported() and bdev_dax_capable().

-Toshi