Linux-NVDIMM Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add alignment check for DAX mount
@ 2016-05-02 18:42 Toshi Kani
  2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Toshi Kani @ 2016-05-02 18:42 UTC (permalink / raw)
  To: dan.j.williams, jack, david
  Cc: tytso, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel

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.

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 (3):
 1/3 ext4: Add alignment check for DAX mount
 2/3 ext2: Add alignment check for DAX mount
 3/3 xfs: Add alignment check for DAX mount

---
 fs/ext2/super.c    | 21 +++++++++++++++++++--
 fs/ext4/super.c    | 20 ++++++++++++++++++--
 fs/xfs/xfs_super.c | 23 +++++++++++++++++++----
 3 files changed, 56 insertions(+), 8 deletions(-)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH v2 1/3] ext4: Add alignment check for DAX mount
  2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
@ 2016-05-02 18:42 ` Toshi Kani
  2016-05-03  7:58   ` Jan Kara
  2016-05-02 18:42 ` [PATCH v2 2/3] ext2: " Toshi Kani
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Toshi Kani @ 2016-05-02 18:42 UTC (permalink / raw)
  To: dan.j.williams, jack, david
  Cc: tytso, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel

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_direct_access to check the alignment when -o dax is
specified.

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 |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 304c712..51ac78e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3416,14 +3416,30 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 	if (sbi->s_mount_opt & EXT4_MOUNT_DAX) {
+		struct blk_dax_ctl dax = {
+			.sector = 0,
+			.size = PAGE_SIZE,
+		};
 		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,
+		err = bdev_direct_access(sb->s_bdev, &dax);
+		if (err < 0) {
+			switch (err) {
+			case -EOPNOTSUPP:
+				ext4_msg(sb, KERN_ERR,
 					"error: device does not support dax");
+				break;
+			case -EINVAL:
+				ext4_msg(sb, KERN_ERR,
+					"error: unaligned partition for dax");
+				break;
+			default:
+				ext4_msg(sb, KERN_ERR,
+					"error: dax access failed (%d)", err);
+			}
 			goto failed_mount;
 		}
 	}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH v2 2/3] ext2: Add alignment check for DAX mount
  2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
  2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
@ 2016-05-02 18:42 ` Toshi Kani
  2016-05-03  8:01   ` Jan Kara
  2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
  2016-05-02 19:20 ` [PATCH v2 0/3] " Boaz Harrosh
  3 siblings, 1 reply; 12+ messages in thread
