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


Groups > linux.kernel > #1260353 > unrolled thread

[PATCH v3 00/15] block, dax updates for 4.4

Started byDan Williams <dan.j.williams@intel.com>
First post2015-11-02 05:40 +0100
Last post2015-11-02 05:40 +0100
Articles 20 on this page of 27 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 00/15] block, dax updates for 4.4 Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
    [PATCH v3 12/15] block: enable dax for raw block devices Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
    [PATCH v3 04/15] libnvdimm,  pmem: move request_queue allocation earlier in probe Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
      Re: [PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation  earlier in probe Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-11-03 20:20 +0100
    [PATCH v3 07/15] kvm: rename pfn_t to kvm_pfn_t Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
    [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
      Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dave Chinner <david@fromorbit.com> - 2015-11-03 01:40 +0100
        Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dan Williams <dan.j.williams@intel.com> - 2015-11-03 08:40 +0100
          Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dave Chinner <david@fromorbit.com> - 2015-11-03 21:30 +0100
            Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dan Williams <dan.j.williams@intel.com> - 2015-11-04 00:10 +0100
              Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dan Williams <dan.j.williams@intel.com> - 2015-11-04 20:30 +0100
    [PATCH v3 05/15] libnvdimm,  pmem: fix size trim in pmem_direct_access() Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
      Re: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in  pmem_direct_access() Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-11-03 20:40 +0100
        Re: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access() Dan Williams <dan.j.williams@intel.com> - 2015-11-03 22:40 +0100
    [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
      Re: [PATCH v3 14/15] dax: dirty extent notification Dave Chinner <david@fromorbit.com> - 2015-11-03 02:20 +0100
        Re: [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-03 06:10 +0100
          Re: [PATCH v3 14/15] dax: dirty extent notification Dave Chinner <david@fromorbit.com> - 2015-11-03 06:50 +0100
            Re: [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-03 08:30 +0100
              Re: [PATCH v3 14/15] dax: dirty extent notification Dave Chinner <david@fromorbit.com> - 2015-11-03 22:00 +0100
                Re: [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-03 22:20 +0100
                Re: [PATCH v3 14/15] dax: dirty extent notification Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-11-03 22:40 +0100
                  Re: [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-03 22:50 +0100
          Re: [PATCH v3 14/15] dax: dirty extent notification Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-11-03 22:20 +0100
            Re: [PATCH v3 14/15] dax: dirty extent notification Dan Williams <dan.j.williams@intel.com> - 2015-11-03 22:40 +0100
    [PATCH v3 10/15] dax,  pmem: introduce zone_device_revoke() and devm_memunmap_pages() Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100
    [PATCH v3 06/15] um: kill pfn_t Dan Williams <dan.j.williams@intel.com> - 2015-11-02 05:40 +0100

Page 1 of 2  [1] 2  Next page →


#1260353 — [PATCH v3 00/15] block, dax updates for 4.4

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 00/15] block, dax updates for 4.4
Message-ID<qqf4t-Og-3@gated-at.bofh.it>
Changes since v2: [1]

1/ Include a fix to revoke dax-mappings at device teardown time

2/ Include a blkdev_issue_flush implementation for pmem

3/ New block device ioctls to set/query dax mode, make dax opt-in

4/ Collect reviewed-by's

[1]: https://lists.01.org/pipermail/linux-nvdimm/2015-October/002538.html

---

There were several topics developed during this cycle, some are ready
and some are not.  The items needing more review and testing are the
pmem+dax updates with mm, ext4, and xfs dependencies.  Topics like the
dax locking reworks, dax get_user_pages, and write-protecting dax ptes
after fsync/msync.

Instead, this more conservative set represents the fs and mm independent
updates to dax and pmem:

1/ Enable dax mappings for raw block devices

2/ Use blk_queue_{enter|exit} and the availability of 'struct page' to
   fix lifetime issues and races with unloading the pmem driver.

3/ Enable blkdev_issue_flush to fix legacy apps that may dirty pmem via
   dax mmap I/O.  Although legacy environments should consider disabling
   dax for apps that don't expect it.

These depend on the latest 'libnvdimm-for-next' branch from nvdimm.git
and the 'for-4.4/integrity' branch from Jens' tree.  All but the last
three have been out for review previously, and I am looking to submit
them towards the back half of the merge window.

---

Dan Williams (15):
      pmem, dax: clean up clear_pmem()
      dax: increase granularity of dax_clear_blocks() operations
      block, dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()
      libnvdimm, pmem: move request_queue allocation earlier in probe
      libnvdimm, pmem: fix size trim in pmem_direct_access()
      um: kill pfn_t
      kvm: rename pfn_t to kvm_pfn_t
      mm, dax, pmem: introduce pfn_t
      block: notify queue death confirmation
      dax, pmem: introduce zone_device_revoke() and devm_memunmap_pages()
      block: introduce bdev_file_inode()
      block: enable dax for raw block devices
      block, dax: make dax mappings opt-in by default
      dax: dirty extent notification
      pmem: blkdev_issue_flush support


 arch/arm/include/asm/kvm_mmu.h        |    5 -
 arch/arm/kvm/mmu.c                    |   10 +
 arch/arm64/include/asm/kvm_mmu.h      |    3 
 arch/mips/include/asm/kvm_host.h      |    6 -
 arch/mips/kvm/emulate.c               |    2 
 arch/mips/kvm/tlb.c                   |   14 +-
 arch/powerpc/include/asm/kvm_book3s.h |    4 -
 arch/powerpc/include/asm/kvm_ppc.h    |    2 
 arch/powerpc/kvm/book3s.c             |    6 -
 arch/powerpc/kvm/book3s_32_mmu_host.c |    2 
 arch/powerpc/kvm/book3s_64_mmu_host.c |    2 
 arch/powerpc/kvm/e500.h               |    2 
 arch/powerpc/kvm/e500_mmu_host.c      |    8 +
 arch/powerpc/kvm/trace_pr.h           |    2 
 arch/powerpc/sysdev/axonram.c         |   11 +
 arch/um/include/asm/page.h            |    6 -
 arch/um/include/asm/pgtable-3level.h  |    4 -
 arch/um/include/asm/pgtable.h         |    2 
 arch/x86/include/asm/cacheflush.h     |    4 +
 arch/x86/include/asm/pmem.h           |    7 -
 arch/x86/kvm/iommu.c                  |   11 +
 arch/x86/kvm/mmu.c                    |   37 +++--
 arch/x86/kvm/mmu_audit.c              |    2 
 arch/x86/kvm/paging_tmpl.h            |    6 -
 arch/x86/kvm/vmx.c                    |    2 
 arch/x86/kvm/x86.c                    |    2 
 block/blk-core.c                      |   13 +-
 block/blk-mq.c                        |   19 ++-
 block/blk.h                           |   13 --
 block/ioctl.c                         |   42 ++++++
 drivers/block/brd.c                   |   10 +
 drivers/nvdimm/pmem.c                 |  247 ++++++++++++++++++++++++++++-----
 drivers/s390/block/dcssblk.c          |   13 +-
 fs/block_dev.c                        |  159 ++++++++++++++++++---
 fs/dax.c                              |  242 ++++++++++++++++++++++----------
 include/linux/blkdev.h                |   41 +++++
 include/linux/fs.h                    |   11 +
 include/linux/io.h                    |   17 --
 include/linux/kvm_host.h              |   37 +++--
 include/linux/kvm_types.h             |    2 
 include/linux/mm.h                    |   90 ++++++++++++
 include/linux/pfn.h                   |    9 +
 include/uapi/linux/fs.h               |    2 
 kernel/memremap.c                     |   98 +++++++++++++
 virt/kvm/kvm_main.c                   |   47 +++---
 45 files changed, 949 insertions(+), 325 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1260354 — [PATCH v3 12/15] block: enable dax for raw block devices

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 12/15] block: enable dax for raw block devices
Message-ID<qqf4u-Og-17@gated-at.bofh.it>
In reply to#1260353
If an application wants exclusive access to all of the persistent memory
provided by an NVDIMM namespace it can use this raw-block-dax facility
to forgo establishing a filesystem.  This capability is targeted
primarily to hypervisors wanting to provision persistent memory for
guests.  It can be disabled / enabled dynamically via the new BLKDAXSET
ioctl.

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>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Reviewed-by: Jan Kara <jack@suse.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 block/ioctl.c           |   43 ++++++++++++++++++++++
 fs/block_dev.c          |   90 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/fs.h      |    3 ++
 include/uapi/linux/fs.h |    2 +
 4 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 8061eba42887..205d57612fbd 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -295,6 +295,35 @@ static inline int is_unrecognized_ioctl(int ret)
 		ret == -ENOIOCTLCMD;
 }
 
+#ifdef CONFIG_FS_DAX
+static int blkdev_set_dax(struct block_device *bdev, int n)
+{
+	struct gendisk *disk = bdev->bd_disk;
+	int rc = 0;
+
+	if (n)
+		n = S_DAX;
+
+	if (n && !disk->fops->direct_access)
+		return -ENOTTY;
+
+	mutex_lock(&bdev->bd_inode->i_mutex);
+	if (bdev->bd_map_count == 0)
+		inode_set_flags(bdev->bd_inode, n, S_DAX);
+	else
+		rc = -EBUSY;
+	mutex_unlock(&bdev->bd_inode->i_mutex);
+	return rc;
+}
+#else
+static int blkdev_set_dax(struct block_device *bdev, int n)
+{
+	if (n)
+		return -ENOTTY;
+	return 0;
+}
+#endif
+
 /*
  * always keep this in sync with compat_blkdev_ioctl()
  */
@@ -449,6 +478,20 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	case BLKTRACETEARDOWN:
 		ret = blk_trace_ioctl(bdev, cmd, (char __user *) arg);
 		break;
+	case BLKDAXSET:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EACCES;
+
+		if (get_user(n, (int __user *)(arg)))
+			return -EFAULT;
+		n = !!n;
+		if (n == !!(bdev->bd_inode->i_flags & S_DAX))
+			return 0;
+
+		return blkdev_set_dax(bdev, n);
+	case BLKDAXGET:
+		return put_int(arg, !!(bdev->bd_inode->i_flags & S_DAX));
+		break;
 	default:
 		ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
 	}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 29cd1eb0765d..13ce6d0ff7f6 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1687,13 +1687,101 @@ static const struct address_space_operations def_blk_aops = {
 	.is_dirty_writeback = buffer_check_dirty_writeback,
 };
 
+#ifdef CONFIG_FS_DAX
+/*
+ * In the raw block case we do not need to contend with truncation nor
+ * unwritten file extents.  Without those concerns there is no need for
+ * additional locking beyond the mmap_sem context that these routines
+ * are already executing under.
+ *
+ * Note, there is no protection if the block device is dynamically
+ * resized (partition grow/shrink) during a fault. A stable block device
+ * size is already not enforced in the blkdev_direct_IO path.
+ *
+ * For DAX, it is the responsibility of the block device driver to
+ * ensure the whole-disk device size is stable while requests are in
+ * flight.
+ *
+ * Finally, unlike the filemap_page_mkwrite() case there is no
+ * filesystem superblock to sync against freezing.  We still include a
+ * pfn_mkwrite callback for dax drivers to receive write fault
+ * notifications.
+ */
+static int blkdev_dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	return __dax_fault(vma, vmf, blkdev_get_block, NULL);
+}
+
+static int blkdev_dax_pmd_fault(struct vm_area_struct *vma, unsigned long addr,
+		pmd_t *pmd, unsigned int flags)
+{
+	return __dax_pmd_fault(vma, addr, pmd, flags, blkdev_get_block, NULL);
+}
+
+static void blkdev_vm_open(struct vm_area_struct *vma)
+{
+	struct inode *bd_inode = bdev_file_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+
+	mutex_lock(&bd_inode->i_mutex);
+	bdev->bd_map_count++;
+	mutex_unlock(&bd_inode->i_mutex);
+}
+
+static void blkdev_vm_close(struct vm_area_struct *vma)
+{
+	struct inode *bd_inode = bdev_file_inode(vma->vm_file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+
+	mutex_lock(&bd_inode->i_mutex);
+	bdev->bd_map_count--;
+	mutex_unlock(&bd_inode->i_mutex);
+}
+
+static const struct vm_operations_struct blkdev_dax_vm_ops = {
+	.open		= blkdev_vm_open,
+	.close		= blkdev_vm_close,
+	.fault		= blkdev_dax_fault,
+	.pmd_fault	= blkdev_dax_pmd_fault,
+	.pfn_mkwrite	= blkdev_dax_fault,
+};
+
+static const struct vm_operations_struct blkdev_default_vm_ops = {
+	.open		= blkdev_vm_open,
+	.close		= blkdev_vm_close,
+	.fault		= filemap_fault,
+	.map_pages	= filemap_map_pages,
+};
+
+static int blkdev_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct inode *bd_inode = bdev_file_inode(file);
+	struct block_device *bdev = I_BDEV(bd_inode);
+
+	file_accessed(file);
+	mutex_lock(&bd_inode->i_mutex);
+	bdev->bd_map_count++;
+	if (IS_DAX(bd_inode)) {
+		vma->vm_ops = &blkdev_dax_vm_ops;
+		vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
+	} else {
+		vma->vm_ops = &blkdev_default_vm_ops;
+	}
+	mutex_unlock(&bd_inode->i_mutex);
+
+	return 0;
+}
+#else
+#define blkdev_mmap generic_file_mmap
+#endif
+
 const struct file_operations def_blk_fops = {
 	.open		= blkdev_open,
 	.release	= blkdev_close,
 	.llseek		= block_llseek,
 	.read_iter	= blkdev_read_iter,
 	.write_iter	= blkdev_write_iter,
-	.mmap		= generic_file_mmap,
+	.mmap		= blkdev_mmap,
 	.fsync		= blkdev_fsync,
 	.unlocked_ioctl	= block_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 72d8a844c692..8fb2d4b848bf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -482,6 +482,9 @@ struct block_device {
 	int			bd_fsfreeze_count;
 	/* Mutex for freeze */
 	struct mutex		bd_fsfreeze_mutex;
+#ifdef CONFIG_FS_DAX
+	int			bd_map_count;
+#endif
 };
 
 /*
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 9b964a5920af..cc2f0fdae707 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -152,6 +152,8 @@ struct inodes_stat_t {
 #define BLKSECDISCARD _IO(0x12,125)
 #define BLKROTATIONAL _IO(0x12,126)
 #define BLKZEROOUT _IO(0x12,127)
+#define BLKDAXSET _IO(0x12,128)
+#define BLKDAXGET _IO(0x12,129)
 
 #define BMAP_IOCTL 1		/* obsolete - kept for compatibility */
 #define FIBMAP	   _IO(0x00,1)	/* bmap access */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1260355 — [PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation earlier in probe

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation earlier in probe
Message-ID<qqf4u-Og-21@gated-at.bofh.it>
In reply to#1260353
Before the dynamically allocated struct pages from devm_memremap_pages()
can be put to use outside the driver, we need a mechanism to track
whether they are still in use at teardown.  Towards that goal reorder
the initialization sequence to allow the 'q_usage_counter' from the
request_queue to be used by the devm_memremap_pages() implementation (in
subsequent patches).

Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/pmem.c |   37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 349f03e7ed06..e46988fbdee5 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -133,6 +133,7 @@ static struct pmem_device *pmem_alloc(struct device *dev,
 		struct resource *res, int id)
 {
 	struct pmem_device *pmem;
+	struct request_queue *q;
 
 	pmem = devm_kzalloc(dev, sizeof(*pmem), GFP_KERNEL);
 	if (!pmem)
@@ -150,16 +151,23 @@ static struct pmem_device *pmem_alloc(struct device *dev,
 		return ERR_PTR(-EBUSY);
 	}
 
-	if (pmem_should_map_pages(dev))
+	q = blk_alloc_queue_node(GFP_KERNEL, dev_to_node(dev));
+	if (!q)
+		return ERR_PTR(-ENOMEM);
+
+	if (pmem_should_map_pages(dev)) {
 		pmem->virt_addr = (void __pmem *) devm_memremap_pages(dev, res);
-	else
+	} else
 		pmem->virt_addr = (void __pmem *) devm_memremap(dev,
 				pmem->phys_addr, pmem->size,
 				ARCH_MEMREMAP_PMEM);
 
-	if (IS_ERR(pmem->virt_addr))
+	if (IS_ERR(pmem->virt_addr)) {
+		blk_cleanup_queue(q);
 		return (void __force *) pmem->virt_addr;
+	}
 
+	pmem->pmem_queue = q;
 	return pmem;
 }
 
@@ -179,10 +187,6 @@ static int pmem_attach_disk(struct device *dev,
 	int nid = dev_to_node(dev);
 	struct gendisk *disk;
 
-	pmem->pmem_queue = blk_alloc_queue_node(GFP_KERNEL, nid);
-	if (!pmem->pmem_queue)
-		return -ENOMEM;
-
 	blk_queue_make_request(pmem->pmem_queue, pmem_make_request);
 	blk_queue_physical_block_size(pmem->pmem_queue, PAGE_SIZE);
 	blk_queue_max_hw_sectors(pmem->pmem_queue, UINT_MAX);
@@ -400,19 +404,22 @@ static int nd_pmem_probe(struct device *dev)
 	dev_set_drvdata(dev, pmem);
 	ndns->rw_bytes = pmem_rw_bytes;
 
-	if (is_nd_btt(dev))
+	if (is_nd_btt(dev)) {
+		/* btt allocates its own request_queue */
+		blk_cleanup_queue(pmem->pmem_queue);
+		pmem->pmem_queue = NULL;
 		return nvdimm_namespace_attach_btt(ndns);
+	}
 
 	if (is_nd_pfn(dev))
 		return nvdimm_namespace_attach_pfn(ndns);
 
-	if (nd_btt_probe(ndns, pmem) == 0) {
-		/* we'll come back as btt-pmem */
-		return -ENXIO;
-	}
-
-	if (nd_pfn_probe(ndns, pmem) == 0) {
-		/* we'll come back as pfn-pmem */
+	if (nd_btt_probe(ndns, pmem) == 0 || nd_pfn_probe(ndns, pmem) == 0) {
+		/*
+		 * We'll come back as either btt-pmem, or pfn-pmem, so
+		 * drop the queue allocation for now.
+		 */
+		blk_cleanup_queue(pmem->pmem_queue);
 		return -ENXIO;
 	}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261806 — Re: [PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation earlier in probe

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2015-11-03 20:20 +0100
SubjectRe: [PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation earlier in probe
Message-ID<qqPhE-6WD-15@gated-at.bofh.it>
In reply to#1260355
On Sun, Nov 01, 2015 at 11:30:04PM -0500, Dan Williams wrote:
> Before the dynamically allocated struct pages from devm_memremap_pages()
> can be put to use outside the driver, we need a mechanism to track
> whether they are still in use at teardown.  Towards that goal reorder
> the initialization sequence to allow the 'q_usage_counter' from the
> request_queue to be used by the devm_memremap_pages() implementation (in
> subsequent patches).
> 
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
<>
> @@ -150,16 +151,23 @@ static struct pmem_device *pmem_alloc(struct device *dev,
>  		return ERR_PTR(-EBUSY);
>  	}
>  
> -	if (pmem_should_map_pages(dev))
> +	q = blk_alloc_queue_node(GFP_KERNEL, dev_to_node(dev));
> +	if (!q)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (pmem_should_map_pages(dev)) {

No need to introduce braces for this if().

Otherwise this looks fine.

Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1260356 — [PATCH v3 07/15] kvm: rename pfn_t to kvm_pfn_t

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 07/15] kvm: rename pfn_t to kvm_pfn_t
Message-ID<qqf4u-Og-19@gated-at.bofh.it>
In reply to#1260353
The core has developed a need for a "pfn_t" type [1].  Move the existing
pfn_t in KVM to kvm_pfn_t [2].

[1]: https://lists.01.org/pipermail/linux-nvdimm/2015-September/002199.html
[2]: https://lists.01.org/pipermail/linux-nvdimm/2015-September/002218.html

Cc: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/arm/include/asm/kvm_mmu.h        |    5 ++--
 arch/arm/kvm/mmu.c                    |   10 ++++---
 arch/arm64/include/asm/kvm_mmu.h      |    3 +-
 arch/mips/include/asm/kvm_host.h      |    6 ++--
 arch/mips/kvm/emulate.c               |    2 +
 arch/mips/kvm/tlb.c                   |   14 +++++-----
 arch/powerpc/include/asm/kvm_book3s.h |    4 +--
 arch/powerpc/include/asm/kvm_ppc.h    |    2 +
 arch/powerpc/kvm/book3s.c             |    6 ++--
 arch/powerpc/kvm/book3s_32_mmu_host.c |    2 +
 arch/powerpc/kvm/book3s_64_mmu_host.c |    2 +
 arch/powerpc/kvm/e500.h               |    2 +
 arch/powerpc/kvm/e500_mmu_host.c      |    8 +++---
 arch/powerpc/kvm/trace_pr.h           |    2 +
 arch/x86/kvm/iommu.c                  |   11 ++++----
 arch/x86/kvm/mmu.c                    |   37 +++++++++++++-------------
 arch/x86/kvm/mmu_audit.c              |    2 +
 arch/x86/kvm/paging_tmpl.h            |    6 ++--
 arch/x86/kvm/vmx.c                    |    2 +
 arch/x86/kvm/x86.c                    |    2 +
 include/linux/kvm_host.h              |   37 +++++++++++++-------------
 include/linux/kvm_types.h             |    2 +
 virt/kvm/kvm_main.c                   |   47 +++++++++++++++++----------------
 23 files changed, 110 insertions(+), 104 deletions(-)

diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index 405aa1883307..8ebd282dfc2b 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -182,7 +182,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 	return (vcpu->arch.cp15[c1_SCTLR] & 0b101) == 0b101;
 }
 
-static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn,
+static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
+					       kvm_pfn_t pfn,
 					       unsigned long size,
 					       bool ipa_uncached)
 {
@@ -246,7 +247,7 @@ static inline void __kvm_flush_dcache_pte(pte_t pte)
 static inline void __kvm_flush_dcache_pmd(pmd_t pmd)
 {
 	unsigned long size = PMD_SIZE;
-	pfn_t pfn = pmd_pfn(pmd);
+	kvm_pfn_t pfn = pmd_pfn(pmd);
 
 	while (size) {
 		void *va = kmap_atomic_pfn(pfn);
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 6984342da13d..e2dcbfdc4a8c 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -988,9 +988,9 @@ out:
 	return ret;
 }
 
-static bool transparent_hugepage_adjust(pfn_t *pfnp, phys_addr_t *ipap)
+static bool transparent_hugepage_adjust(kvm_pfn_t *pfnp, phys_addr_t *ipap)
 {
-	pfn_t pfn = *pfnp;
+	kvm_pfn_t pfn = *pfnp;
 	gfn_t gfn = *ipap >> PAGE_SHIFT;
 
 	if (PageTransCompound(pfn_to_page(pfn))) {
@@ -1202,7 +1202,7 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 	kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
-static void coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn,
+static void coherent_cache_guest_page(struct kvm_vcpu *vcpu, kvm_pfn_t pfn,
 				      unsigned long size, bool uncached)
 {
 	__coherent_cache_guest_page(vcpu, pfn, size, uncached);
@@ -1219,7 +1219,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	struct kvm *kvm = vcpu->kvm;
 	struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
 	struct vm_area_struct *vma;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	pgprot_t mem_type = PAGE_S2;
 	bool fault_ipa_uncached;
 	bool logging_active = memslot_is_logging(memslot);
@@ -1347,7 +1347,7 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
 {
 	pmd_t *pmd;
 	pte_t *pte;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	bool pfn_valid = false;
 
 	trace_kvm_access_fault(fault_ipa);
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 61505676d085..385fc8cef82d 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -230,7 +230,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 	return (vcpu_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
 }
 
-static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn,
+static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
+					       kvm_pfn_t pfn,
 					       unsigned long size,
 					       bool ipa_uncached)
 {
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 5a1a882e0a75..9c67f05a0a1b 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -101,9 +101,9 @@
 #define CAUSEF_DC			(_ULCAST_(1) << 27)
 
 extern atomic_t kvm_mips_instance;
-extern pfn_t(*kvm_mips_gfn_to_pfn) (struct kvm *kvm, gfn_t gfn);
-extern void (*kvm_mips_release_pfn_clean) (pfn_t pfn);
-extern bool(*kvm_mips_is_error_pfn) (pfn_t pfn);
+extern kvm_pfn_t (*kvm_mips_gfn_to_pfn)(struct kvm *kvm, gfn_t gfn);
+extern void (*kvm_mips_release_pfn_clean)(kvm_pfn_t pfn);
+extern bool (*kvm_mips_is_error_pfn)(kvm_pfn_t pfn);
 
 struct kvm_vm_stat {
 	u32 remote_tlb_flush;
diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index d5fa3eaf39a1..476296cf37d3 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -1525,7 +1525,7 @@ int kvm_mips_sync_icache(unsigned long va, struct kvm_vcpu *vcpu)
 	struct kvm *kvm = vcpu->kvm;
 	unsigned long pa;
 	gfn_t gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	gfn = va >> PAGE_SHIFT;
 
diff --git a/arch/mips/kvm/tlb.c b/arch/mips/kvm/tlb.c
index aed0ac2a4972..570479c03bdc 100644
--- a/arch/mips/kvm/tlb.c
+++ b/arch/mips/kvm/tlb.c
@@ -38,13 +38,13 @@ atomic_t kvm_mips_instance;
 EXPORT_SYMBOL(kvm_mips_instance);
 
 /* These function pointers are initialized once the KVM module is loaded */
-pfn_t (*kvm_mips_gfn_to_pfn)(struct kvm *kvm, gfn_t gfn);
+kvm_pfn_t (*kvm_mips_gfn_to_pfn)(struct kvm *kvm, gfn_t gfn);
 EXPORT_SYMBOL(kvm_mips_gfn_to_pfn);
 
-void (*kvm_mips_release_pfn_clean)(pfn_t pfn);
+void (*kvm_mips_release_pfn_clean)(kvm_pfn_t pfn);
 EXPORT_SYMBOL(kvm_mips_release_pfn_clean);
 
-bool (*kvm_mips_is_error_pfn)(pfn_t pfn);
+bool (*kvm_mips_is_error_pfn)(kvm_pfn_t pfn);
 EXPORT_SYMBOL(kvm_mips_is_error_pfn);
 
 uint32_t kvm_mips_get_kernel_asid(struct kvm_vcpu *vcpu)
@@ -144,7 +144,7 @@ EXPORT_SYMBOL(kvm_mips_dump_guest_tlbs);
 static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
 {
 	int srcu_idx, err = 0;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
 		return 0;
@@ -262,7 +262,7 @@ int kvm_mips_handle_kseg0_tlb_fault(unsigned long badvaddr,
 				    struct kvm_vcpu *vcpu)
 {
 	gfn_t gfn;
-	pfn_t pfn0, pfn1;
+	kvm_pfn_t pfn0, pfn1;
 	unsigned long vaddr = 0;
 	unsigned long entryhi = 0, entrylo0 = 0, entrylo1 = 0;
 	int even;
@@ -313,7 +313,7 @@ EXPORT_SYMBOL(kvm_mips_handle_kseg0_tlb_fault);
 int kvm_mips_handle_commpage_tlb_fault(unsigned long badvaddr,
 	struct kvm_vcpu *vcpu)
 {
-	pfn_t pfn0, pfn1;
+	kvm_pfn_t pfn0, pfn1;
 	unsigned long flags, old_entryhi = 0, vaddr = 0;
 	unsigned long entrylo0 = 0, entrylo1 = 0;
 
@@ -360,7 +360,7 @@ int kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
 {
 	unsigned long entryhi = 0, entrylo0 = 0, entrylo1 = 0;
 	struct kvm *kvm = vcpu->kvm;
-	pfn_t pfn0, pfn1;
+	kvm_pfn_t pfn0, pfn1;
 
 	if ((tlb->tlb_hi & VPN2_MASK) == 0) {
 		pfn0 = 0;
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 9fac01cb89c1..8f39796c9da8 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -154,8 +154,8 @@ extern void kvmppc_set_bat(struct kvm_vcpu *vcpu, struct kvmppc_bat *bat,
 			   bool upper, u32 val);
 extern void kvmppc_giveup_ext(struct kvm_vcpu *vcpu, ulong msr);
 extern int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu);
-extern pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
-			bool *writable);
+extern kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa,
+			bool writing, bool *writable);
 extern void kvmppc_add_revmap_chain(struct kvm *kvm, struct revmap_entry *rev,
 			unsigned long *rmap, long pte_index, int realmode);
 extern void kvmppc_update_rmap_change(unsigned long *rmap, unsigned long psize);
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index c6ef05bd0765..2241d5357129 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -515,7 +515,7 @@ void kvmppc_claim_lpid(long lpid);
 void kvmppc_free_lpid(long lpid);
 void kvmppc_init_lpid(unsigned long nr_lpids);
 
-static inline void kvmppc_mmu_flush_icache(pfn_t pfn)
+static inline void kvmppc_mmu_flush_icache(kvm_pfn_t pfn)
 {
 	struct page *page;
 	/*
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 099c79d8c160..638c6d9be9e0 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -366,7 +366,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvmppc_core_prepare_to_enter);
 
-pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
+kvm_pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 			bool *writable)
 {
 	ulong mp_pa = vcpu->arch.magic_page_pa & KVM_PAM;
@@ -379,9 +379,9 @@ pfn_t kvmppc_gpa_to_pfn(struct kvm_vcpu *vcpu, gpa_t gpa, bool writing,
 	gpa &= ~0xFFFULL;
 	if (unlikely(mp_pa) && unlikely((gpa & KVM_PAM) == mp_pa)) {
 		ulong shared_page = ((ulong)vcpu->arch.shared) & PAGE_MASK;
-		pfn_t pfn;
+		kvm_pfn_t pfn;
 
-		pfn = (pfn_t)virt_to_phys((void*)shared_page) >> PAGE_SHIFT;
+		pfn = (kvm_pfn_t)virt_to_phys((void*)shared_page) >> PAGE_SHIFT;
 		get_page(pfn_to_page(pfn));
 		if (writable)
 			*writable = true;
diff --git a/arch/powerpc/kvm/book3s_32_mmu_host.c b/arch/powerpc/kvm/book3s_32_mmu_host.c
index d5c9bfeb0c9c..55c4d51ea3e2 100644
--- a/arch/powerpc/kvm/book3s_32_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_32_mmu_host.c
@@ -142,7 +142,7 @@ extern char etext[];
 int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte,
 			bool iswrite)
 {
-	pfn_t hpaddr;
+	kvm_pfn_t hpaddr;
 	u64 vpn;
 	u64 vsid;
 	struct kvmppc_sid_map *map;
diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c
index 79ad35abd196..913cd2198fa6 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
@@ -83,7 +83,7 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte,
 			bool iswrite)
 {
 	unsigned long vpn;
-	pfn_t hpaddr;
+	kvm_pfn_t hpaddr;
 	ulong hash, hpteg;
 	u64 vsid;
 	int ret;
diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index 72920bed3ac6..94f04fcb373e 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -41,7 +41,7 @@ enum vcpu_ftr {
 #define E500_TLB_MAS2_ATTR	(0x7f)
 
 struct tlbe_ref {
-	pfn_t pfn;		/* valid only for TLB0, except briefly */
+	kvm_pfn_t pfn;		/* valid only for TLB0, except briefly */
 	unsigned int flags;	/* E500_TLB_* */
 };
 
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 4d33e199edcc..8a5bb6dfcc2d 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -163,9 +163,9 @@ void kvmppc_map_magic(struct kvm_vcpu *vcpu)
 	struct kvm_book3e_206_tlb_entry magic;
 	ulong shared_page = ((ulong)vcpu->arch.shared) & PAGE_MASK;
 	unsigned int stid;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
-	pfn = (pfn_t)virt_to_phys((void *)shared_page) >> PAGE_SHIFT;
+	pfn = (kvm_pfn_t)virt_to_phys((void *)shared_page) >> PAGE_SHIFT;
 	get_page(pfn_to_page(pfn));
 
 	preempt_disable();
@@ -246,7 +246,7 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe)
 
 static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref,
 					 struct kvm_book3e_206_tlb_entry *gtlbe,
-					 pfn_t pfn, unsigned int wimg)
+					 kvm_pfn_t pfn, unsigned int wimg)
 {
 	ref->pfn = pfn;
 	ref->flags = E500_TLB_VALID;
@@ -309,7 +309,7 @@ static void kvmppc_e500_setup_stlbe(
 	int tsize, struct tlbe_ref *ref, u64 gvaddr,
 	struct kvm_book3e_206_tlb_entry *stlbe)
 {
-	pfn_t pfn = ref->pfn;
+	kvm_pfn_t pfn = ref->pfn;
 	u32 pr = vcpu->arch.shared->msr & MSR_PR;
 
 	BUG_ON(!(ref->flags & E500_TLB_VALID));
diff --git a/arch/powerpc/kvm/trace_pr.h b/arch/powerpc/kvm/trace_pr.h
index 810507cb688a..d44f324184fb 100644
--- a/arch/powerpc/kvm/trace_pr.h
+++ b/arch/powerpc/kvm/trace_pr.h
@@ -30,7 +30,7 @@ TRACE_EVENT(kvm_book3s_reenter,
 #ifdef CONFIG_PPC_BOOK3S_64
 
 TRACE_EVENT(kvm_book3s_64_mmu_map,
-	TP_PROTO(int rflags, ulong hpteg, ulong va, pfn_t hpaddr,
+	TP_PROTO(int rflags, ulong hpteg, ulong va, kvm_pfn_t hpaddr,
 		 struct kvmppc_pte *orig_pte),
 	TP_ARGS(rflags, hpteg, va, hpaddr, orig_pte),
 
diff --git a/arch/x86/kvm/iommu.c b/arch/x86/kvm/iommu.c
index 5c520ebf6343..a22a488b4622 100644
--- a/arch/x86/kvm/iommu.c
+++ b/arch/x86/kvm/iommu.c
@@ -43,11 +43,11 @@ static int kvm_iommu_unmap_memslots(struct kvm *kvm);
 static void kvm_iommu_put_pages(struct kvm *kvm,
 				gfn_t base_gfn, unsigned long npages);
 
-static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
+static kvm_pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
 			   unsigned long npages)
 {
 	gfn_t end_gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	pfn     = gfn_to_pfn_memslot(slot, gfn);
 	end_gfn = gfn + npages;
@@ -62,7 +62,8 @@ static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
 	return pfn;
 }
 
-static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
+static void kvm_unpin_pages(struct kvm *kvm, kvm_pfn_t pfn,
+		unsigned long npages)
 {
 	unsigned long i;
 
@@ -73,7 +74,7 @@ static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
 int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 {
 	gfn_t gfn, end_gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	int r = 0;
 	struct iommu_domain *domain = kvm->arch.iommu_domain;
 	int flags;
@@ -275,7 +276,7 @@ static void kvm_iommu_put_pages(struct kvm *kvm,
 {
 	struct iommu_domain *domain;
 	gfn_t end_gfn, gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	u64 phys;
 
 	domain  = kvm->arch.iommu_domain;
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ff606f507913..6ab963ae0427 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -259,7 +259,7 @@ static unsigned get_mmio_spte_access(u64 spte)
 }
 
 static bool set_mmio_spte(struct kvm_vcpu *vcpu, u64 *sptep, gfn_t gfn,
-			  pfn_t pfn, unsigned access)
+			  kvm_pfn_t pfn, unsigned access)
 {
 	if (unlikely(is_noslot_pfn(pfn))) {
 		mark_mmio_spte(vcpu, sptep, gfn, access);
@@ -325,7 +325,7 @@ static int is_last_spte(u64 pte, int level)
 	return 0;
 }
 
-static pfn_t spte_to_pfn(u64 pte)
+static kvm_pfn_t spte_to_pfn(u64 pte)
 {
 	return (pte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT;
 }
@@ -587,7 +587,7 @@ static bool mmu_spte_update(u64 *sptep, u64 new_spte)
  */
 static int mmu_spte_clear_track_bits(u64 *sptep)
 {
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	u64 old_spte = *sptep;
 
 	if (!spte_has_volatile_bits(old_spte))
@@ -1369,7 +1369,7 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	int need_flush = 0;
 	u64 new_spte;
 	pte_t *ptep = (pte_t *)data;
-	pfn_t new_pfn;
+	kvm_pfn_t new_pfn;
 
 	WARN_ON(pte_huge(*ptep));
 	new_pfn = pte_pfn(*ptep);
@@ -2456,7 +2456,7 @@ static int mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn,
 	return 0;
 }
 
-static bool kvm_is_mmio_pfn(pfn_t pfn)
+static bool kvm_is_mmio_pfn(kvm_pfn_t pfn)
 {
 	if (pfn_valid(pfn))
 		return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn));
@@ -2466,7 +2466,7 @@ static bool kvm_is_mmio_pfn(pfn_t pfn)
 
 static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 		    unsigned pte_access, int level,
-		    gfn_t gfn, pfn_t pfn, bool speculative,
+		    gfn_t gfn, kvm_pfn_t pfn, bool speculative,
 		    bool can_unsync, bool host_writable)
 {
 	u64 spte;
@@ -2546,7 +2546,7 @@ done:
 
 static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 			 unsigned pte_access, int write_fault, int *emulate,
-			 int level, gfn_t gfn, pfn_t pfn, bool speculative,
+			 int level, gfn_t gfn, kvm_pfn_t pfn, bool speculative,
 			 bool host_writable)
 {
 	int was_rmapped = 0;
@@ -2606,7 +2606,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	kvm_release_pfn_clean(pfn);
 }
 
-static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
+static kvm_pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
 				     bool no_dirty_log)
 {
 	struct kvm_memory_slot *slot;
@@ -2689,7 +2689,7 @@ static void direct_pte_prefetch(struct kvm_vcpu *vcpu, u64 *sptep)
 }
 
 static int __direct_map(struct kvm_vcpu *vcpu, gpa_t v, int write,
-			int map_writable, int level, gfn_t gfn, pfn_t pfn,
+			int map_writable, int level, gfn_t gfn, kvm_pfn_t pfn,
 			bool prefault)
 {
 	struct kvm_shadow_walk_iterator iterator;
@@ -2739,7 +2739,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *
 	send_sig_info(SIGBUS, &info, tsk);
 }
 
-static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, pfn_t pfn)
+static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 {
 	/*
 	 * Do not cache the mmio info caused by writing the readonly gfn
@@ -2759,9 +2759,10 @@ static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, pfn_t pfn)
 }
 
 static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,
-					gfn_t *gfnp, pfn_t *pfnp, int *levelp)
+					gfn_t *gfnp, kvm_pfn_t *pfnp,
+					int *levelp)
 {
-	pfn_t pfn = *pfnp;
+	kvm_pfn_t pfn = *pfnp;
 	gfn_t gfn = *gfnp;
 	int level = *levelp;
 
@@ -2800,7 +2801,7 @@ static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,
 }
 
 static bool handle_abnormal_pfn(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,
-				pfn_t pfn, unsigned access, int *ret_val)
+				kvm_pfn_t pfn, unsigned access, int *ret_val)
 {
 	bool ret = true;
 
@@ -2954,7 +2955,7 @@ exit:
 }
 
 static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,
-			 gva_t gva, pfn_t *pfn, bool write, bool *writable);
+			 gva_t gva, kvm_pfn_t *pfn, bool write, bool *writable);
 static void make_mmu_pages_available(struct kvm_vcpu *vcpu);
 
 static int nonpaging_map(struct kvm_vcpu *vcpu, gva_t v, u32 error_code,
@@ -2963,7 +2964,7 @@ static int nonpaging_map(struct kvm_vcpu *vcpu, gva_t v, u32 error_code,
 	int r;
 	int level;
 	int force_pt_level;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	unsigned long mmu_seq;
 	bool map_writable, write = error_code & PFERR_WRITE_MASK;
 
@@ -3435,7 +3436,7 @@ static bool can_do_async_pf(struct kvm_vcpu *vcpu)
 }
 
 static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,
-			 gva_t gva, pfn_t *pfn, bool write, bool *writable)
+			 gva_t gva, kvm_pfn_t *pfn, bool write, bool *writable)
 {
 	struct kvm_memory_slot *slot;
 	bool async;
@@ -3473,7 +3474,7 @@ check_hugepage_cache_consistency(struct kvm_vcpu *vcpu, gfn_t gfn, int level)
 static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 			  bool prefault)
 {
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	int r;
 	int level;
 	int force_pt_level;
@@ -4627,7 +4628,7 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 	u64 *sptep;
 	struct rmap_iterator iter;
 	int need_tlb_flush = 0;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	struct kvm_mmu_page *sp;
 
 restart:
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 03d518e499a6..37a4d14115c0 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -97,7 +97,7 @@ static void audit_mappings(struct kvm_vcpu *vcpu, u64 *sptep, int level)
 {
 	struct kvm_mmu_page *sp;
 	gfn_t gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	hpa_t hpa;
 
 	sp = page_header(__pa(sptep));
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 736e6ab8784d..9dd02cb74724 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -456,7 +456,7 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 {
 	unsigned pte_access;
 	gfn_t gfn;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	if (FNAME(prefetch_invalid_gpte)(vcpu, sp, spte, gpte))
 		return false;
@@ -551,7 +551,7 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,
 static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
 			 struct guest_walker *gw,
 			 int write_fault, int hlevel,
-			 pfn_t pfn, bool map_writable, bool prefault)
+			 kvm_pfn_t pfn, bool map_writable, bool prefault)
 {
 	struct kvm_mmu_page *sp = NULL;
 	struct kvm_shadow_walk_iterator it;
@@ -696,7 +696,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 	int user_fault = error_code & PFERR_USER_MASK;
 	struct guest_walker walker;
 	int r;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 	int level = PT_PAGE_TABLE_LEVEL;
 	int force_pt_level;
 	unsigned long mmu_seq;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6a8bc64566ab..52aba0f1207b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4046,7 +4046,7 @@ out:
 static int init_rmode_identity_map(struct kvm *kvm)
 {
 	int i, idx, r = 0;
-	pfn_t identity_map_pfn;
+	kvm_pfn_t identity_map_pfn;
 	u32 tmp;
 
 	if (!enable_ept)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9a9a19830321..60b08427ca6c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4935,7 +4935,7 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t cr2,
 				  int emulation_type)
 {
 	gpa_t gpa = cr2;
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	if (emulation_type & EMULTYPE_NO_REEXECUTE)
 		return false;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1bef9e21e725..2420b43f3acc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -65,7 +65,7 @@
  * error pfns indicate that the gfn is in slot but faild to
  * translate it to pfn on host.
  */
-static inline bool is_error_pfn(pfn_t pfn)
+static inline bool is_error_pfn(kvm_pfn_t pfn)
 {
 	return !!(pfn & KVM_PFN_ERR_MASK);
 }
@@ -75,13 +75,13 @@ static inline bool is_error_pfn(pfn_t pfn)
  * translated to pfn - it is not in slot or failed to
  * translate it to pfn.
  */
-static inline bool is_error_noslot_pfn(pfn_t pfn)
+static inline bool is_error_noslot_pfn(kvm_pfn_t pfn)
 {
 	return !!(pfn & KVM_PFN_ERR_NOSLOT_MASK);
 }
 
 /* noslot pfn indicates that the gfn is not in slot. */
-static inline bool is_noslot_pfn(pfn_t pfn)
+static inline bool is_noslot_pfn(kvm_pfn_t pfn)
 {
 	return pfn == KVM_PFN_NOSLOT;
 }
@@ -569,19 +569,20 @@ void kvm_release_page_clean(struct page *page);
 void kvm_release_page_dirty(struct page *page);
 void kvm_set_page_accessed(struct page *page);
 
-pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn);
-pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn);
-pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
+kvm_pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn);
+kvm_pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn);
+kvm_pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
 		      bool *writable);
-pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn);
-pfn_t gfn_to_pfn_memslot_atomic(struct kvm_memory_slot *slot, gfn_t gfn);
-pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, bool atomic,
-			   bool *async, bool write_fault, bool *writable);
+kvm_pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn);
+kvm_pfn_t gfn_to_pfn_memslot_atomic(struct kvm_memory_slot *slot, gfn_t gfn);
+kvm_pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn,
+			       bool atomic, bool *async, bool write_fault,
+			       bool *writable);
 
-void kvm_release_pfn_clean(pfn_t pfn);
-void kvm_set_pfn_dirty(pfn_t pfn);
-void kvm_set_pfn_accessed(pfn_t pfn);
-void kvm_get_pfn(pfn_t pfn);
+void kvm_release_pfn_clean(kvm_pfn_t pfn);
+void kvm_set_pfn_dirty(kvm_pfn_t pfn);
+void kvm_set_pfn_accessed(kvm_pfn_t pfn);
+void kvm_get_pfn(kvm_pfn_t pfn);
 
 int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 			int len);
@@ -607,8 +608,8 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 struct kvm_memslots *kvm_vcpu_memslots(struct kvm_vcpu *vcpu);
 struct kvm_memory_slot *kvm_vcpu_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn);
-pfn_t kvm_vcpu_gfn_to_pfn_atomic(struct kvm_vcpu *vcpu, gfn_t gfn);
-pfn_t kvm_vcpu_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn);
+kvm_pfn_t kvm_vcpu_gfn_to_pfn_atomic(struct kvm_vcpu *vcpu, gfn_t gfn);
+kvm_pfn_t kvm_vcpu_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn);
 struct page *kvm_vcpu_gfn_to_page(struct kvm_vcpu *vcpu, gfn_t gfn);
 unsigned long kvm_vcpu_gfn_to_hva(struct kvm_vcpu *vcpu, gfn_t gfn);
 unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu *vcpu, gfn_t gfn, bool *writable);
@@ -789,7 +790,7 @@ void kvm_arch_sync_events(struct kvm *kvm);
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
 
-bool kvm_is_reserved_pfn(pfn_t pfn);
+bool kvm_is_reserved_pfn(kvm_pfn_t pfn);
 
 struct kvm_irq_ack_notifier {
 	struct hlist_node link;
@@ -940,7 +941,7 @@ static inline gfn_t gpa_to_gfn(gpa_t gpa)
 	return (gfn_t)(gpa >> PAGE_SHIFT);
 }
 
-static inline hpa_t pfn_to_hpa(pfn_t pfn)
+static inline hpa_t pfn_to_hpa(kvm_pfn_t pfn)
 {
 	return (hpa_t)pfn << PAGE_SHIFT;
 }
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 1b47a185c2f0..8bf259dae9f6 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -53,7 +53,7 @@ typedef unsigned long  hva_t;
 typedef u64            hpa_t;
 typedef u64            hfn_t;
 
-typedef hfn_t pfn_t;
+typedef hfn_t kvm_pfn_t;
 
 struct gfn_to_hva_cache {
 	u64 generation;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8db1d9361993..02cd2eddd3ff 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -111,7 +111,7 @@ static void hardware_disable_all(void);
 
 static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
 
-static void kvm_release_pfn_dirty(pfn_t pfn);
+static void kvm_release_pfn_dirty(kvm_pfn_t pfn);
 static void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot, gfn_t gfn);
 
 __visible bool kvm_rebooting;
@@ -119,7 +119,7 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
 
 static bool largepages_enabled = true;
 
-bool kvm_is_reserved_pfn(pfn_t pfn)
+bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
 	if (pfn_valid(pfn))
 		return PageReserved(pfn_to_page(pfn));
@@ -1296,7 +1296,7 @@ static inline int check_user_page_hwpoison(unsigned long addr)
  * true indicates success, otherwise false is returned.
  */
 static bool hva_to_pfn_fast(unsigned long addr, bool atomic, bool *async,
-			    bool write_fault, bool *writable, pfn_t *pfn)
+			    bool write_fault, bool *writable, kvm_pfn_t *pfn)
 {
 	struct page *page[1];
 	int npages;
@@ -1329,7 +1329,7 @@ static bool hva_to_pfn_fast(unsigned long addr, bool atomic, bool *async,
  * 1 indicates success, -errno is returned if error is detected.
  */
 static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault,
-			   bool *writable, pfn_t *pfn)
+			   bool *writable, kvm_pfn_t *pfn)
 {
 	struct page *page[1];
 	int npages = 0;
@@ -1393,11 +1393,11 @@ static bool vma_is_valid(struct vm_area_struct *vma, bool write_fault)
  * 2): @write_fault = false && @writable, @writable will tell the caller
  *     whether the mapping is writable.
  */
-static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
+static kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 			bool write_fault, bool *writable)
 {
 	struct vm_area_struct *vma;
-	pfn_t pfn = 0;
+	kvm_pfn_t pfn = 0;
 	int npages;
 
 	/* we can do it either atomically or asynchronously, not both */
@@ -1438,8 +1438,9 @@ exit:
 	return pfn;
 }
 
-pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, bool atomic,
-			   bool *async, bool write_fault, bool *writable)
+kvm_pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn,
+			       bool atomic, bool *async, bool write_fault,
+			       bool *writable)
 {
 	unsigned long addr = __gfn_to_hva_many(slot, gfn, NULL, write_fault);
 
@@ -1460,7 +1461,7 @@ pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, bool atomic,
 }
 EXPORT_SYMBOL_GPL(__gfn_to_pfn_memslot);
 
-pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
+kvm_pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
 		      bool *writable)
 {
 	return __gfn_to_pfn_memslot(gfn_to_memslot(kvm, gfn), gfn, false, NULL,
@@ -1468,37 +1469,37 @@ pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
 }
 EXPORT_SYMBOL_GPL(gfn_to_pfn_prot);
 
-pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn)
+kvm_pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn)
 {
 	return __gfn_to_pfn_memslot(slot, gfn, false, NULL, true, NULL);
 }
 EXPORT_SYMBOL_GPL(gfn_to_pfn_memslot);
 
-pfn_t gfn_to_pfn_memslot_atomic(struct kvm_memory_slot *slot, gfn_t gfn)
+kvm_pfn_t gfn_to_pfn_memslot_atomic(struct kvm_memory_slot *slot, gfn_t gfn)
 {
 	return __gfn_to_pfn_memslot(slot, gfn, true, NULL, true, NULL);
 }
 EXPORT_SYMBOL_GPL(gfn_to_pfn_memslot_atomic);
 
-pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn)
+kvm_pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn)
 {
 	return gfn_to_pfn_memslot_atomic(gfn_to_memslot(kvm, gfn), gfn);
 }
 EXPORT_SYMBOL_GPL(gfn_to_pfn_atomic);
 
-pfn_t kvm_vcpu_gfn_to_pfn_atomic(struct kvm_vcpu *vcpu, gfn_t gfn)
+kvm_pfn_t kvm_vcpu_gfn_to_pfn_atomic(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	return gfn_to_pfn_memslot_atomic(kvm_vcpu_gfn_to_memslot(vcpu, gfn), gfn);
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_gfn_to_pfn_atomic);
 
-pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn)
+kvm_pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn)
 {
 	return gfn_to_pfn_memslot(gfn_to_memslot(kvm, gfn), gfn);
 }
 EXPORT_SYMBOL_GPL(gfn_to_pfn);
 
-pfn_t kvm_vcpu_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn)
+kvm_pfn_t kvm_vcpu_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	return gfn_to_pfn_memslot(kvm_vcpu_gfn_to_memslot(vcpu, gfn), gfn);
 }
@@ -1521,7 +1522,7 @@ int gfn_to_page_many_atomic(struct kvm_memory_slot *slot, gfn_t gfn,
 }
 EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);
 
