Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1398313 > unrolled thread

[PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency

Started byToshi Kani <toshi.kani@hpe.com>
First post2016-05-10 18:40 +0200
Last post2016-05-11 17:30 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency Toshi Kani <toshi.kani@hpe.com> - 2016-05-10 18:40 +0200
    Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency Dan Williams <dan.j.williams@intel.com> - 2016-05-10 21:50 +0200
      Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for  consistency Toshi Kani <toshi.kani@hpe.com> - 2016-05-10 23:50 +0200
    Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency Jan Kara <jack@suse.cz> - 2016-05-11 10:10 +0200
      Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for  consistency Toshi Kani <toshi.kani@hpe.com> - 2016-05-11 16:40 +0200
        Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency Jan Kara <jack@suse.cz> - 2016-05-11 17:30 +0200

#1398313 — [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency

FromToshi Kani <toshi.kani@hpe.com>
Date2016-05-10 18:40 +0200
Subject[PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency
Message-ID<rxiRv-6cC-65@gated-at.bofh.it>
blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
to remain as a separate interface for checking dax capability of
a raw block device.

Rename and relocate blkdev_dax_capable() to keep them maintained
consistently, and call bdev_direct_access() for the dax capability
check.

There is no change in the behavior.

Link: https://lkml.org/lkml/2016/5/9/950
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Jens Axboe <axboe@fb.com>
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>
---
 block/ioctl.c          |   30 ------------------------------
 fs/block_dev.c         |   39 +++++++++++++++++++++++++++++++++++++--
 include/linux/blkdev.h |    1 +
 include/linux/fs.h     |    8 --------
 4 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 4ff1f92..7eeda07 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -4,7 +4,6 @@
 #include <linux/gfp.h>
 #include <linux/blkpg.h>
 #include <linux/hdreg.h>
-#include <linux/badblocks.h>
 #include <linux/backing-dev.h>
 #include <linux/fs.h>
 #include <linux/blktrace_api.h>
@@ -407,35 +406,6 @@ static inline int is_unrecognized_ioctl(int ret)
 		ret == -ENOIOCTLCMD;
 }
 
-#ifdef CONFIG_FS_DAX
-bool blkdev_dax_capable(struct block_device *bdev)
-{
-	struct gendisk *disk = bdev->bd_disk;
-
-	if (!disk->fops->direct_access)
-		return false;
-
-	/*
-	 * If the partition is not aligned on a page boundary, we can't
-	 * do dax I/O to it.
-	 */
-	if ((bdev->bd_part->start_sect % (PAGE_SIZE / 512))
-			|| (bdev->bd_part->nr_sects % (PAGE_SIZE / 512)))
-		return false;
-
-	/*
-	 * If the device has known bad blocks, force all I/O through the
-	 * driver / page cache.
-	 *
-	 * TODO: support finer grained dax error handling
-	 */
-	if (disk->bb && disk->bb->count)
-		return false;
-
-	return true;
-}
-#endif
-
 static int blkdev_flushbuf(struct block_device *bdev, fmode_t mode,
 		unsigned cmd, unsigned long arg)
 {
diff --git a/fs/block_dev.c b/fs/block_dev.c
index db55638..5d79093 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -29,6 +29,7 @@
 #include <linux/log2.h>
 #include <linux/cleancache.h>
 #include <linux/dax.h>
+#include <linux/badblocks.h>
 #include <asm/uaccess.h>
 #include "internal.h"
 
@@ -554,6 +555,40 @@ int bdev_dax_supported(struct super_block *sb, int blocksize)
 }
 EXPORT_SYMBOL_GPL(bdev_dax_supported);
 
+/**
+ * bdev_dax_capable() - Return if the raw device is capable for dax
+ * @bdev: The device for raw block device access
+ */
+bool bdev_dax_capable(struct block_device *bdev)
+{
+	struct gendisk *disk = bdev->bd_disk;
+	struct blk_dax_ctl dax = {
+		.size = PAGE_SIZE,
+	};
+
+	if (!IS_ENABLED(CONFIG_FS_DAX))
+		return false;
+
+	dax.sector = 0;
+	if (bdev_direct_access(bdev, &dax) < 0)
+		return false;
+
+	dax.sector = bdev->bd_part->nr_sects - (PAGE_SIZE / 512);
+	if (bdev_direct_access(bdev, &dax) < 0)
+		return false;
+
+	/*
+	 * If the device has known bad blocks, force all I/O through the
+	 * driver / page cache.
+	 *
+	 * TODO: support finer grained dax error handling
+	 */
+	if (disk->bb && disk->bb->count)
+		return false;
+
+	return true;
+}
+
 /*
  * pseudo-fs
  */
@@ -1295,7 +1330,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 
 			if (!ret) {
 				bd_set_size(bdev,(loff_t)get_capacity(disk)<<9);
-				if (!blkdev_dax_capable(bdev))
+				if (!bdev_dax_capable(bdev))
 					bdev->bd_inode->i_flags &= ~S_DAX;
 			}
 
@@ -1332,7 +1367,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 				goto out_clear;
 			}
 			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
