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

Groups > linux.kernel > #1321826 > unrolled thread

[PATCH 2/2] block: use DAX for partition table reads

Started byDan Williams <dan.j.williams@intel.com>
First post2016-01-29 16:20 +0100
Last post2016-01-30 01:40 +0100
Articles 6 — 4 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 2/2] block: use DAX for partition table reads Dan Williams <dan.j.williams@intel.com> - 2016-01-29 16:20 +0100
    Re: [PATCH 2/2] block: use DAX for partition table reads Jens Axboe <axboe@fb.com> - 2016-01-29 18:50 +0100
      Re: [PATCH 2/2] block: use DAX for partition table reads Dan Williams <dan.j.williams@intel.com> - 2016-01-29 19:00 +0100
    Re: [PATCH 2/2] block: use DAX for partition table reads Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-01-29 20:30 +0100
    Re: [PATCH 2/2] block: use DAX for partition table reads Matthew Wilcox <willy@linux.intel.com> - 2016-01-29 23:50 +0100
    [PATCH v2] block: use DAX for partition table reads Dan Williams <dan.j.williams@intel.com> - 2016-01-30 01:40 +0100

#1321826 — [PATCH 2/2] block: use DAX for partition table reads

FromDan Williams <dan.j.williams@intel.com>
Date2016-01-29 16:20 +0100
Subject[PATCH 2/2] block: use DAX for partition table reads
Message-ID<qWj07-517-55@gated-at.bofh.it>
Avoid populating pagecache when the block device is in DAX mode.
Otherwise these page cache entries collide with the fsync/msync
implementation and break data durability guarantees.

Cc: Jan Kara <jack@suse.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 block/partition-generic.c |   18 +++++++++++++++---
 fs/dax.c                  |   20 ++++++++++++++++++++
 include/linux/blkdev.h    |   10 ++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/block/partition-generic.c b/block/partition-generic.c
index 746935a5973c..8e6fa1868249 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -16,6 +16,7 @@
 #include <linux/kmod.h>
 #include <linux/ctype.h>
 #include <linux/genhd.h>
+#include <linux/blkdev.h>
 #include <linux/blktrace_api.h>
 
 #include "partitions/check.h"
@@ -550,13 +551,24 @@ int invalidate_partitions(struct gendisk *disk, struct block_device *bdev)
 	return 0;
 }
 
-unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
+static struct page *read_pagecache_sector(struct block_device *bdev, sector_t n)
 {
 	struct address_space *mapping = bdev->bd_inode->i_mapping;
+
+	return read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
+			NULL);
+}
+
+unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
+{
 	struct page *page;
 
-	page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
-				 NULL);
+	/* don't populate page cache for dax capable devices */
+	if (IS_DAX(bdev->bd_inode))
+		page = read_dax_sector(bdev, n);
+	else
+		page = read_pagecache_sector(bdev, n);
+
 	if (!IS_ERR(page)) {
 		if (PageError(page))
 			goto fail;
diff --git a/fs/dax.c b/fs/dax.c
index 4fd6b0c5c6b5..227974adecb9 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -58,6 +58,26 @@ static void dax_unmap_atomic(struct block_device *bdev,
 	blk_queue_exit(bdev->bd_queue);
 }
 
+struct page *read_dax_sector(struct block_device *bdev, sector_t n)
+{
+	struct page *page = __page_cache_alloc(GFP_KERNEL | __GFP_COLD);
+	struct blk_dax_ctl dax = {
+		.size = PAGE_SIZE,
+		.sector = n & ~((((int) PAGE_SIZE) / 512) - 1),
+	};
+	long rc;
+
+	if (!page)
+		return ERR_PTR(-ENOMEM);
+
+	rc = dax_map_atomic(bdev, &dax);
+	if (rc < 0)
+		return ERR_PTR(rc);
+	memcpy_from_pmem(page_address(page), dax.addr, PAGE_SIZE);
+	dax_unmap_atomic(bdev, &dax);
+	return page;
+}
+
 /*
  * dax_clear_blocks() is called from within transaction context from XFS,
  * and hence this means the stack from this point must follow GFP_NOFS
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 29189aeace19..b1452c04f1a9 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1633,6 +1633,16 @@ struct blk_dax_ctl {
 	pfn_t pfn;
 };
 
+#ifdef CONFIG_FS_DAX
+struct page *read_dax_sector(struct block_device *bdev, sector_t n);
+#else
+static inline struct page *read_dax_sector(struct block_device *bdev,
+		sector_t n)
+{
+	return ERR_PTR(-ENXIO);
+}
+#endif
+
 struct block_device_operations {
 	int (*open) (struct block_device *, fmode_t);
 	void (*release) (struct gendisk *, fmode_t);

[toc] | [next] | [standalone]

#1321918

FromJens Axboe <axboe@fb.com>
Date2016-01-29 18:50 +0100
Message-ID<qWllf-6BJ-7@gated-at.bofh.it>
In reply to#1321826
On 01/29/2016 08:18 AM, Dan Williams wrote:
> +unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
> +{
>   	struct page *page;
>
> -	page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
> -				 NULL);
> +	/* don't populate page cache for dax capable devices */
> +	if (IS_DAX(bdev->bd_inode))
> +		page = read_dax_sector(bdev, n);
> +	else
> +		page = read_pagecache_sector(bdev, n);
> +

Fall back to non-dax, if dax fails?

> +struct page *read_dax_sector(struct block_device *bdev, sector_t n)
> +{
> +	struct page *page = __page_cache_alloc(GFP_KERNEL | __GFP_COLD);

Why isn't that just alloc_pages()?

-- 
Jens Axboe

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

#1321926

FromDan Williams <dan.j.williams@intel.com>
Date2016-01-29 19:00 +0100
Message-ID<qWluW-6FF-5@gated-at.bofh.it>
In reply to#1321918
On Fri, Jan 29, 2016 at 9:46 AM, Jens Axboe <axboe@fb.com> wrote:
> On 01/29/2016 08:18 AM, Dan Williams wrote:
>>
>> +unsigned char *read_dev_sector(struct block_device *bdev, sector_t n,
>> Sector *p)
>> +{
>>         struct page *page;
>>
>> -       page = read_mapping_page(mapping, (pgoff_t)(n >>
>> (PAGE_CACHE_SHIFT-9)),
>> -                                NULL);
>> +       /* don't populate page cache for dax capable devices */
>> +       if (IS_DAX(bdev->bd_inode))
>> +               page = read_dax_sector(bdev, n);
>> +       else
>> +               page = read_pagecache_sector(bdev, n);
>> +
>
>
> Fall back to non-dax, if dax fails?

I think we need to fail hard otherwise we're back to the original
problem of confusing the dax code that expects to find an empty page
cache.

>
>> +struct page *read_dax_sector(struct block_device *bdev, sector_t n)
>> +{
>> +       struct page *page = __page_cache_alloc(GFP_KERNEL | __GFP_COLD);
>
>
> Why isn't that just alloc_pages()?

Just for symmetry with the same allocation that the pagecache path
makes, but alloc_pages() works too...

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

#1322010

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-01-29 20:30 +0100
Message-ID<qWmU2-7SX-1@gated-at.bofh.it>
In reply to#1321826
On Fri, Jan 29, 2016 at 07:18:46AM -0800, Dan Williams wrote:
> Avoid populating pagecache when the block device is in DAX mode.
> Otherwise these page cache entries collide with the fsync/msync
> implementation and break data durability guarantees.
> 
> Cc: Jan Kara <jack@suse.com>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Dave Chinner <david@fromorbit.com>
> Cc: Matthew Wilcox <willy@linux.intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

This solves the problem for me, thanks!

Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>

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

#1322154

FromMatthew Wilcox <willy@linux.intel.com>
Date2016-01-29 23:50 +0100
Message-ID<qWq1A-1yb-21@gated-at.bofh.it>
In reply to#1321826
On Fri, Jan 29, 2016 at 07:18:46AM -0800, Dan Williams wrote:
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 29189aeace19..b1452c04f1a9 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1633,6 +1633,16 @@ struct blk_dax_ctl {
>  	pfn_t pfn;
>  };
>  
> +#ifdef CONFIG_FS_DAX
> +struct page *read_dax_sector(struct block_device *bdev, sector_t n);
> +#else
> +static inline struct page *read_dax_sector(struct block_device *bdev,
> +		sector_t n)
> +{
> +	return ERR_PTR(-ENXIO);
> +}
> +#endif
> +

Can you move this to include/linux/dax.h?  I'd like to keep it that all
functions in dax.c have a prototype in dax.h.

With that change, Reviewed-by: Matthew Wilcox <willy@linux.intel.com>

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

#1322205 — [PATCH v2] block: use DAX for partition table reads

FromDan Williams <dan.j.williams@intel.com>
Date2016-01-30 01:40 +0100
Subject[PATCH v2] block: use DAX for partition table reads
Message-ID<qWrK3-2Ro-33@gated-at.bofh.it>
In reply to#1321826
Avoid populating pagecache when the block device is in DAX mode.
Otherwise these page cache entries collide with the fsync/msync
implementation and break data durability guarantees.

Cc: Jan Kara <jack@suse.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Matthew Wilcox <willy@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes in v2:
1/ Switch from __page_cache_alloc to alloc_pages (Jens)

2/ Move read_dax_sector() declaration to include/linux/dax.h (Willy)

3/ Collect Reviewed-by and Tested-by tags from Willy and Ross.

 block/partition-generic.c |   18 +++++++++++++++---
 fs/dax.c                  |   20 ++++++++++++++++++++
 include/linux/dax.h       |   11 +++++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/block/partition-generic.c b/block/partition-generic.c
index 746935a5973c..fefd01b496a0 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -16,6 +16,7 @@
 #include <linux/kmod.h>
 #include <linux/ctype.h>
 #include <linux/genhd.h>
+#include <linux/dax.h>
 #include <linux/blktrace_api.h>
 
 #include "partitions/check.h"
@@ -550,13 +551,24 @@ int invalidate_partitions(struct gendisk *disk, struct block_device *bdev)
 	return 0;
 }
 
-unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
+static struct page *read_pagecache_sector(struct block_device *bdev, sector_t n)
 {
 	struct address_space *mapping = bdev->bd_inode->i_mapping;
+
+	return read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
+			NULL);
+}
+
+unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
+{
 	struct page *page;
 
-	page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
-				 NULL);
+	/* don't populate page cache for dax capable devices */
+	if (IS_DAX(bdev->bd_inode))
+		page = read_dax_sector(bdev, n);
+	else
+		page = read_pagecache_sector(bdev, n);
+
 	if (!IS_ERR(page)) {
 		if (PageError(page))
 			goto fail;
diff --git a/fs/dax.c b/fs/dax.c
index 4fd6b0c5c6b5..e0e9358baf35 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -58,6 +58,26 @@ static void dax_unmap_atomic(struct block_device *bdev,
 	blk_queue_exit(bdev->bd_queue);
 }
 
+struct page *read_dax_sector(struct block_device *bdev, sector_t n)
+{
+	struct page *page = alloc_pages(GFP_KERNEL, 0);
+	struct blk_dax_ctl dax = {
+		.size = PAGE_SIZE,
+		.sector = n & ~((((int) PAGE_SIZE) / 512) - 1),
+	};
+	long rc;
+
+	if (!page)
+		return ERR_PTR(-ENOMEM);
+
+	rc = dax_map_atomic(bdev, &dax);
+	if (rc < 0)
+		return ERR_PTR(rc);
+	memcpy_from_pmem(page_address(page), dax.addr, PAGE_SIZE);
+	dax_unmap_atomic(bdev, &dax);
+	return page;
+}
+
 /*
  * dax_clear_blocks() is called from within transaction context from XFS,
  * and hence this means the stack from this point must follow GFP_NOFS
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 8204c3dc3800..818e45078929 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -14,6 +14,17 @@ int dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t,
 		dax_iodone_t);
 int __dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t,
 		dax_iodone_t);
+
+#ifdef CONFIG_FS_DAX
+struct page *read_dax_sector(struct block_device *bdev, sector_t n);
+#else
+static inline struct page *read_dax_sector(struct block_device *bdev,
+		sector_t n)
+{
+	return ERR_PTR(-ENXIO);
+}
+#endif
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 int dax_pmd_fault(struct vm_area_struct *, unsigned long addr, pmd_t *,
 				unsigned int flags, get_block_t, dax_iodone_t);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web