| Message ID | 1461962375-3720-2-git-send-email-toshi.kani@hpe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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");
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(+) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html