From: Toshi Kani @ 2016-05-02 18:42 UTC (permalink / raw)
  To: dan.j.williams, jack, david
  Cc: tytso, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel

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_direct_access to check the alignment when -o dax is
specified.

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 |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index b78caf2..e636b56 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -922,14 +922,31 @@ 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) {
+		struct blk_dax_ctl dax = {
+			.sector = 0,
+			.size = PAGE_SIZE,
+		};
 		if (blocksize != PAGE_SIZE) {
 			ext2_msg(sb, KERN_ERR,
 					"error: unsupported blocksize for dax");
 			goto failed_mount;
 		}
-		if (!sb->s_bdev->bd_disk->fops->direct_access) {
-			ext2_msg(sb, KERN_ERR,
+		err = bdev_direct_access(sb->s_bdev, &dax);
+		if (err < 0) {
+			switch (err) {
+			case -EOPNOTSUPP:
+				ext2_msg(sb, KERN_ERR,
 					"error: device does not support dax");
+				break;
+			case -EINVAL:
+				ext2_msg(sb, KERN_ERR,
+					"error: unaligned partition for dax");
+				break;
+			default:
+				ext2_msg(sb, KERN_ERR,
+					"error: dax access failed (%d)", err);
+				break;
+			}
 			goto failed_mount;
 		}
 	}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
  2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
  2016-05-02 18:42 ` [PATCH v2 2/3] ext2: " Toshi Kani
@ 2016-05-02 18:42 ` Toshi Kani
  2016-05-02 19:50   ` Ross Zwisler
  2016-05-04 23:18   ` Dave Chinner
  2016-05-02 19:20 ` [PATCH v2 0/3] " Boaz Harrosh
  3 siblings, 2 replies; 12+ messages in thread
From: Toshi Kani @ 2016-05-02 18:42 UTC (permalink / raw)
  To: dan.j.williams, jack, david
  Cc: tytso, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel

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_direct_access to check the alignment when -o dax is
specified.

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 |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 187e14b..b7ee323 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1557,15 +1557,30 @@ xfs_fs_fill_super(
 		sb->s_flags |= MS_I_VERSION;
 
 	if (mp->m_flags & XFS_MOUNT_DAX) {
+		struct blk_dax_ctl dax = {
+			.sector = 0,
+			.size = PAGE_SIZE,
+		};
 		xfs_warn(mp,
-	"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
+		"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) {
-			xfs_alert(mp,
-		"Block device does not support DAX Turning DAX off.");
+		} else if ((error = bdev_direct_access(sb->s_bdev, &dax)) < 0) {
+			switch (error) {
+			case -EOPNOTSUPP:
+				xfs_alert(mp,
+			"Block device does not support DAX Turning DAX off.");
+				break;
+			case -EINVAL:
+				xfs_alert(mp,
+			"Partition alignment invalid for DAX Turning DAX off.");
+				break;
+			default:
+				xfs_alert(mp,
+			"DAX access failed (%d) DAX Turning DAX off.", error);
+			}
 			mp->m_flags &= ~XFS_MOUNT_DAX;
 		}
 	}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH v2 0/3] Add alignment check for DAX mount
  2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
                   ` (2 preceding siblings ...)
  2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
@ 2016-05-02 19:20 ` Boaz Harrosh
  3 siblings, 0 replies; 12+ messages in thread
From: Boaz Harrosh @ 2016-05-02 19:20 UTC (permalink / raw)
  To: Toshi Kani, dan.j.williams, jack, david
  Cc: tytso, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel

On 05/02/2016 09:42 PM, 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.
> 
> Add alignment check to ext4, ext2, and xfs.
> 
> 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 (3):
>  1/3 ext4: Add alignment check for DAX mount
>  2/3 ext2: Add alignment check for DAX mount
>  3/3 xfs: Add alignment check for DAX mount
> 

All patches look very good to me, and keep the
internals internal. Thanks Toshi

Review-by: Boaz Harrosh <boaz@plexistor.com>

> ---
>  fs/ext2/super.c    | 21 +++++++++++++++++++--
>  fs/ext4/super.c    | 20 ++++++++++++++++++--
>  fs/xfs/xfs_super.c | 23 +++++++++++++++++++----
>  3 files changed, 56 insertions(+), 8 deletions(-)
> 

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

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

* Re: [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
@ 2016-05-02 19:50   ` Ross Zwisler
  2016-05-02 19:54     ` Toshi Kani
  2016-05-04 23:18   ` Dave Chinner
  1 sibling, 1 reply; 12+ messages in thread
From: Ross Zwisler @ 2016-05-02 19:50 UTC (permalink / raw)
  To: Toshi Kani
  Cc: jack, linux-nvdimm, david, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Mon, May 02, 2016 at 12:42:58PM -0600, 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_direct_access to check the alignment when -o dax is
> specified.
> 
> 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 |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 187e14b..b7ee323 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1557,15 +1557,30 @@ xfs_fs_fill_super(
>  		sb->s_flags |= MS_I_VERSION;
>  
>  	if (mp->m_flags & XFS_MOUNT_DAX) {
> +		struct blk_dax_ctl dax = {
> +			.sector = 0,
> +			.size = PAGE_SIZE,
> +		};
>  		xfs_warn(mp,
> -	"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> +		"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) {
> -			xfs_alert(mp,
> -		"Block device does not support DAX Turning DAX off.");
> +		} else if ((error = bdev_direct_access(sb->s_bdev, &dax)) < 0) {
> +			switch (error) {
> +			case -EOPNOTSUPP:
> +				xfs_alert(mp,
> +			"Block device does not support DAX Turning DAX off.");

Since you're already in here editing all the strings, can you add a period to
make it more readable?   Applies to all strings.

> +			"Block device does not support DAX. Turning DAX off.");
							  ^

> +				break;
> +			case -EINVAL:
> +				xfs_alert(mp,
> +			"Partition alignment invalid for DAX Turning DAX off.");
> +				break;
> +			default:
> +				xfs_alert(mp,
> +			"DAX access failed (%d) DAX Turning DAX off.", error);

I DAX think you might DAX have too many DAXes in here. :)

> +			}
>  			mp->m_flags &= ~XFS_MOUNT_DAX;
>  		}
>  	}

Other than the nit-picking about the strings, this seems fine.

You can add this for the series:
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-02 19:50   ` Ross Zwisler
@ 2016-05-02 19:54     ` Toshi Kani
  0 siblings, 0 replies; 12+ messages in thread
From: Toshi Kani @ 2016-05-02 19:54 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: jack, linux-nvdimm, david, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Mon, 2016-05-02 at 13:50 -0600, Ross Zwisler wrote:
> On Mon, May 02, 2016 at 12:42:58PM -0600, Toshi Kani wrote:
 :
> >  		xfs_warn(mp,
> > -	"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> > +		"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)
> > {
> > -			xfs_alert(mp,
> > -		"Block device does not support DAX Turning DAX off.");
> > +		} else if ((error = bdev_direct_access(sb->s_bdev,
> > &dax)) < 0) {
> > +			switch (error) {
> > +			case -EOPNOTSUPP:
> > +				xfs_alert(mp,
> > +			"Block device does not support DAX Turning DAX
> > off.");
>
> Since you're already in here editing all the strings, can you add a
> period to make it more readable?   Applies to all strings.

Right. Will do.

> > 
> > +			"Block device does not support DAX. Turning
> > DAX off.");
> 							  ^
> 
> > 
> > +				break;
> > +			case -EINVAL:
> > +				xfs_alert(mp,
> > +			"Partition alignment invalid for DAX Turning
> > DAX off.");
> > +				break;
> > +			default:
> > +				xfs_alert(mp,
> > +			"DAX access failed (%d) DAX Turning DAX off.",
> > error);
>
> I DAX think you might DAX have too many DAXes in here. :)

Oops! :)

> > 
> > +			}
> >  			mp->m_flags &= ~XFS_MOUNT_DAX;
> >  		}
> >  	}
>
> Other than the nit-picking about the strings, this seems fine.
> 
> You can add this for the series:
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

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

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