-static struct page *kvm_pfn_to_page(pfn_t pfn)
+static struct page *kvm_pfn_to_page(kvm_pfn_t pfn)
 {
 	if (is_error_noslot_pfn(pfn))
 		return KVM_ERR_PTR_BAD_PAGE;
@@ -1536,7 +1537,7 @@ static struct page *kvm_pfn_to_page(pfn_t pfn)
 
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
 {
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	pfn = gfn_to_pfn(kvm, gfn);
 
@@ -1546,7 +1547,7 @@ EXPORT_SYMBOL_GPL(gfn_to_page);
 
 struct page *kvm_vcpu_gfn_to_page(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
-	pfn_t pfn;
+	kvm_pfn_t pfn;
 
 	pfn = kvm_vcpu_gfn_to_pfn(vcpu, gfn);
 
@@ -1562,7 +1563,7 @@ void kvm_release_page_clean(struct page *page)
 }
 EXPORT_SYMBOL_GPL(kvm_release_page_clean);
 
-void kvm_release_pfn_clean(pfn_t pfn)
+void kvm_release_pfn_clean(kvm_pfn_t pfn)
 {
 	if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn))
 		put_page(pfn_to_page(pfn));
@@ -1577,13 +1578,13 @@ void kvm_release_page_dirty(struct page *page)
 }
 EXPORT_SYMBOL_GPL(kvm_release_page_dirty);
 
-static void kvm_release_pfn_dirty(pfn_t pfn)
+static void kvm_release_pfn_dirty(kvm_pfn_t pfn)
 {
 	kvm_set_pfn_dirty(pfn);
 	kvm_release_pfn_clean(pfn);
 }
 