-			if (!blkdev_dax_capable(bdev))
+			if (!bdev_dax_capable(bdev))
 				bdev->bd_inode->i_flags &= ~S_DAX;
 		}
 	} else {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 71231a5..27cbefe 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1689,6 +1689,7 @@ 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_dax_supported(struct super_block *, int);
+extern bool bdev_dax_capable(struct block_device *);
 #else /* CONFIG_BLOCK */
 
 struct block_device;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 70e61b5..8363a10 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2320,14 +2320,6 @@ extern struct super_block *freeze_bdev(struct block_device *);
 extern void emergency_thaw_all(void);
 extern int thaw_bdev(struct block_device *bdev, struct super_block *sb);
 extern int fsync_bdev(struct block_device *);
-#ifdef CONFIG_FS_DAX
-extern bool blkdev_dax_capable(struct block_device *bdev);
-#else
-static inline bool blkdev_dax_capable(struct block_device *bdev)
-{
-	return false;
-}
-#endif
 
 extern struct super_block *blockdev_superblock;
 

[toc] | [next] | [standalone]


#1398452

FromDan Williams <dan.j.williams@intel.com>
Date2016-05-10 21:50 +0200
Message-ID<rxlPk-Q2-19@gated-at.bofh.it>
In reply to#1398313
On Tue, May 10, 2016 at 9:23 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> to remain as a separate interface for checking dax capability of
> a raw block device.
>
> Rename and relocate blkdev_dax_capable() to keep them maintained
> consistently, and call bdev_direct_access() for the dax capability
> check.
>
> There is no change in the behavior.
>
> Link: https://lkml.org/lkml/2016/5/9/950
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Jens Axboe <axboe@fb.com>
> 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>
> ---
>  block/ioctl.c          |   30 ------------------------------
>  fs/block_dev.c         |   39 +++++++++++++++++++++++++++++++++++++--
>  include/linux/blkdev.h |    1 +
>  include/linux/fs.h     |    8 --------
>  4 files changed, 38 insertions(+), 40 deletions(-)
>
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 4ff1f92..7eeda07 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -4,7 +4,6 @@
>  #include <linux/gfp.h>
>  #include <linux/blkpg.h>
>  #include <linux/hdreg.h>
> -#include <linux/badblocks.h>
>  #include <linux/backing-dev.h>
>  #include <linux/fs.h>
>  #include <linux/blktrace_api.h>
> @@ -407,35 +406,6 @@ static inline int is_unrecognized_ioctl(int ret)
>                 ret == -ENOIOCTLCMD;
>  }
>
> -#ifdef CONFIG_FS_DAX
> -bool blkdev_dax_capable(struct block_device *bdev)
> -{
> -       struct gendisk *disk = bdev->bd_disk;
> -
> -       if (!disk->fops->direct_access)
> -               return false;
> -
> -       /*
> -        * If the partition is not aligned on a page boundary, we can't
> -        * do dax I/O to it.
> -        */
> -       if ((bdev->bd_part->start_sect % (PAGE_SIZE / 512))
> -                       || (bdev->bd_part->nr_sects % (PAGE_SIZE / 512)))
> -               return false;
> -
> -       /*
> -        * If the device has known bad blocks, force all I/O through the
> -        * driver / page cache.
> -        *
> -        * TODO: support finer grained dax error handling
> -        */
> -       if (disk->bb && disk->bb->count)
> -               return false;
> -
> -       return true;
> -}
> -#endif

