| Message ID | 1462494596-20938-3-git-send-email-toshi.kani@hpe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
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 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? -- 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 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 -- 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, 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. -- 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, 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.
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 -- 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-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 -- 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/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;
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(+) -- 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