-void kvm_set_pfn_dirty(pfn_t pfn)
+void kvm_set_pfn_dirty(kvm_pfn_t pfn)
 {
 	if (!kvm_is_reserved_pfn(pfn)) {
 		struct page *page = pfn_to_page(pfn);
@@ -1594,14 +1595,14 @@ void kvm_set_pfn_dirty(pfn_t pfn)
 }
 EXPORT_SYMBOL_GPL(kvm_set_pfn_dirty);
 
-void kvm_set_pfn_accessed(pfn_t pfn)
+void kvm_set_pfn_accessed(kvm_pfn_t pfn)
 {
 	if (!kvm_is_reserved_pfn(pfn))
 		mark_page_accessed(pfn_to_page(pfn));
 }
 EXPORT_SYMBOL_GPL(kvm_set_pfn_accessed);
 
-void kvm_get_pfn(pfn_t pfn)
+void kvm_get_pfn(kvm_pfn_t pfn)
 {
 	if (!kvm_is_reserved_pfn(pfn))
 		get_page(pfn_to_page(pfn));

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1260357 — [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qqf4u-Og-25@gated-at.bofh.it>
In reply to#1260353
Now that we have the ability to dynamically enable DAX for a raw block
inode, make the behavior opt-in by default.  DAX does not have feature
parity with pagecache backed mappings, so applications should knowingly
enable DAX semantics.

Note, this is only for mappings returned to userspace.  For the
synchronous usages of DAX, dax_do_io(), there is no semantic difference
with the bio submission path, so that path remains default enabled.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 block/ioctl.c      |    3 +--
 fs/block_dev.c     |   33 +++++++++++++++++++++++----------
 include/linux/fs.h |    8 ++++++++
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 205d57612fbd..c4c3a09d9ca9 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -298,13 +298,12 @@ static inline int is_unrecognized_ioctl(int ret)
 #ifdef CONFIG_FS_DAX
 static int blkdev_set_dax(struct block_device *bdev, int n)
 {
-	struct gendisk *disk = bdev->bd_disk;
 	int rc = 0;
 
 	if (n)
 		n = S_DAX;
 
-	if (n && !disk->fops->direct_access)
+	if (n && !blkdev_dax_capable(bdev))
 		return -ENOTTY;
 
 	mutex_lock(&bdev->bd_inode->i_mutex);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 13ce6d0ff7f6..ee34a31e6fa4 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -152,16 +152,37 @@ static struct inode *bdev_file_inode(struct file *file)
 	return file->f_mapping->host;
 }
 
+#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;
+
+	return true;
+}
+#endif
+
 static ssize_t
 blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = bdev_file_inode(file);
+	struct block_device *bdev = I_BDEV(inode);
 
-	if (IS_DAX(inode))
+	if (blkdev_dax_capable(bdev))
 		return dax_do_io(iocb, inode, iter, offset, blkdev_get_block,
 				NULL, DIO_SKIP_DIO_COUNT);
-	return __blockdev_direct_IO(iocb, inode, I_BDEV(inode), iter, offset,
+	return __blockdev_direct_IO(iocb, inode, bdev, iter, offset,
 				    blkdev_get_block, NULL, NULL,
 				    DIO_SKIP_DIO_COUNT);
 }
@@ -1185,7 +1206,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 		bdev->bd_disk = disk;
 		bdev->bd_queue = disk->queue;
 		bdev->bd_contains = bdev;
-		bdev->bd_inode->i_flags = disk->fops->direct_access ? S_DAX : 0;
 		if (!partno) {
 			ret = -ENXIO;
 			bdev->bd_part = disk_get_part(disk, partno);
@@ -1247,13 +1267,6 @@ 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 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)))
-				bdev->bd_inode->i_flags &= ~S_DAX;
 		}
 	} else {
 		if (bdev->bd_contains == bdev) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8fb2d4b848bf..5a9e14538f69 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2282,6 +2282,14 @@ 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;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261086 — Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDave Chinner <david@fromorbit.com>
Date2015-11-03 01:40 +0100
SubjectRe: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qqxNM-3Pp-3@gated-at.bofh.it>
In reply to#1260357
On Sun, Nov 01, 2015 at 11:30:53PM -0500, Dan Williams wrote:
> Now that we have the ability to dynamically enable DAX for a raw block
> inode, make the behavior opt-in by default.  DAX does not have feature
> parity with pagecache backed mappings, so applications should knowingly
> enable DAX semantics.
> 
> Note, this is only for mappings returned to userspace.  For the
> synchronous usages of DAX, dax_do_io(), there is no semantic difference
> with the bio submission path, so that path remains default enabled.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  block/ioctl.c      |    3 +--
>  fs/block_dev.c     |   33 +++++++++++++++++++++++----------
>  include/linux/fs.h |    8 ++++++++
>  3 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 205d57612fbd..c4c3a09d9ca9 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -298,13 +298,12 @@ static inline int is_unrecognized_ioctl(int ret)
>  #ifdef CONFIG_FS_DAX
>  static int blkdev_set_dax(struct block_device *bdev, int n)
>  {
> -	struct gendisk *disk = bdev->bd_disk;
>  	int rc = 0;
>  
>  	if (n)
>  		n = S_DAX;
>  
> -	if (n && !disk->fops->direct_access)
> +	if (n && !blkdev_dax_capable(bdev))
>  		return -ENOTTY;
>  
>  	mutex_lock(&bdev->bd_inode->i_mutex);
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 13ce6d0ff7f6..ee34a31e6fa4 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -152,16 +152,37 @@ static struct inode *bdev_file_inode(struct file *file)
>  	return file->f_mapping->host;
>  }
>  
> +#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;
> +
> +	return true;

Where do you check that S_DAX has been enabled on the block device
now?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261255 — Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-03 08:40 +0100
SubjectRe: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qqEmd-86s-7@gated-at.bofh.it>
In reply to#1261086
On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sun, Nov 01, 2015 at 11:30:53PM -0500, Dan Williams wrote:
>> Now that we have the ability to dynamically enable DAX for a raw block
>> inode, make the behavior opt-in by default.  DAX does not have feature
>> parity with pagecache backed mappings, so applications should knowingly
>> enable DAX semantics.
>>
>> Note, this is only for mappings returned to userspace.  For the
>> synchronous usages of DAX, dax_do_io(), there is no semantic difference
>> with the bio submission path, so that path remains default enabled.
>>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  block/ioctl.c      |    3 +--
>>  fs/block_dev.c     |   33 +++++++++++++++++++++++----------
>>  include/linux/fs.h |    8 ++++++++
>>  3 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index 205d57612fbd..c4c3a09d9ca9 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -298,13 +298,12 @@ static inline int is_unrecognized_ioctl(int ret)
>>  #ifdef CONFIG_FS_DAX
>>  static int blkdev_set_dax(struct block_device *bdev, int n)
>>  {
>> -     struct gendisk *disk = bdev->bd_disk;
>>       int rc = 0;
>>
>>       if (n)
>>               n = S_DAX;
>>
>> -     if (n && !disk->fops->direct_access)
>> +     if (n && !blkdev_dax_capable(bdev))
>>               return -ENOTTY;
>>
>>       mutex_lock(&bdev->bd_inode->i_mutex);
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 13ce6d0ff7f6..ee34a31e6fa4 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -152,16 +152,37 @@ static struct inode *bdev_file_inode(struct file *file)
>>       return file->f_mapping->host;
>>  }
>>
>> +#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;
>> +
>> +     return true;
>
> Where do you check that S_DAX has been enabled on the block device
> now?
>

Only in the mmap path:

static int blkdev_mmap(struct file *file, struct vm_area_struct *vma)
{
        struct inode *bd_inode = bdev_file_inode(file);
        struct block_device *bdev = I_BDEV(bd_inode);

        file_accessed(file);
        mutex_lock(&bd_inode->i_mutex);
        bdev->bd_map_count++;
        if (IS_DAX(bd_inode)) {
                vma->vm_ops = &blkdev_dax_vm_ops;
                vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
        } else {
                vma->vm_ops = &blkdev_default_vm_ops;
        }
        mutex_unlock(&bd_inode->i_mutex);

        return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261856 — Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDave Chinner <david@fromorbit.com>
Date2015-11-03 21:30 +0100
SubjectRe: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qqQno-7AN-25@gated-at.bofh.it>
In reply to#1261255
On Mon, Nov 02, 2015 at 11:35:04PM -0800, Dan Williams wrote:
> On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sun, Nov 01, 2015 at 11:30:53PM -0500, Dan Williams wrote:
> >> Now that we have the ability to dynamically enable DAX for a raw block
> >> inode, make the behavior opt-in by default.  DAX does not have feature
> >> parity with pagecache backed mappings, so applications should knowingly
> >> enable DAX semantics.
> >>
> >> Note, this is only for mappings returned to userspace.  For the
> >> synchronous usages of DAX, dax_do_io(), there is no semantic difference
> >> with the bio submission path, so that path remains default enabled.
> >>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> ---
> >>  block/ioctl.c      |    3 +--
> >>  fs/block_dev.c     |   33 +++++++++++++++++++++++----------
> >>  include/linux/fs.h |    8 ++++++++
> >>  3 files changed, 32 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/block/ioctl.c b/block/ioctl.c
> >> index 205d57612fbd..c4c3a09d9ca9 100644
> >> --- a/block/ioctl.c
> >> +++ b/block/ioctl.c
> >> @@ -298,13 +298,12 @@ static inline int is_unrecognized_ioctl(int ret)
> >>  #ifdef CONFIG_FS_DAX
> >>  static int blkdev_set_dax(struct block_device *bdev, int n)
> >>  {
> >> -     struct gendisk *disk = bdev->bd_disk;
> >>       int rc = 0;
> >>
> >>       if (n)
> >>               n = S_DAX;
> >>
> >> -     if (n && !disk->fops->direct_access)
> >> +     if (n && !blkdev_dax_capable(bdev))
> >>               return -ENOTTY;
> >>
> >>       mutex_lock(&bdev->bd_inode->i_mutex);
> >> diff --git a/fs/block_dev.c b/fs/block_dev.c
> >> index 13ce6d0ff7f6..ee34a31e6fa4 100644
> >> --- a/fs/block_dev.c
> >> +++ b/fs/block_dev.c
> >> @@ -152,16 +152,37 @@ static struct inode *bdev_file_inode(struct file *file)
> >>       return file->f_mapping->host;
> >>  }
> >>
> >> +#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;
> >> +
> >> +     return true;
> >
> > Where do you check that S_DAX has been enabled on the block device
> > now?
> >
> 
> Only in the mmap path:

which means blkdev_direct_IO() is now always going to go down the
dax_do_io() path for any driver with a ->direct_access method rather
than the direct IO path, regardless of whether DAX is enabled on the
device or not.

That really seems wrong to me - you've replace explicit "is DAX
enabled" checks with "is DAX possible" checks, and so DAX paths are
used regardless of whether DAX is enabled or not. And it's not
obvious why this is done, nor is it now obvious how DAX interacts
with the block device.

This really seems like a step backwards to me.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261945 — Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-04 00:10 +0100
SubjectRe: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qqSSe-O3-5@gated-at.bofh.it>
In reply to#1261856
On Tue, Nov 3, 2015 at 12:20 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Nov 02, 2015 at 11:35:04PM -0800, Dan Williams wrote:
>> On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner <david@fromorbit.com> wrote:
[..]
>> Only in the mmap path:
>
> which means blkdev_direct_IO() is now always going to go down the
> dax_do_io() path for any driver with a ->direct_access method rather
> than the direct IO path, regardless of whether DAX is enabled on the
> device or not.
>
> That really seems wrong to me - you've replace explicit "is DAX
> enabled" checks with "is DAX possible" checks, and so DAX paths are
> used regardless of whether DAX is enabled or not. And it's not
> obvious why this is done, nor is it now obvious how DAX interacts
> with the block device.
>
> This really seems like a step backwards to me.

I think the reason it is not obvious is the original justification for
the bypass as stated in commit bbab37ddc20b "block: Add support for
DAX reads/writes to block devices" was:

    "instead of allocating a DIO and a BIO"

It turns out it's faster and as far as I can tell semantically
equivalent to the __blockdev_direct_IO() path.  The DAX mmap path in
comparison has plenty of sharp edges and semantic differences that
would be avoided by turning off DAX.

I'm not opposed to also turning off dax_do_io() when S_DAX is clear,
but I don't currently see the point.  At the very least I need to add
the above comments to the code, but do you still think opt-in DAX is a
backwards step?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1262534 — Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-04 20:30 +0100
SubjectRe: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default
Message-ID<qrbUR-4Bi-13@gated-at.bofh.it>
In reply to#1261945
On Tue, Nov 3, 2015 at 3:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Nov 3, 2015 at 12:20 PM, Dave Chinner <david@fromorbit.com> wrote:
>> On Mon, Nov 02, 2015 at 11:35:04PM -0800, Dan Williams wrote:
>>> On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner <david@fromorbit.com> wrote:
> [..]
>>> Only in the mmap path:
>>
>> which means blkdev_direct_IO() is now always going to go down the
>> dax_do_io() path for any driver with a ->direct_access method rather
>> than the direct IO path, regardless of whether DAX is enabled on the
>> device or not.
>>
>> That really seems wrong to me - you've replace explicit "is DAX
>> enabled" checks with "is DAX possible" checks, and so DAX paths are
>> used regardless of whether DAX is enabled or not. And it's not
>> obvious why this is done, nor is it now obvious how DAX interacts
>> with the block device.
>>
>> This really seems like a step backwards to me.
>
> I think the reason it is not obvious is the original justification for
> the bypass as stated in commit bbab37ddc20b "block: Add support for
> DAX reads/writes to block devices" was:
>
>     "instead of allocating a DIO and a BIO"
>
> It turns out it's faster and as far as I can tell semantically
> equivalent to the __blockdev_direct_IO() path.  The DAX mmap path in
> comparison has plenty of sharp edges and semantic differences that
> would be avoided by turning off DAX.
>
> I'm not opposed to also turning off dax_do_io() when S_DAX is clear,
> but I don't currently see the point.  At the very least I need to add
> the above comments to the code, but do you still think opt-in DAX is a
> backwards step?

I thought of one way dax_do_io() breaks current semantics, it defeats
blktrace and i/o stat tracking.  I'll restore the existing behavior
that gates dax_do_io() on S_DAX.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1260358 — [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()
Message-ID<qqf4u-Og-23@gated-at.bofh.it>
In reply to#1260353
This masking prevents access to the end of the device via dax_do_io(),
and is unnecessary as arch_add_memory() would have rejected an unaligned
allocation.

Cc: <stable@vger.kernel.org>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/pmem.c |   17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index e46988fbdee5..93472953e231 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -100,26 +100,15 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
 }
 
 static long pmem_direct_access(struct block_device *bdev, sector_t sector,
-		      void __pmem **kaddr, unsigned long *pfn)
+		      void __pmem **kaddr, pfn_t *pfn)
 {
 	struct pmem_device *pmem = bdev->bd_disk->private_data;
 	resource_size_t offset = sector * 512 + pmem->data_offset;
-	resource_size_t size;
 
-	if (pmem->data_offset) {
-		/*
-		 * Limit the direct_access() size to what is covered by
-		 * the memmap
-		 */
-		size = (pmem->size - offset) & ~ND_PFN_MASK;
-	} else
-		size = pmem->size - offset;
-
-	/* FIXME convert DAX to comprehend that this mapping has a lifetime */
 	*kaddr = pmem->virt_addr + offset;
-	*pfn = (pmem->phys_addr + offset) >> PAGE_SHIFT;
+	*pfn = __phys_to_pfn(pmem->phys_addr + offset, pmem->pfn_flags);
 
-	return size;
+	return pmem->size - offset;
 }
 
 static const struct block_device_operations pmem_fops = {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261815 — Re: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2015-11-03 20:40 +0100
SubjectRe: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()
Message-ID<qqPB0-72R-15@gated-at.bofh.it>
In reply to#1260358
On Sun, Nov 01, 2015 at 11:30:10PM -0500, Dan Williams wrote:
> This masking prevents access to the end of the device via dax_do_io(),
> and is unnecessary as arch_add_memory() would have rejected an unaligned
> allocation.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/nvdimm/pmem.c |   17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index e46988fbdee5..93472953e231 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -100,26 +100,15 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
>  }
>  
>  static long pmem_direct_access(struct block_device *bdev, sector_t sector,
> -		      void __pmem **kaddr, unsigned long *pfn)
> +		      void __pmem **kaddr, pfn_t *pfn)

It seems kind of weird to change only this instance of direct_access() to have
the last argument as a pfn_t instead of an unsigned long?   If pfn_t is more
descriptive (I think it is) should we update the definition in struct
block_device_operations and all the other implementors of direct_access as
well?  If that's touching too much, let's do them all together later, but
let's not change one now and have them be inconsistent.

>  {
>  	struct pmem_device *pmem = bdev->bd_disk->private_data;
>  	resource_size_t offset = sector * 512 + pmem->data_offset;
> -	resource_size_t size;
>  
> -	if (pmem->data_offset) {
> -		/*
> -		 * Limit the direct_access() size to what is covered by
> -		 * the memmap
> -		 */
> -		size = (pmem->size - offset) & ~ND_PFN_MASK;
> -	} else
> -		size = pmem->size - offset;
> -
> -	/* FIXME convert DAX to comprehend that this mapping has a lifetime */
>  	*kaddr = pmem->virt_addr + offset;
> -	*pfn = (pmem->phys_addr + offset) >> PAGE_SHIFT;
> +	*pfn = __phys_to_pfn(pmem->phys_addr + offset, pmem->pfn_flags);

__phys_to_pfn() only takes a single argument (the paddr) in v4.3,
jens/for-4.4/integrity and in nvdimm/libnvdimm-for-next.  Is this second
argument of pfn_flags actually correct?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261911 — Re: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-03 22:40 +0100
SubjectRe: [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access()
Message-ID<qqRt8-8fd-19@gated-at.bofh.it>
In reply to#1261815
On Tue, Nov 3, 2015 at 11:32 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Sun, Nov 01, 2015 at 11:30:10PM -0500, Dan Williams wrote:
>> This masking prevents access to the end of the device via dax_do_io(),
>> and is unnecessary as arch_add_memory() would have rejected an unaligned
>> allocation.
>>
>> Cc: <stable@vger.kernel.org>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  drivers/nvdimm/pmem.c |   17 +++--------------
>>  1 file changed, 3 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
>> index e46988fbdee5..93472953e231 100644
>> --- a/drivers/nvdimm/pmem.c
>> +++ b/drivers/nvdimm/pmem.c
>> @@ -100,26 +100,15 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
>>  }
>>
>>  static long pmem_direct_access(struct block_device *bdev, sector_t sector,
>> -                   void __pmem **kaddr, unsigned long *pfn)
>> +                   void __pmem **kaddr, pfn_t *pfn)
>
> It seems kind of weird to change only this instance of direct_access() to have
> the last argument as a pfn_t instead of an unsigned long?   If pfn_t is more
> descriptive (I think it is) should we update the definition in struct
> block_device_operations and all the other implementors of direct_access as
> well?  If that's touching too much, let's do them all together later, but
> let's not change one now and have them be inconsistent.
>

