From: Boaz Harrosh <boaz@plexistor.com>
To: Toshi Kani <toshi.kani@hpe.com>,
dan.j.williams@intel.com, david@fromorbit.com, jack@suse.cz
Cc: linux-fsdevel@vger.kernel.org, adilger.kernel@dilger.ca,
tytso@mit.edu, linux-kernel@vger.kernel.org,
linux-nvdimm@lists.01.org
Subject: Re: [PATCH 1/3] ext4: Add alignment check for DAX mount
Date: Sun, 01 May 2016 12:35:14 +0300 [thread overview]
Message-ID: <5725CDD2.60904@plexistor.com> (raw)
In-Reply-To: <1461962375-3720-2-git-send-email-toshi.kani@hpe.com>
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
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Boaz Harrosh <boaz@plexistor.com>
To: Toshi Kani <toshi.kani@hpe.com>,
dan.j.williams@intel.com, david@fromorbit.com, jack@suse.cz
Cc: tytso@mit.edu, linux-nvdimm@lists.01.org,
linux-kernel@vger.kernel.org, adilger.kernel@dilger.ca,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 1/3] ext4: Add alignment check for DAX mount
Date: Sun, 01 May 2016 12:35:14 +0300 [thread overview]
Message-ID: <5725CDD2.60904@plexistor.com> (raw)
In-Reply-To: <1461962375-3720-2-git-send-email-toshi.kani@hpe.com>
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
>
WARNING: multiple messages have this Message-ID (diff)
From: Boaz Harrosh <boaz@plexistor.com>
To: Toshi Kani <toshi.kani@hpe.com>,
dan.j.williams@intel.com, david@fromorbit.com, jack@suse.cz
Cc: tytso@mit.edu, linux-nvdimm@ml01.01.org,
linux-kernel@vger.kernel.org, adilger.kernel@dilger.ca,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 1/3] ext4: Add alignment check for DAX mount
Date: Sun, 01 May 2016 12:35:14 +0300 [thread overview]
Message-ID: <5725CDD2.60904@plexistor.com> (raw)
In-Reply-To: <1461962375-3720-2-git-send-email-toshi.kani@hpe.com>
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
>
next prev parent reply other threads:[~2016-05-01 9:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-29 20:39 [PATCH 0/3] Add alignment check for DAX mount Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` [PATCH 1/3] ext4: " Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-05-01 9:35 ` Boaz Harrosh [this message]
2016-05-01 9:35 ` Boaz Harrosh
2016-05-01 9:35 ` Boaz Harrosh
2016-05-01 17:31 ` Christoph Hellwig
2016-05-02 9:49 ` Boaz Harrosh
2016-05-02 15:23 ` Toshi Kani
2016-04-29 20:39 ` [PATCH 2/3] ext2: " Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` [PATCH 3/3] xfs: " Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 20:39 ` Toshi Kani
2016-04-29 23:12 ` [PATCH 0/3] " Ross Zwisler
2016-04-29 23:12 ` Ross Zwisler
2016-04-29 23:12 ` Ross Zwisler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5725CDD2.60904@plexistor.com \
--to=boaz@plexistor.com \
--cc=adilger.kernel@dilger.ca \
--cc=dan.j.williams@intel.com \
--cc=david@fromorbit.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=toshi.kani@hpe.com \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.