* Re: [PATCH v2 1/3] ext4: Add alignment check for DAX mount
  2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
@ 2016-05-03  7:58   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2016-05-03  7:58 UTC (permalink / raw)
  To: Toshi Kani
  Cc: jack, linux-nvdimm, david, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Mon 02-05-16 12:42:56, 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_direct_access to check the alignment when -o dax is
> specified.
> 
> 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 to me. You can add:

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

								Honza
> ---
>  fs/ext4/super.c |   20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 304c712..51ac78e 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3416,14 +3416,30 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  	}
>  
>  	if (sbi->s_mount_opt & EXT4_MOUNT_DAX) {
> +		struct blk_dax_ctl dax = {
> +			.sector = 0,
> +			.size = PAGE_SIZE,
> +		};
>  		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,
> +		err = bdev_direct_access(sb->s_bdev, &dax);
> +		if (err < 0) {
> +			switch (err) {
> +			case -EOPNOTSUPP:
> +				ext4_msg(sb, KERN_ERR,
>  					"error: device does not support dax");
> +				break;
> +			case -EINVAL:
> +				ext4_msg(sb, KERN_ERR,
> +					"error: unaligned partition for dax");
> +				break;
> +			default:
> +				ext4_msg(sb, KERN_ERR,
> +					"error: dax access failed (%d)", err);
> +			}
>  			goto failed_mount;
>  		}
>  	}
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH v2 2/3] ext2: Add alignment check for DAX mount
  2016-05-02 18:42 ` [PATCH v2 2/3] ext2: " Toshi Kani
@ 2016-05-03  8:01   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2016-05-03  8:01 UTC (permalink / raw)
  To: Toshi Kani
  Cc: jack, linux-nvdimm, david, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Mon 02-05-16 12:42:57, 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_direct_access to check the alignment when -o dax is
> specified.
> 
> 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>

The patch looks good. I've added it to my tree and will push it to Linus in
the coming merge window.

								Honza

> ---
>  fs/ext2/super.c |   21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index b78caf2..e636b56 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -922,14 +922,31 @@ 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) {
> +		struct blk_dax_ctl dax = {
> +			.sector = 0,
> +			.size = PAGE_SIZE,
> +		};
>  		if (blocksize != PAGE_SIZE) {
>  			ext2_msg(sb, KERN_ERR,
>  					"error: unsupported blocksize for dax");
>  			goto failed_mount;
>  		}
> -		if (!sb->s_bdev->bd_disk->fops->direct_access) {
> -			ext2_msg(sb, KERN_ERR,
> +		err = bdev_direct_access(sb->s_bdev, &dax);
> +		if (err < 0) {
> +			switch (err) {
> +			case -EOPNOTSUPP:
> +				ext2_msg(sb, KERN_ERR,
>  					"error: device does not support dax");
> +				break;
> +			case -EINVAL:
> +				ext2_msg(sb, KERN_ERR,
> +					"error: unaligned partition for dax");
> +				break;
> +			default:
> +				ext2_msg(sb, KERN_ERR,
> +					"error: dax access failed (%d)", err);
> +				break;
> +			}
>  			goto failed_mount;
>  		}
>  	}
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
  2016-05-02 19:50   ` Ross Zwisler