Oh, nice catch, that's just a mistake when I moved this patch earlier
in the series... and I wonder why 0day didn't complain about it?  In
any event, will fix.

>>  {
>>       struct pmem_device *pmem = bdev->bd_disk->private_data;
>>       resource_size_t offset = sector * 512 + pmem->data_offset;
>> -     resource_size_t size;
>>
>> -     if (pmem->data_offset) {
>> -             /*
>> -              * Limit the direct_access() size to what is covered by
>> -              * the memmap
>> -              */
>> -             size = (pmem->size - offset) & ~ND_PFN_MASK;
>> -     } else
>> -             size = pmem->size - offset;
>> -
>> -     /* FIXME convert DAX to comprehend that this mapping has a lifetime */
>>       *kaddr = pmem->virt_addr + offset;
>> -     *pfn = (pmem->phys_addr + offset) >> PAGE_SHIFT;
>> +     *pfn = __phys_to_pfn(pmem->phys_addr + offset, pmem->pfn_flags);
>
> __phys_to_pfn() only takes a single argument (the paddr) in v4.3,
> jens/for-4.4/integrity and in nvdimm/libnvdimm-for-next.  Is this second
> argument of pfn_flags actually correct?

Yeah, this shouldn't even compile.

Thanks Ross!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1260359 — [PATCH v3 14/15] dax: dirty extent notification

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-02 05:40 +0100
Subject[PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqf4u-Og-27@gated-at.bofh.it>
In reply to#1260353
DAX-enabled block device drivers can use hints from fs/dax.c to
optimize their internal tracking of potentially dirty cpu cache lines.
If a DAX mapping is being used for synchronous operations, dax_do_io(),
a dax-enabled block-driver knows that fs/dax.c will handle immediate
flushing.  For asynchronous mappings, i.e.  returned to userspace via
mmap, the driver can track active extents of the media for flushing.

We can later extend the DAX paths to indicate when an async mapping is
"closed" allowing the active extents to be marked clean.

Because this capability requires adding two new parameters to
->direct_access ('size' and 'flags') convert the function to a control
parameter block.  As a result this cleans up dax_map_atomic() usage as
there is no longer a need to have a separate __dax_map_atomic, and the
return value can match bdev_direct_access().

No functional change results from this patch, just code movement to the
new parameter scheme.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/powerpc/sysdev/axonram.c |    9 +-
 drivers/block/brd.c           |   10 +-
 drivers/nvdimm/pmem.c         |   10 +-
 drivers/s390/block/dcssblk.c  |    9 +-
 fs/block_dev.c                |   17 ++--
 fs/dax.c                      |  167 ++++++++++++++++++++++-------------------
 include/linux/blkdev.h        |   24 +++++-
 7 files changed, 136 insertions(+), 110 deletions(-)

diff --git a/arch/powerpc/sysdev/axonram.c b/arch/powerpc/sysdev/axonram.c
index 59ca4c0ab529..11aeb47a6540 100644
--- a/arch/powerpc/sysdev/axonram.c
+++ b/arch/powerpc/sysdev/axonram.c
@@ -140,14 +140,13 @@ axon_ram_make_request(struct request_queue *queue, struct bio *bio)
  * @device, @sector, @data: see block_device_operations method
  */
 static long
-axon_ram_direct_access(struct block_device *device, sector_t sector,
-		       void __pmem **kaddr, pfn_t *pfn)
+axon_ram_direct_access(struct block_device *device, struct blk_dax_ctl *dax)
 {
 	struct axon_ram_bank *bank = device->bd_disk->private_data;
-	loff_t offset = (loff_t)sector << AXON_RAM_SECTOR_SHIFT;
+	loff_t offset = (loff_t)dax->sector << AXON_RAM_SECTOR_SHIFT;
 
-	*kaddr = (void __pmem __force *) bank->io_addr + offset;
-	*pfn = phys_to_pfn_t(bank->ph_addr + offset, PFN_DEV);
+	dax->addr = (void __pmem __force *) bank->io_addr + offset;
+	dax->pfn = phys_to_pfn_t(bank->ph_addr + offset, PFN_DEV);
 	return bank->size - offset;
 }
 
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 0bbc60463779..686e1e7a5973 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -373,19 +373,19 @@ static int brd_rw_page(struct block_device *bdev, sector_t sector,
 }
 
 #ifdef CONFIG_BLK_DEV_RAM_DAX
-static long brd_direct_access(struct block_device *bdev, sector_t sector,
-			void __pmem **kaddr, pfn_t *pfn)
+static long brd_direct_access(struct block_device *bdev,
+		struct blk_dax_ctl *dax)
 {
 	struct brd_device *brd = bdev->bd_disk->private_data;
 	struct page *page;
 
 	if (!brd)
 		return -ENODEV;
-	page = brd_insert_page(brd, sector);
+	page = brd_insert_page(brd, dax->sector);
 	if (!page)
 		return -ENOSPC;
-	*kaddr = (void __pmem *)page_address(page);
-	*pfn = page_to_pfn_t(page);
+	dax->addr = (void __pmem *)page_address(page);
+	dax->pfn = page_to_pfn_t(page);
 
 	return PAGE_SIZE;
 }
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index aa2f1292120a..3d83f3079602 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -103,14 +103,14 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
 	return 0;
 }
 
-static long pmem_direct_access(struct block_device *bdev, sector_t sector,
-		      void __pmem **kaddr, pfn_t *pfn)
+static long pmem_direct_access(struct block_device *bdev,
+		struct blk_dax_ctl *dax)
 {
 	struct pmem_device *pmem = bdev->bd_disk->private_data;
-	resource_size_t offset = sector * 512 + pmem->data_offset;
+	resource_size_t offset = dax->sector * 512 + pmem->data_offset;
 
-	*kaddr = pmem->virt_addr + offset;
-	*pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
+	dax->addr = pmem->virt_addr + offset;
+	dax->pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
 
 	return pmem->size - offset;
 }
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index e2b2839e4de5..6b01f56373e0 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -880,8 +880,7 @@ fail:
 }
 
 static long