This will collide with my pending change to revert raw block device
dax support, and also with Vishal's DAX error handling changes.  For
coordination purposes I'm thining this should all go on top of the
branch that Vishal is putting together with the dax zeroing changes
from Jan and Christoph as well.

[toc] | [prev] | [next] | [standalone]


#1398542 — Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency

FromToshi Kani <toshi.kani@hpe.com>
Date2016-05-10 23:50 +0200
SubjectRe: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency
Message-ID<rxnHs-2F8-3@gated-at.bofh.it>
In reply to#1398452
On Tue, 2016-05-10 at 12:49 -0700, Dan Williams wrote:
> On Tue, May 10, 2016 at 9:23 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > 
> > blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> > to remain as a separate interface for checking dax capability of
> > a raw block device.
> > 
> > Rename and relocate blkdev_dax_capable() to keep them maintained
> > consistently, and call bdev_direct_access() for the dax capability
> > check.
> > 
> > There is no change in the behavior.
 :
> > diff --git a/block/ioctl.c b/block/ioctl.c
> > index 4ff1f92..7eeda07 100644
> > --- a/block/ioctl.c
> > +++ b/block/ioctl.c
> > @@ -4,7 +4,6 @@
> >  #include <linux/gfp.h>
> >  #include <linux/blkpg.h>
> >  #include <linux/hdreg.h>
> > -#include <linux/badblocks.h>
> >  #include <linux/backing-dev.h>
> >  #include <linux/fs.h>
> >  #include <linux/blktrace_api.h>
> > @@ -407,35 +406,6 @@ static inline int is_unrecognized_ioctl(int ret)
> >                 ret == -ENOIOCTLCMD;
> >  }
> > 
> > -#ifdef CONFIG_FS_DAX
> > -bool blkdev_dax_capable(struct block_device *bdev)
> > -{
> > -       struct gendisk *disk = bdev->bd_disk;
> > -
> > -       if (!disk->fops->direct_access)
> > -               return false;
> > -
> > -       /*
> > -        * If the partition is not aligned on a page boundary, we can't
> > -        * do dax I/O to it.
> > -        */
> > -       if ((bdev->bd_part->start_sect % (PAGE_SIZE / 512))
> > -                       || (bdev->bd_part->nr_sects % (PAGE_SIZE /
> > 512)))
> > -               return false;
> > -
> > -       /*
> > -        * If the device has known bad blocks, force all I/O through
> > the
> > -        * driver / page cache.
> > -        *
> > -        * TODO: support finer grained dax error handling
> > -        */
> > -       if (disk->bb && disk->bb->count)
> > -               return false;
> > -
> > -       return true;
> > -}
> > -#endif
>
> This will collide with my pending change to revert raw block device
> dax support, and also with Vishal's DAX error handling changes.  For
> coordination purposes I'm thining this should all go on top of the
> branch that Vishal is putting together with the dax zeroing changes
> from Jan and Christoph as well.

This patch does not depend on the rest of the series, so it can be handled
separately.  There is a minor conflict -- bdev_dax_capable() is put under
bdev_dax_supported() in the same file.  This should be easy to resolve, but
let me know if you need me to merge it up.

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]


#1398752

FromJan Kara <jack@suse.cz>
Date2016-05-11 10:10 +0200
Message-ID<rxxnr-467-1@gated-at.bofh.it>
In reply to#1398313
On Tue 10-05-16 10:23:57, Toshi Kani wrote:
> blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> to remain as a separate interface for checking dax capability of
> a raw block device.
> 
> Rename and relocate blkdev_dax_capable() to keep them maintained
> consistently, and call bdev_direct_access() for the dax capability
> check.
...
> +bool bdev_dax_capable(struct block_device *bdev)
> +{
> +	struct gendisk *disk = bdev->bd_disk;
> +	struct blk_dax_ctl dax = {
> +		.size = PAGE_SIZE,
> +	};
> +
> +	if (!IS_ENABLED(CONFIG_FS_DAX))
> +		return false;

Frankly, I prefer the #ifdef CONFIG_FS_DAX and just compile the code out
when DAX is not enabled (like it was with blkdev_dax_capable()). That way we
don't grow the kernel for people who don't care about DAX.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]