@ 2016-05-04 23:18   ` Dave Chinner
  2016-05-04 23:41     ` Toshi Kani
  1 sibling, 1 reply; 12+ messages in thread
From: Dave Chinner @ 2016-05-04 23:18 UTC (permalink / raw)
  To: Toshi Kani
  Cc: jack, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Mon, May 02, 2016 at 12:42:58PM -0600, 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_direct_access to check the alignment when -o dax is
> specified.
> 
> 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 |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 187e14b..b7ee323 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1557,15 +1557,30 @@ xfs_fs_fill_super(
>  		sb->s_flags |= MS_I_VERSION;
>  
>  	if (mp->m_flags & XFS_MOUNT_DAX) {
> +		struct blk_dax_ctl dax = {
> +			.sector = 0,
> +			.size = PAGE_SIZE,
> +		};
>  		xfs_warn(mp,
> -	"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> +		"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) {
> -			xfs_alert(mp,
> -		"Block device does not support DAX Turning DAX off.");
> +		} else if ((error = bdev_direct_access(sb->s_bdev, &dax)) < 0) {
> +			switch (error) {
> +			case -EOPNOTSUPP:
> +				xfs_alert(mp,
> +			"Block device does not support DAX Turning DAX off.");
> +				break;
> +			case -EINVAL:
> +				xfs_alert(mp,
> +			"Partition alignment invalid for DAX Turning DAX off.");
> +				break;
> +			default:
> +				xfs_alert(mp,
> +			"DAX access failed (%d) DAX Turning DAX off.", error);
> +			}

Please write a helper along the lines of:

	error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);

and encapsulate all this, including the specific error messages in the
helper (i.e. "Block device %s does not support DAX."). Then the rest
of the filesystem code looks something like this:

	if (mp->m_flags & XFS_MOUNT_DAX) {
		error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);
		if (error) {
			xfs_alert(mp,
		"DAX unsupported by block device. Turning off DAX.");
			mp->m_flags &= ~XFS_MOUNT_DAX;
		}
	}

And each filesystem can choose to do what it wants with the error
without having to care exactly why DAX is not supported.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-04 23:18   ` Dave Chinner
@ 2016-05-04 23:41     ` Toshi Kani
  2016-05-05  8:03       ` Jan Kara
  0 siblings, 1 reply; 12+ messages in thread
From: Toshi Kani @ 2016-05-04 23:41 UTC (permalink / raw)
  To: Dave Chinner
  Cc: jack, linux-nvdimm, linux-kernel, micah.parrish, hch,
	adilger.kernel, linux-fsdevel, tytso

On Thu, 2016-05-05 at 09:18 +1000, Dave Chinner wrote:
> On Mon, May 02, 2016 at 12:42:58PM -0600, Toshi Kani wrote:
> > 
:
> Please write a helper along the lines of:
> 
> 	error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);
> 
> and encapsulate all this, including the specific error messages in the
> helper (i.e. "Block device %s does not support DAX."). Then the rest
> of the filesystem code looks something like this:
> 
> 	if (mp->m_flags & XFS_MOUNT_DAX) {
> 		error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);
> 		if (error) {
> 			xfs_alert(mp,
> 		"DAX unsupported by block device. Turning off DAX.");
> 			mp->m_flags &= ~XFS_MOUNT_DAX;
> 		}
> 	}
> 
> And each filesystem can choose to do what it wants with the error
> without having to care exactly why DAX is not supported.

Yes, I had this change in mind and was wondering if you are OK with it
since I am incline to keep the ext2/4 message style as majority rule. :)
https://lkml.org/lkml/2016/5/3/543

Assuming that's OK, I will make this change.

Thanks!
-Toshi

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

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

* Re: [PATCH v2 3/3] xfs: Add alignment check for DAX mount
  2016-05-04 23:41     ` Toshi Kani
@ 2016-05-05  8:03       ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2016-05-05  8:03 UTC (permalink / raw)
  To: Toshi Kani
  Cc: jack, linux-nvdimm, Dave Chinner, linux-kernel, micah.parrish,
	hch, adilger.kernel, linux-fsdevel, tytso

On Wed 04-05-16 17:41:26, Toshi Kani wrote:
> On Thu, 2016-05-05 at 09:18 +1000, Dave Chinner wrote:
> > On Mon, May 02, 2016 at 12:42:58PM -0600, Toshi Kani wrote:
> > > 
> :
> > Please write a helper along the lines of:
> > 
> > 	error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);
> > 
> > and encapsulate all this, including the specific error messages in the
> > helper (i.e. "Block device %s does not support DAX."). Then the rest
> > of the filesystem code looks something like this:
> > 
> > 	if (mp->m_flags & XFS_MOUNT_DAX) {
> > 		error = blkdev_supports_dax(sb->s_bdev, sb->s_blocksize);
> > 		if (error) {
> > 			xfs_alert(mp,
> > 		"DAX unsupported by block device. Turning off DAX.");
> > 			mp->m_flags &= ~XFS_MOUNT_DAX;
> > 		}
> > 	}
> > 
> > And each filesystem can choose to do what it wants with the error
> > without having to care exactly why DAX is not supported.
> 
> Yes, I had this change in mind and was wondering if you are OK with it
> since I am incline to keep the ext2/4 message style as majority rule. :)
> https://lkml.org/lkml/2016/5/3/543
> 
> Assuming that's OK, I will make this change.

Yeah, just send me ext2 changes on top of your v2 and I can pull it to my
tree.


								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

end of thread, other threads:[~2016-05-05  8:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
2016-05-03  7:58   ` Jan Kara
2016-05-02 18:42 ` [PATCH v2 2/3] ext2: " Toshi Kani
2016-05-03  8:01   ` Jan Kara
2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
2016-05-02 19:50   ` Ross Zwisler
2016-05-02 19:54     ` Toshi Kani
2016-05-04 23:18   ` Dave Chinner
2016-05-04 23:41     ` Toshi Kani
2016-05-05  8:03       ` Jan Kara
2016-05-02 19:20 ` [PATCH v2 0/3] " Boaz Harrosh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox