Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1391415 > unrolled thread
| Started by | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| First post | 2016-04-29 22:50 +0200 |
| Last post | 2016-04-30 01:20 +0200 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] Add alignment check for DAX mount Toshi Kani <toshi.kani@hpe.com> - 2016-04-29 22:50 +0200
[PATCH 1/3] ext4: Add alignment check for DAX mount Toshi Kani <toshi.kani@hpe.com> - 2016-04-29 22:50 +0200
Re: [PATCH 1/3] ext4: Add alignment check for DAX mount Boaz Harrosh <boaz@plexistor.com> - 2016-05-01 11:40 +0200
Re: [PATCH 1/3] ext4: Add alignment check for DAX mount Christoph Hellwig <hch@infradead.org> - 2016-05-01 19:40 +0200
Re: [PATCH 1/3] ext4: Add alignment check for DAX mount Boaz Harrosh <boaz@plexistor.com> - 2016-05-02 11:50 +0200
Re: [PATCH 1/3] ext4: Add alignment check for DAX mount Toshi Kani <toshi.kani@hpe.com> - 2016-05-02 17:40 +0200
[PATCH 3/3] xfs: Add alignment check for DAX mount Toshi Kani <toshi.kani@hpe.com> - 2016-04-29 22:50 +0200
Re: [PATCH 0/3] Add alignment check for DAX mount Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-04-30 01:20 +0200
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2016-04-29 22:50 +0200 |
| Subject | [PATCH 0/3] Add alignment check for DAX mount |
| Message-ID | <rtnwl-3Q2-3@gated-at.bofh.it> |
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. --- 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 | 6 ++++++ fs/ext4/super.c | 6 ++++++ fs/xfs/xfs_super.c | 6 ++++++ 3 files changed, 18 insertions(+)
[toc] | [next] | [standalone]
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2016-04-29 22:50 +0200 |
| Subject | [PATCH 1/3] ext4: Add alignment check for DAX mount |
| Message-ID | <rtnwm-3Q2-21@gated-at.bofh.it> |
| In reply to | #1391415 |
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_fill_super() 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>
---
fs/ext4/super.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 304c712..90a8670 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3421,6 +3421,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
"error: unsupported blocksize for dax");
goto failed_mount;
}
+ if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE / 512) ||
+ sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE / 512)) {
+ ext4_msg(sb, KERN_ERR,
+ "error: unaligned partition 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");
[toc] | [prev] | [next] | [standalone]
| From | Boaz Harrosh <boaz@plexistor.com> |
|---|---|
| Date | 2016-05-01 11:40 +0200 |
| Subject | Re: [PATCH 1/3] ext4: Add alignment check for DAX mount |
| Message-ID | <rtW14-7j9-11@gated-at.bofh.it> |
| In reply to | #1391416 |
On 04/29/2016 11:39 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_fill_super() 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>
> ---
> fs/ext4/super.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 304c712..90a8670 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3421,6 +3421,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> "error: unsupported blocksize for dax");
> goto failed_mount;
> }
> + if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE / 512) ||
> + sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE / 512)) {
Can you please not do this like this? For me is a layering violation only
the device should know what are its limits.
I would prefer if you just try to bdev_direct_access() the 0 sector and if
it fails then fail here. This way you let the device decide what it needs
and if/how to support unaligned partitions.
(For example it could by shrinking its size)
Thanks
Boaz
> + ext4_msg(sb, KERN_ERR,
> + "error: unaligned partition 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");
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
>
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2016-05-01 19:40 +0200 |
| Subject | Re: [PATCH 1/3] ext4: Add alignment check for DAX mount |
| Message-ID | <ru3vz-4Re-13@gated-at.bofh.it> |
| In reply to | #1391416 |
On Fri, Apr 29, 2016 at 02:39:33PM -0600, Toshi Kani wrote:
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 304c712..90a8670 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3421,6 +3421,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> "error: unsupported blocksize for dax");
> goto failed_mount;
> }
> + if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE / 512) ||
> + sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE / 512)) {
> + ext4_msg(sb, KERN_ERR,
> + "error: unaligned partition 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");
Factor your new checks and the ->direct_access into a new helper. It
should take the block device as file systems might have multiple
underlying devices.
[toc] | [prev] | [next] | [standalone]
| From | Boaz Harrosh <boaz@plexistor.com> |
|---|---|
| Date | 2016-05-02 11:50 +0200 |
| Subject | Re: [PATCH 1/3] ext4: Add alignment check for DAX mount |
| Message-ID | <ruiEi-Zl-15@gated-at.bofh.it> |
| In reply to | #1391855 |
On 05/01/2016 08:31 PM, Christoph Hellwig wrote:
> On Fri, Apr 29, 2016 at 02:39:33PM -0600, Toshi Kani wrote:
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 304c712..90a8670 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -3421,6 +3421,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>> "error: unsupported blocksize for dax");
>> goto failed_mount;
>> }
>> + if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE / 512) ||
>> + sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE / 512)) {
>> + ext4_msg(sb, KERN_ERR,
>> + "error: unaligned partition 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");
>
> Factor your new checks and the ->direct_access into a new helper. It
> should take the block device as file systems might have multiple
> underlying devices.
This already exists as part of bdev_direct_access()
All the code needs to do is call bdev_direct_access() on sector ZERO and
see if it is successful. If the alignment is broken it will fail.
Infact the FS does not even need the second if (!...bd_disk->fops->direct_access)
check either because it is also done inside bdev_direct_access(). The only check
needed is calling bdev_direct_access() and the generic layer does all the
checks needed already.
Cheers
Boaz
[toc] | [prev] | [next] | [standalone]
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2016-05-02 17:40 +0200 |
| Subject | Re: [PATCH 1/3] ext4: Add alignment check for DAX mount |
| Message-ID | <ruo70-61q-9@gated-at.bofh.it> |
| In reply to | #1392098 |
On Mon, 2016-05-02 at 12:49 +0300, Boaz Harrosh wrote:
> On 05/01/2016 08:31 PM, Christoph Hellwig wrote:
> >
> > On Fri, Apr 29, 2016 at 02:39:33PM -0600, Toshi Kani wrote:
> > >
> > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > > index 304c712..90a8670 100644
> > > --- a/fs/ext4/super.c
> > > +++ b/fs/ext4/super.c
> > > @@ -3421,6 +3421,12 @@ static int ext4_fill_super(struct super_block
> > > *sb, void *data, int silent)
> > > "error: unsupported
> > > blocksize for dax");
> > > goto failed_mount;
> > > }
> > > + if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE /
> > > 512) ||
> > > + sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE /
> > > 512)) {
> > > + ext4_msg(sb, KERN_ERR,
> > > + "error: unaligned partition
> > > 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");
> >
> > Factor your new checks and the ->direct_access into a new helper. It
> > should take the block device as file systems might have multiple
> > underlying devices.
>
> This already exists as part of bdev_direct_access()
>
> All the code needs to do is call bdev_direct_access() on sector ZERO and
> see if it is successful. If the alignment is broken it will fail.
>
> Infact the FS does not even need the second if (!...bd_disk->fops-
> >direct_access) check either because it is also done inside
> bdev_direct_access(). The only check needed is calling
> bdev_direct_access() and the generic layer does all the
> checks needed already.
Sounds good. I will call bdev_direct_access() for this check. I assume we
do not need to check the alignment of 'bd_part->nr_sects' since the rest of
unaligned sectors will be ignored with block size 4KB.
Thanks!
-Toshi
[toc] | [prev] | [next] | [standalone]
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2016-04-29 22:50 +0200 |
| Subject | [PATCH 3/3] xfs: Add alignment check for DAX mount |
| Message-ID | <rtnwm-3Q2-23@gated-at.bofh.it> |
| In reply to | #1391415 |
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 xfs_fs_fill_super() when -o dax is
specified.
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Dave Chinner <david@fromorbit.com> (supporter:XFS FILESYSTEM)
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
---
fs/xfs/xfs_super.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 187e14b..a17aebc 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1563,6 +1563,11 @@ xfs_fs_fill_super(
xfs_alert(mp,
"Filesystem block size invalid for DAX Turning DAX off.");
mp->m_flags &= ~XFS_MOUNT_DAX;
+ } else if (sb->s_bdev->bd_part->start_sect % (PAGE_SIZE / 512)
+ || sb->s_bdev->bd_part->nr_sects % (PAGE_SIZE / 512)) {
+ xfs_alert(mp,
+ "Partition alignment 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.");
[toc] | [prev] | [next] | [standalone]
| From | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| Date | 2016-04-30 01:20 +0200 |
| Message-ID | <rtpRw-69r-9@gated-at.bofh.it> |
| In reply to | #1391415 |
On Fri, Apr 29, 2016 at 02:39:32PM -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. > > Add alignment check to ext4, ext2, and xfs. > > --- > 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 | 6 ++++++ > fs/ext4/super.c | 6 ++++++ > fs/xfs/xfs_super.c | 6 ++++++ > 3 files changed, 18 insertions(+) For the series: Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web