#1399188 — Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency

FromToshi Kani <toshi.kani@hpe.com>
Date2016-05-11 16:40 +0200
SubjectRe: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency
Message-ID<rxDsT-1yk-27@gated-at.bofh.it>
In reply to#1398752
On Wed, 2016-05-11 at 10:05 +0200, Jan Kara wrote:
> On Tue 10-05-16 10:23:57, Toshi Kani wrote:
> > 
> > blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> > to remain as a separate interface for checking dax capability of
> > a raw block device.
> > 
> > Rename and relocate blkdev_dax_capable() to keep them maintained
> > consistently, and call bdev_direct_access() for the dax capability
> > check.
> ...
> > 
> > +bool bdev_dax_capable(struct block_device *bdev)
> > +{
> > +	struct gendisk *disk = bdev->bd_disk;
> > +	struct blk_dax_ctl dax = {
> > +		.size = PAGE_SIZE,
> > +	};
> > +
> > +	if (!IS_ENABLED(CONFIG_FS_DAX))
> > +		return false;
>
> Frankly, I prefer the #ifdef CONFIG_FS_DAX and just compile the code out
> when DAX is not enabled (like it was with blkdev_dax_capable()). That way
> we don't grow the kernel for people who don't care about DAX.

When CONFIG_FS_DAX is not set, the rest of the code is optimized out.  So,
I think the code size is the same.

(gdb) disas bdev_dax_capable
Dump of assembler code for function bdev_dax_capable:
   0xffffffff81260d20 <+0>:     callq  0xffffffff81813c30 <__fentry__>
   0xffffffff81260d25 <+5>:     push   %rbp
   0xffffffff81260d26 <+6>:     xor    %eax,%eax
   0xffffffff81260d28 <+8>:     mov    %rsp,%rbp
   0xffffffff81260d2b <+11>:    pop    %rbp
   0xffffffff81260d2c <+12>:    retq   
End of assembler dump.

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]


#1399260

FromJan Kara <jack@suse.cz>
Date2016-05-11 17:30 +0200
Message-ID<rxEfg-2vs-25@gated-at.bofh.it>
In reply to#1399188
On Wed 11-05-16 08:25:10, Toshi Kani wrote:
> On Wed, 2016-05-11 at 10:05 +0200, Jan Kara wrote:
> > On Tue 10-05-16 10:23:57, Toshi Kani wrote:
> > > 
> > > blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> > > to remain as a separate interface for checking dax capability of
> > > a raw block device.
> > > 
> > > Rename and relocate blkdev_dax_capable() to keep them maintained
> > > consistently, and call bdev_direct_access() for the dax capability
> > > check.
> > ...
> > > 
> > > +bool bdev_dax_capable(struct block_device *bdev)
> > > +{
> > > +	struct gendisk *disk = bdev->bd_disk;
> > > +	struct blk_dax_ctl dax = {
> > > +		.size = PAGE_SIZE,
> > > +	};
> > > +
> > > +	if (!IS_ENABLED(CONFIG_FS_DAX))
> > > +		return false;
> >
> > Frankly, I prefer the #ifdef CONFIG_FS_DAX and just compile the code out
> > when DAX is not enabled (like it was with blkdev_dax_capable()). That way
> > we don't grow the kernel for people who don't care about DAX.
> 
> When CONFIG_FS_DAX is not set, the rest of the code is optimized out.  So,
> I think the code size is the same.
> 
> (gdb) disas bdev_dax_capable
> Dump of assembler code for function bdev_dax_capable:
>    0xffffffff81260d20 <+0>:     callq  0xffffffff81813c30 <__fentry__>
>    0xffffffff81260d25 <+5>:     push   %rbp
>    0xffffffff81260d26 <+6>:     xor    %eax,%eax
>    0xffffffff81260d28 <+8>:     mov    %rsp,%rbp
>    0xffffffff81260d2b <+11>:    pop    %rbp
>    0xffffffff81260d2c <+12>:    retq   
> End of assembler dump.

Ah, good. So feel free to add:

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

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web