-dcssblk_direct_access (struct block_device *bdev, sector_t secnum,
-			void __pmem **kaddr, pfn_t *pfn)
+dcssblk_direct_access (struct block_device *bdev, struct blk_dax_ctl *dax)
 {
 	struct dcssblk_dev_info *dev_info;
 	unsigned long offset, dev_sz;
@@ -890,9 +889,9 @@ dcssblk_direct_access (struct block_device *bdev, sector_t secnum,
 	if (!dev_info)
 		return -ENODEV;
 	dev_sz = dev_info->end - dev_info->start;
-	offset = secnum * 512;
-	*kaddr = (void __pmem *) (dev_info->start + offset);
-	*pfn = phys_to_pfn_t(dev_info->start + offset, PFN_DEV);
+	offset = dax->sector * 512;
+	dax->addr = (void __pmem *) (dev_info->start + offset);
+	dax->pfn = phys_to_pfn_t(dev_info->start + offset, PFN_DEV);
 
 	return dev_sz - offset;
 }
diff --git a/fs/block_dev.c b/fs/block_dev.c
index ee34a31e6fa4..d1b0bbf00bd3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -453,10 +453,7 @@ EXPORT_SYMBOL_GPL(bdev_write_page);
 /**
  * bdev_direct_access() - Get the address for directly-accessibly memory
  * @bdev: The device containing the memory
- * @sector: The offset within the device
- * @addr: Where to put the address of the memory
- * @pfn: The Page Frame Number for the memory
- * @size: The number of bytes requested
+ * @ctl: control and output parameters for ->direct_access
  *
  * If a block device is made up of directly addressable memory, this function
  * will tell the caller the PFN and the address of the memory.  The address
@@ -467,10 +464,10 @@ EXPORT_SYMBOL_GPL(bdev_write_page);
  * Return: negative errno if an error occurs, otherwise the number of bytes
  * accessible at this address.
  */
-long bdev_direct_access(struct block_device *bdev, sector_t sector,
-			void __pmem **addr, pfn_t *pfn, long size)
+long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *ctl)
 {
-	long avail;
+	sector_t sector, save;
+	long avail, size = ctl->size;
 	const struct block_device_operations *ops = bdev->bd_disk->fops;
 
 	/*
@@ -479,6 +476,8 @@ long bdev_direct_access(struct block_device *bdev, sector_t sector,
 	 */
 	might_sleep();
 
+	save = ctl->sector;
+	sector = ctl->sector;
 	if (size < 0)
 		return size;
 	if (!ops->direct_access)
@@ -489,7 +488,9 @@ long bdev_direct_access(struct block_device *bdev, sector_t sector,
 	sector += get_start_sect(bdev);
 	if (sector % (PAGE_SIZE / 512))
 		return -EINVAL;
-	avail = ops->direct_access(bdev, sector, addr, pfn);
+	ctl->sector = sector;
+	avail = ops->direct_access(bdev, ctl);
+	ctl->sector = save;
 	if (!avail)
 		return -ERANGE;
 	return min(avail, size);
diff --git a/fs/dax.c b/fs/dax.c
index ac8992e86779..f5835c4a7e1f 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -30,36 +30,28 @@
 #include <linux/vmstat.h>
 #include <linux/sizes.h>
 
-static void __pmem *__dax_map_atomic(struct block_device *bdev, sector_t sector,
-		long size, pfn_t *pfn, long *len)
+static long dax_map_atomic(struct block_device *bdev, struct blk_dax_ctl *dax)
 {
-	long rc;
-	void __pmem *addr;
 	struct request_queue *q = bdev->bd_queue;
+	long rc = -EIO;
 
+	dax->addr = (void __pmem *) ERR_PTR(-EIO);
 	if (blk_queue_enter(q, GFP_NOWAIT) != 0)
-		return (void __pmem *) ERR_PTR(-EIO);
-	rc = bdev_direct_access(bdev, sector, &addr, pfn, size);
-	if (len)
-		*len = rc;
+		return rc;
+
+	rc = bdev_direct_access(bdev, dax);
 	if (rc < 0) {
+		dax->addr = (void __pmem *) ERR_PTR(rc);
 		blk_queue_exit(q);
-		return (void __pmem *) ERR_PTR(rc);
+		return rc;
 	}
-	return addr;
-}
-
-static void __pmem *dax_map_atomic(struct block_device *bdev, sector_t sector,
-		long size)
-{
-	pfn_t pfn;
-
-	return __dax_map_atomic(bdev, sector, size, &pfn, NULL);
+	return rc;
 }
 
-static void dax_unmap_atomic(struct block_device *bdev, void __pmem *addr)
+static void dax_unmap_atomic(struct block_device *bdev,
+		const struct blk_dax_ctl *dax)
 {
-	if (IS_ERR(addr))
+	if (IS_ERR(dax->addr))
 		return;
 	blk_queue_exit(bdev->bd_queue);
 }
@@ -67,28 +59,29 @@ static void dax_unmap_atomic(struct block_device *bdev, void __pmem *addr)
 int dax_clear_blocks(struct inode *inode, sector_t block, long size)
 {
 	struct block_device *bdev = inode->i_sb->s_bdev;
-	sector_t sector = block << (inode->i_blkbits - 9);
+	struct blk_dax_ctl dax;
 
 	might_sleep();
+	dax.sector = block << (inode->i_blkbits - 9),
+	dax.flags = 0;
+	dax.size = size;
 	do {
-		void __pmem *addr;
 		long count, sz;
-		pfn_t pfn;
 
 		sz = min_t(long, size, SZ_1M);
-		addr = __dax_map_atomic(bdev, sector, size, &pfn, &count);
-		if (IS_ERR(addr))
-			return PTR_ERR(addr);
+		count = dax_map_atomic(bdev, &dax);
+		if (count < 0)
+			return count;
 		if (count < sz)
 			sz = count;
-		clear_pmem(addr, sz);
-		addr += sz;
-		size -= sz;
+		clear_pmem(dax.addr, sz);
+		dax_unmap_atomic(bdev, &dax);
+		dax.addr += sz;
+		dax.size -= sz;
 		BUG_ON(sz & 511);
-		sector += sz / 512;
-		dax_unmap_atomic(bdev, addr);
+		dax.sector += sz / 512;
 		cond_resched();
-	} while (size);
+	} while (dax.size);
 
 	wmb_pmem();
 	return 0;
@@ -141,9 +134,11 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 	struct block_device *bdev = NULL;
 	int rw = iov_iter_rw(iter), rc;
 	long map_len = 0;
-	pfn_t pfn;
 	void __pmem *addr = NULL;
-	void __pmem *kmap = (void __pmem *) ERR_PTR(-EIO);
+	struct blk_dax_ctl dax = {
+		.addr = (void __pmem *) ERR_PTR(-EIO),
+		.flags = 0,
+	};
 	bool hole = false;
 	bool need_wmb = false;
 
@@ -181,15 +176,15 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 				addr = NULL;
 				size = bh->b_size - first;
 			} else {
-				dax_unmap_atomic(bdev, kmap);
-				kmap = __dax_map_atomic(bdev,
-						to_sector(bh, inode),
-						bh->b_size, &pfn, &map_len);
-				if (IS_ERR(kmap)) {
-					rc = PTR_ERR(kmap);
+				dax_unmap_atomic(bdev, &dax);
+				dax.sector = to_sector(bh, inode);
+				dax.size = bh->b_size;
+				map_len = dax_map_atomic(bdev, &dax);
+				if (map_len < 0) {
+					rc = map_len;
 					break;
 				}
-				addr = kmap;
+				addr = dax.addr;
 				if (buffer_unwritten(bh) || buffer_new(bh)) {
 					dax_new_buf(addr, map_len, first, pos,
 							end);
@@ -219,7 +214,7 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 
 	if (need_wmb)
 		wmb_pmem();
-	dax_unmap_atomic(bdev, kmap);
+	dax_unmap_atomic(bdev, &dax);
 
 	return (pos == start) ? rc : pos - start;
 }
@@ -313,17 +308,20 @@ static int dax_load_hole(struct address_space *mapping, struct page *page,
 static int copy_user_bh(struct page *to, struct inode *inode,
 		struct buffer_head *bh, unsigned long vaddr)
 {
+	struct blk_dax_ctl dax = {
+		.sector = to_sector(bh, inode),
+		.size = bh->b_size,
+		.flags = 0,
+	};
 	struct block_device *bdev = bh->b_bdev;
-	void __pmem *vfrom;
 	void *vto;
 
-	vfrom = dax_map_atomic(bdev, to_sector(bh, inode), bh->b_size);
-	if (IS_ERR(vfrom))
-		return PTR_ERR(vfrom);
+	if (dax_map_atomic(bdev, &dax) < 0)
+		return PTR_ERR(dax.addr);
 	vto = kmap_atomic(to);
-	copy_user_page(vto, (void __force *)vfrom, vaddr, to);
+	copy_user_page(vto, (void __force *)dax.addr, vaddr, to);
 	kunmap_atomic(vto);
-	dax_unmap_atomic(bdev, vfrom);
+	dax_unmap_atomic(bdev, &dax);
 	return 0;
 }
 
@@ -344,15 +342,25 @@ static void dax_account_mapping(struct block_device *bdev, pfn_t pfn,
 	}
 }
 
+static unsigned long vm_fault_to_dax_flags(struct vm_fault *vmf)
+{
+	if (vmf->flags & (FAULT_FLAG_WRITE | FAULT_FLAG_MKWRITE))
+		return BLKDAX_F_DIRTY;
+	return 0;
+}
+
 static int dax_insert_mapping(struct inode *inode, struct buffer_head *bh,
 			struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	unsigned long vaddr = (unsigned long)vmf->virtual_address;
 	struct address_space *mapping = inode->i_mapping;
 	struct block_device *bdev = bh->b_bdev;
-	void __pmem *addr;
+	struct blk_dax_ctl dax = {
+		.sector = to_sector(bh, inode),
+		.size = bh->b_size,
+		.flags = vm_fault_to_dax_flags(vmf),
+	};
 	pgoff_t size;
-	pfn_t pfn;
 	int error;
 
 	i_mmap_lock_read(mapping);
@@ -370,22 +378,20 @@ static int dax_insert_mapping(struct inode *inode, struct buffer_head *bh,
 		goto out;
 	}
 
-	addr = __dax_map_atomic(bdev, to_sector(bh, inode), bh->b_size,
-			&pfn, NULL);
-	if (IS_ERR(addr)) {
-		error = PTR_ERR(addr);
+	if (dax_map_atomic(bdev, &dax) < 0) {
+		error = PTR_ERR(dax.addr);
 		goto out;
 	}
 
 	if (buffer_unwritten(bh) || buffer_new(bh)) {
-		clear_pmem(addr, PAGE_SIZE);
+		clear_pmem(dax.addr, PAGE_SIZE);
 		wmb_pmem();
 	}
 
-	dax_account_mapping(bdev, pfn, mapping);
-	dax_unmap_atomic(bdev, addr);
+	dax_account_mapping(bdev, dax.pfn, mapping);
+	dax_unmap_atomic(bdev, &dax);
 
-	error = vm_insert_mixed(vma, vaddr, pfn_t_to_pfn(pfn));
+	error = vm_insert_mixed(vma, vaddr, pfn_t_to_pfn(dax.pfn));
 
  out:
 	i_mmap_unlock_read(mapping);
@@ -674,33 +680,35 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
 		result = VM_FAULT_NOPAGE;
 		spin_unlock(ptl);
 	} else {
-		pfn_t pfn;
-		long length;
-		void __pmem *kaddr = __dax_map_atomic(bdev,
-				to_sector(&bh, inode), HPAGE_SIZE, &pfn,
-				&length);
-
-		if (IS_ERR(kaddr)) {
+		struct blk_dax_ctl dax = {
+			.sector = to_sector(&bh, inode),
+			.size = HPAGE_SIZE,
+			.flags = flags,
+		};
+		long length = dax_map_atomic(bdev, &dax);
+
+		if (length < 0) {
 			result = VM_FAULT_SIGBUS;
 			goto out;
 		}
-		if ((length < PMD_SIZE) || (pfn_t_to_pfn(pfn) & PG_PMD_COLOUR)) {
-			dax_unmap_atomic(bdev, kaddr);
+		if ((length < HPAGE_SIZE)
+				|| (pfn_t_to_pfn(dax.pfn) & PG_PMD_COLOUR)) {
+			dax_unmap_atomic(bdev, &dax);
 			goto fallback;
 		}
 
 		if (buffer_unwritten(&bh) || buffer_new(&bh)) {
-			clear_pmem(kaddr, HPAGE_SIZE);
+			clear_pmem(dax.addr, HPAGE_SIZE);
 			wmb_pmem();
 			count_vm_event(PGMAJFAULT);
 			mem_cgroup_count_vm_event(vma->vm_mm, PGMAJFAULT);
 			result |= VM_FAULT_MAJOR;
 		}
-		dax_account_mapping(bdev, pfn, mapping);
-		dax_unmap_atomic(bdev, kaddr);
+		dax_account_mapping(bdev, dax.pfn, mapping);
+		dax_unmap_atomic(bdev, &dax);
 
 		result |= vmf_insert_pfn_pmd(vma, address, pmd,
-				pfn_t_to_pfn(pfn), write);
+				pfn_t_to_pfn(dax.pfn), write);
 	}
 
  out:
@@ -803,14 +811,17 @@ int dax_zero_page_range(struct inode *inode, loff_t from, unsigned length,
 		return err;
 	if (buffer_written(&bh)) {
 		struct block_device *bdev = bh.b_bdev;
-		void __pmem *addr = dax_map_atomic(bdev, to_sector(&bh, inode),
-				PAGE_CACHE_SIZE);
-
-		if (IS_ERR(addr))
-			return PTR_ERR(addr);
-		clear_pmem(addr + offset, length);
+		struct blk_dax_ctl dax = {
+			.sector = to_sector(&bh, inode),
+			.size = PAGE_CACHE_SIZE,
+			.flags = 0,
+		};
+
+		if (dax_map_atomic(bdev, &dax) < 0)
+			return PTR_ERR(dax.addr);
+		clear_pmem(dax.addr + offset, length);
 		wmb_pmem();
-		dax_unmap_atomic(bdev, addr);
+		dax_unmap_atomic(bdev, &dax);
 	}
 
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e121e5e0c6ac..663e9974820f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1615,14 +1615,31 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
 
 #endif /* CONFIG_BLK_DEV_INTEGRITY */
 
+#define BLKDAX_F_DIRTY (1UL << 0) /* range is mapped writable to userspace */
+
+/**
+ * struct blk_dax_ctl - control and output parameters for ->direct_access
+ * @sector: (input) offset relative to a block_device
+ * @addr: (output) kernel virtual address for @sector populated by driver
+ * @flags: (input) BLKDAX_F_*
+ * @pfn: (output) page frame number for @addr populated by driver
+ * @size: (input) number of bytes requested
+ */
+struct blk_dax_ctl {
+	sector_t sector;
+	void __pmem *addr;
+	unsigned long flags;
+	long size;
+	pfn_t pfn;
+};
+
 struct block_device_operations {
 	int (*open) (struct block_device *, fmode_t);
 	void (*release) (struct gendisk *, fmode_t);
 	int (*rw_page)(struct block_device *, sector_t, struct page *, int rw);
 	int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
 	int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
-	long (*direct_access)(struct block_device *, sector_t, void __pmem **,
-			pfn_t *);
+	long (*direct_access)(struct block_device *, struct blk_dax_ctl *);
 	unsigned int (*check_events) (struct gendisk *disk,
 				      unsigned int clearing);
 	/* ->media_changed() is DEPRECATED, use ->check_events() instead */
@@ -1640,8 +1657,7 @@ extern int __blkdev_driver_ioctl(struct block_device *, fmode_t, unsigned int,
 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 *, sector_t,
-		void __pmem **addr, pfn_t *pfn, long size);
+extern long bdev_direct_access(struct block_device *, struct blk_dax_ctl *);
 #else /* CONFIG_BLOCK */
 
 struct block_device;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261109 — Re: [PATCH v3 14/15] dax: dirty extent notification

FromDave Chinner <david@fromorbit.com>
Date2015-11-03 02:20 +0100
SubjectRe: [PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqyqu-4js-15@gated-at.bofh.it>
In reply to#1260359
On Sun, Nov 01, 2015 at 11:30:58PM -0500, Dan Williams wrote:
> DAX-enabled block device drivers can use hints from fs/dax.c to
> optimize their internal tracking of potentially dirty cpu cache lines.
> If a DAX mapping is being used for synchronous operations, dax_do_io(),
> a dax-enabled block-driver knows that fs/dax.c will handle immediate
> flushing.  For asynchronous mappings, i.e.  returned to userspace via
> mmap, the driver can track active extents of the media for flushing.

So, essentially, you are marking the calls into the mapping calls
with BLKDAX_F_DIRTY when the mapping is requested for a write page
fault?  Hence allowing the block device to track "dirty pages"
exactly?

But, really, if we're going to use Ross's mapping tree patches that
use exceptional entries to track dirty pfns, why do we need to this
special interface from DAX to the block device? Ross's changes will
track mmap'd ranges that are dirtied at the filesytem inode level,
and the fsync/writeback will trigger CPU cache writeback of those
dirty ranges. This will work for block devices that are mapped by
DAX, too, because they have a inode+mapping tree, too.

And if we are going to use Ross's infrastructure (which, when we
work the kinks out of, I think we will), we really should change
dax_do_io() to track pfns that are dirtied this way, too. That will
allow us to get rid of all the cache flushing from the DAX layer
(they'll get pushed into fsync/writeback) and so we only take the
CPU cache flushing penalties when synchronous operations are
requested by userspace...

> We can later extend the DAX paths to indicate when an async mapping is
> "closed" allowing the active extents to be marked clean.

Yes, that's a basic feature of Ross's patches. Hence I think this
special case DAX<->bdev interface is the wrong direction to be
taking.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261198 — Re: [PATCH v3 14/15] dax: dirty extent notification

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-03 06:10 +0100
SubjectRe: [PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqC13-6GC-1@gated-at.bofh.it>
In reply to#1261109
On Mon, Nov 2, 2015 at 5:16 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sun, Nov 01, 2015 at 11:30:58PM -0500, Dan Williams wrote:
>> DAX-enabled block device drivers can use hints from fs/dax.c to
>> optimize their internal tracking of potentially dirty cpu cache lines.
>> If a DAX mapping is being used for synchronous operations, dax_do_io(),
>> a dax-enabled block-driver knows that fs/dax.c will handle immediate
>> flushing.  For asynchronous mappings, i.e.  returned to userspace via
>> mmap, the driver can track active extents of the media for flushing.
>
> So, essentially, you are marking the calls into the mapping calls
> with BLKDAX_F_DIRTY when the mapping is requested for a write page
> fault?  Hence allowing the block device to track "dirty pages"
> exactly?

Not pages, but larger extents (1 extent = 1/NUM_DAX_EXTENTS of the
total storage capacity), because tracking dirty mappings should be
temporary compatibility hack and not a first class citizen.

> But, really, if we're going to use Ross's mapping tree patches that
> use exceptional entries to track dirty pfns, why do we need to this
> special interface from DAX to the block device? Ross's changes will
> track mmap'd ranges that are dirtied at the filesytem inode level,
> and the fsync/writeback will trigger CPU cache writeback of those
> dirty ranges. This will work for block devices that are mapped by
> DAX, too, because they have a inode+mapping tree, too.
>
> And if we are going to use Ross's infrastructure (which, when we
> work the kinks out of, I think we will), we really should change
> dax_do_io() to track pfns that are dirtied this way, too. That will
> allow us to get rid of all the cache flushing from the DAX layer
> (they'll get pushed into fsync/writeback) and so we only take the
> CPU cache flushing penalties when synchronous operations are
> requested by userspace...

No, we definitely can't do that.   I think your mental model of the
cache flushing is similar to the disk model where a small buffer is
flushed after a large streaming write.  Both Ross' patches and my
approach suffer from the same horror that the cache flushing is O(N)
currently, so we don't want to make it responsible for more data
ranges areas than is strictly necessary.

>> We can later extend the DAX paths to indicate when an async mapping is
>> "closed" allowing the active extents to be marked clean.
>
> Yes, that's a basic feature of Ross's patches. Hence I think this
> special case DAX<->bdev interface is the wrong direction to be
> taking.

So here's my problem with the "track dirty mappings" in the core
mm/vfs approach, it's harder to unwind and delete when it turns out no
application actually needs it, or the platform gives us an O(1) flush
method that is independent of dirty pte tracking.

We have the NVML [1] library as the recommended method for
applications to interact with persistent memory and it is not using
fsync/msync for its synchronization primitives, it's managing the
cache directly.  The *only* user for tracking dirty DAX mappings is
unmodified legacy applications that do mmap I/O and call fsync/msync.

DAX in my opinion is not a transparent accelerator of all existing
apps, it's a targeted mechanism for applications ready to take
advantage of byte addressable persistent memory.  This is why I'm a
big supporter of your per-inode DAX control proposal.  The fact that
fsync is painful for large amounts of dirty data is a feature.  It
detects inodes that should have had DAX-disabled in the first
instance.  The only advantage of the radix approach is that the second
fsync after the big hit may be faster, but that still can't beat
either targeted disabling of DAX or updating the app to use NVML.

So, again, I remain to be convinced that we need to carry complexity
in the core kernel when we have the page cache to cover those cases.
The driver solution is a minimal extension of the data
bdev_direct_access() is already sending down to the driver, and covers
the gap without mm/fs entanglements while we figure out a longer term
solution.

[1]: https://github.com/pmem/nvml
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261209 — Re: [PATCH v3 14/15] dax: dirty extent notification

FromDave Chinner <david@fromorbit.com>
Date2015-11-03 06:50 +0100
SubjectRe: [PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqCDM-6X8-17@gated-at.bofh.it>
In reply to#1261198
On Mon, Nov 02, 2015 at 08:56:24PM -0800, Dan Williams wrote:
> On Mon, Nov 2, 2015 at 5:16 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sun, Nov 01, 2015 at 11:30:58PM -0500, Dan Williams wrote:
> >> DAX-enabled block device drivers can use hints from fs/dax.c to
> >> optimize their internal tracking of potentially dirty cpu cache lines.
> >> If a DAX mapping is being used for synchronous operations, dax_do_io(),
> >> a dax-enabled block-driver knows that fs/dax.c will handle immediate
> >> flushing.  For asynchronous mappings, i.e.  returned to userspace via
> >> mmap, the driver can track active extents of the media for flushing.
> >
> > So, essentially, you are marking the calls into the mapping calls
> > with BLKDAX_F_DIRTY when the mapping is requested for a write page
> > fault?  Hence allowing the block device to track "dirty pages"
> > exactly?
> 
> Not pages, but larger extents (1 extent = 1/NUM_DAX_EXTENTS of the
> total storage capacity), because tracking dirty mappings should be
> temporary compatibility hack and not a first class citizen.
> 
> > But, really, if we're going to use Ross's mapping tree patches that
> > use exceptional entries to track dirty pfns, why do we need to this
> > special interface from DAX to the block device? Ross's changes will
> > track mmap'd ranges that are dirtied at the filesytem inode level,
> > and the fsync/writeback will trigger CPU cache writeback of those
> > dirty ranges. This will work for block devices that are mapped by
> > DAX, too, because they have a inode+mapping tree, too.
> >
> > And if we are going to use Ross's infrastructure (which, when we
> > work the kinks out of, I think we will), we really should change
> > dax_do_io() to track pfns that are dirtied this way, too. That will
> > allow us to get rid of all the cache flushing from the DAX layer
> > (they'll get pushed into fsync/writeback) and so we only take the
> > CPU cache flushing penalties when synchronous operations are
> > requested by userspace...
> 
> No, we definitely can't do that.   I think your mental model of the
> cache flushing is similar to the disk model where a small buffer is
> flushed after a large streaming write.  Both Ross' patches and my
> approach suffer from the same horror that the cache flushing is O(N)
> currently, so we don't want to make it responsible for more data
> ranges areas than is strictly necessary.

I didn't see anything that was O(N) in Ross's patches. What part of
the fsync algorithm that Ross proposed are you refering to here?

> >> We can later extend the DAX paths to indicate when an async mapping is
> >> "closed" allowing the active extents to be marked clean.
> >
> > Yes, that's a basic feature of Ross's patches. Hence I think this
> > special case DAX<->bdev interface is the wrong direction to be
> > taking.
> 
> So here's my problem with the "track dirty mappings" in the core
> mm/vfs approach, it's harder to unwind and delete when it turns out no
> application actually needs it, or the platform gives us an O(1) flush
> method that is independent of dirty pte tracking.
> 
> We have the NVML [1] library as the recommended method for
> applications to interact with persistent memory and it is not using
> fsync/msync for its synchronization primitives, it's managing the
> cache directly.  The *only* user for tracking dirty DAX mappings is
> unmodified legacy applications that do mmap I/O and call fsync/msync.

I'm pretty sure there are going to be many people still writing new
applications that use POSIX APIs they expect to work correctly on
pmem because, well, it's going to take 10 years before persistent
memory is common enough for most application developers to only
target storage via NVML.

The whole world is not crazy HFT applications that need to bypass
the kernel for *everything* because even a few nanoseconds of extra
latency matters.

> DAX in my opinion is not a transparent accelerator of all existing
> apps, it's a targeted mechanism for applications ready to take
> advantage of byte addressable persistent memory. 

And this is where we disagree. DAX is a method of allowing POSIX
compliant applications get the best of both worlds - portability
with existing storage and filesystems, yet with the speed and byte
addressiblity of persistent storage through the use of mmap.

Applications designed specifically for persistent memory don't want
a general purpose, POSIX compatible filesystem underneath them. The
should be interacting directly with, and only with, your NVML
library. If the NVML library is implemented by using DAX on a POSIX
compatible, general purpose filesystem, then you're just going to
have to live with everything we need to do to make DAX work with
general purpose POSIX compatible applications.

DAX has always been intended as a *stopgap measure* designed to
bridge the gap between existing POSIX based storage APIs and PMEM
native filesystem implementations. You're advocating that DAX should
only be used by PMEM native applications using NVML and then saying
anything that might be needed for POSIX compatible behaviour is
unacceptible overhead...

> This is why I'm a
> big supporter of your per-inode DAX control proposal.  The fact that
> fsync is painful for large amounts of dirty data is a feature.  It
> detects inodes that should have had DAX-disabled in the first
> instance.

fsync is painful for any storage when there is large amounts of
dirty data. DAX is no different, and it's not a reason for saying
"don't use DAX". DAX + fsync should be faster than "buffered IO
through the page cache on pmem + fsync" because there is only one
memory copy being done in the DAX case.

The buffered IO case has all that per-page radix tree tracking in it,
writeback, etc. Yet:

# mount -o dax /dev/ram0 /mnt/scratch
# time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
wrote 3221225472/3221225472 bytes at offset 0
3.000 GiB, 384 ops; 0:00:10.00 (305.746 MiB/sec and 38.2182 ops/sec)
0.00user 10.05system 0:10.05elapsed 100%CPU (0avgtext+0avgdata 10512maxresident)k
0inputs+0outputs (0major+2156minor)pagefaults 0swaps
# umount /mnt/scratch
# mount /dev/ram0 /mnt/scratch
# time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
wrote 3221225472/3221225472 bytes at offset 0
3.000 GiB, 384 ops; 0:00:02.00 (1.218 GiB/sec and 155.9046 ops/sec)
0.00user 2.83system 0:02.86elapsed 99%CPU (0avgtext+0avgdata 10468maxresident)k
0inputs+0outputs (0major+2154minor)pagefaults 0swaps
#

So don't tell me that tracking dirty pages in the radix tree too
slow for DAX and that DAX should not be used for POSIX IO based
applications - it should be as fast as buffered IO, if not faster,
and if it isn't then we've screwed up real bad. And right now, we're
screwing up real bad.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261252 — Re: [PATCH v3 14/15] dax: dirty extent notification

FromDan Williams <dan.j.williams@intel.com>
Date2015-11-03 08:30 +0100
SubjectRe: [PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqEcy-83d-27@gated-at.bofh.it>
In reply to#1261209
On Mon, Nov 2, 2015 at 9:40 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Nov 02, 2015 at 08:56:24PM -0800, Dan Williams wrote:
>> No, we definitely can't do that.   I think your mental model of the
>> cache flushing is similar to the disk model where a small buffer is
>> flushed after a large streaming write.  Both Ross' patches and my
>> approach suffer from the same horror that the cache flushing is O(N)
>> currently, so we don't want to make it responsible for more data
>> ranges areas than is strictly necessary.
>
> I didn't see anything that was O(N) in Ross's patches. What part of
> the fsync algorithm that Ross proposed are you refering to here?

We have to issue clflush per touched virtual address rather than a
constant number of physical ways, or a flush-all instruction.

>> >> We can later extend the DAX paths to indicate when an async mapping is
>> >> "closed" allowing the active extents to be marked clean.
>> >
>> > Yes, that's a basic feature of Ross's patches. Hence I think this
>> > special case DAX<->bdev interface is the wrong direction to be
>> > taking.
>>
>> So here's my problem with the "track dirty mappings" in the core
>> mm/vfs approach, it's harder to unwind and delete when it turns out no
>> application actually needs it, or the platform gives us an O(1) flush
>> method that is independent of dirty pte tracking.
>>
>> We have the NVML [1] library as the recommended method for
>> applications to interact with persistent memory and it is not using
>> fsync/msync for its synchronization primitives, it's managing the
>> cache directly.  The *only* user for tracking dirty DAX mappings is
>> unmodified legacy applications that do mmap I/O and call fsync/msync.
>
> I'm pretty sure there are going to be many people still writing new
> applications that use POSIX APIs they expect to work correctly on
> pmem because, well, it's going to take 10 years before persistent
> memory is common enough for most application developers to only
> target storage via NVML.
>
> The whole world is not crazy HFT applications that need to bypass
> the kernel for *everything* because even a few nanoseconds of extra
> latency matters.

I agree with all of that...

>> DAX in my opinion is not a transparent accelerator of all existing
>> apps, it's a targeted mechanism for applications ready to take
>> advantage of byte addressable persistent memory.
>
> And this is where we disagree. DAX is a method of allowing POSIX
> compliant applications get the best of both worlds - portability
> with existing storage and filesystems, yet with the speed and byte
> addressiblity of persistent storage through the use of mmap.
>
> Applications designed specifically for persistent memory don't want
> a general purpose, POSIX compatible filesystem underneath them. The
> should be interacting directly with, and only with, your NVML
> library. If the NVML library is implemented by using DAX on a POSIX
> compatible, general purpose filesystem, then you're just going to
> have to live with everything we need to do to make DAX work with
> general purpose POSIX compatible applications.
>
> DAX has always been intended as a *stopgap measure* designed to
> bridge the gap between existing POSIX based storage APIs and PMEM
> native filesystem implementations. You're advocating that DAX should
> only be used by PMEM native applications using NVML and then saying
> anything that might be needed for POSIX compatible behaviour is
> unacceptible overhead...

Also agreed, up until you this last sentence which is not what I am
saying at all.  I didn't say it is unacceptable overhead, my solution
in the driver has the exact same overhead.

Where I instead think we disagree is the acceptable cost of the "flush
cache" operation before the recommended solution is to locally disable
DAX, or require help from the platform to do this operation more
efficiently.  What I submit is unacceptable is to have the cpu loop
over every address heading out to storage.  The radix solution only
makes the second fsync after the first potentially less costly over
time.

I don't think we'll need it long term, or so I hope. The question
becomes do we want to carry this complexity in the core or push
selectively disabling DAX in the interim and have the simple driver
approach for cases where it's not feasible to disable DAX.  For 4.4 we
have the practical matter of not having the time to get mm folks to
review the radix approach.

I'm not opposed to ripping out the driver solution in 4.5 when we have
the time to get Ross' implementation reviewed.  I'm also holding back
the get_user_page() patches until 4.5 and given the big fat comment in
write_protect_page() about gup-fast interactions we'll need to think
through similar implications.

>
>> This is why I'm a
>> big supporter of your per-inode DAX control proposal.  The fact that
>> fsync is painful for large amounts of dirty data is a feature.  It
>> detects inodes that should have had DAX-disabled in the first
>> instance.
>
> fsync is painful for any storage when there is large amounts of
> dirty data. DAX is no different, and it's not a reason for saying
> "don't use DAX". DAX + fsync should be faster than "buffered IO
> through the page cache on pmem + fsync" because there is only one
> memory copy being done in the DAX case.
>
> The buffered IO case has all that per-page radix tree tracking in it,
> writeback, etc. Yet:
>
> # mount -o dax /dev/ram0 /mnt/scratch
> # time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
> wrote 3221225472/3221225472 bytes at offset 0
> 3.000 GiB, 384 ops; 0:00:10.00 (305.746 MiB/sec and 38.2182 ops/sec)
> 0.00user 10.05system 0:10.05elapsed 100%CPU (0avgtext+0avgdata 10512maxresident)k
> 0inputs+0outputs (0major+2156minor)pagefaults 0swaps
> # umount /mnt/scratch
> # mount /dev/ram0 /mnt/scratch
> # time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
> wrote 3221225472/3221225472 bytes at offset 0
> 3.000 GiB, 384 ops; 0:00:02.00 (1.218 GiB/sec and 155.9046 ops/sec)
> 0.00user 2.83system 0:02.86elapsed 99%CPU (0avgtext+0avgdata 10468maxresident)k
> 0inputs+0outputs (0major+2154minor)pagefaults 0swaps
> #
>
> So don't tell me that tracking dirty pages in the radix tree too
> slow for DAX and that DAX should not be used for POSIX IO based
> applications - it should be as fast as buffered IO, if not faster,
> and if it isn't then we've screwed up real bad. And right now, we're
> screwing up real bad.

Again, it's not the dirty tracking in the radix I'm worried about it's
looping through all the virtual addresses within those pages..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1261889 — Re: [PATCH v3 14/15] dax: dirty extent notification

FromDave Chinner <david@fromorbit.com>
Date2015-11-03 22:00 +0100
SubjectRe: [PATCH v3 14/15] dax: dirty extent notification
Message-ID<qqQQp-7KR-5@gated-at.bofh.it>
In reply to#1261252
On Mon, Nov 02, 2015 at 11:20:49PM -0800, Dan Williams wrote:
> On Mon, Nov 2, 2015 at 9:40 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Nov 02, 2015 at 08:56:24PM -0800, Dan Williams wrote:
> >> No, we definitely can't do that.   I think your mental model of the
> >> cache flushing is similar to the disk model where a small buffer is
> >> flushed after a large streaming write.  Both Ross' patches and my
> >> approach suffer from the same horror that the cache flushing is O(N)
> >> currently, so we don't want to make it responsible for more data
> >> ranges areas than is strictly necessary.
> >
> > I didn't see anything that was O(N) in Ross's patches. What part of
> > the fsync algorithm that Ross proposed are you refering to here?
> 
> We have to issue clflush per touched virtual address rather than a
> constant number of physical ways, or a flush-all instruction.
.....
> > So don't tell me that tracking dirty pages in the radix tree too
> > slow for DAX and that DAX should not be used for POSIX IO based
> > applications - it should be as fast as buffered IO, if not faster,
> > and if it isn't then we've screwed up real bad. And right now, we're
> > screwing up real bad.
> 
> Again, it's not the dirty tracking in the radix I'm worried about it's
> looping through all the virtual addresses within those pages..

So, let me summarise what I think you've just said. You are

1. fine with looping through the virtual addresses doing cache flushes
   synchronously when doing IO despite it having significant
   latency and performance costs.

2. Happy to hack a method into DAX to bypass the filesystems by
   pushing information to the block device for it to track regions that
   need cache flushes, then add infrastructure to the block device to
   track those dirty regions and then walk those addresses and issue
   cache flushes when the filesystem issues a REQ_FLUSH IO regardless
   of whether the filesystem actually needs those cachelines flushed
   for that specific IO?

3. Not happy to use the generic mm/vfs level infrastructure
   architectected specifically to provide the exact asynchronous
   cache flushing/writeback semantics we require because it will
   cause too many cache flushes, even though the number of cache
   flushes will be, at worst, the same as in 2).


1) will work, but as we can see it is *slow*. 3) is what Ross is
implementing - it's a tried and tested architecture that all mm/fs
developers understand, and his explanation of why it will work for
pmem is pretty solid and completely platform/hardware architecture
independent.

Which leaves this question: How does 2) save us anything in terms of
avoiding iterating virtual addresses and issuing cache flushes
over 3)? And is it sufficient to justify hacking a bypass into DAX
and the additional driver level complexity of having to add dirty
region tracking, flushing and cleaning to REQ_FLUSH operations?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web