[PATCH v4 00/16] pmem: stop abusing copy_user_nocache(), and other reworks

[PATCH v4 00/16] pmem: stop abusing copy_user_nocache(), and other reworks

[Dan Williams]

Changes since v3 [1]:

* Remove default copy_from_iter() fallback in the dax core (Christoph).

* Don't abuse block/queue sysfs for cache-flush control, introduce a
  dax-specific interface (Christoph).

* Don't export clean_cache_range() export an arch_wb_cache_pmem() wrapper
  instead (Christoph). This also allows us to remove asm/pmem.h along
  with include/linux/pmem.h.

[1]: https://lkml.org/lkml/2017/6/9/842

The changes above are constrained to the last 4 patches of the series.
Patch1 still needs Al's review.

---

A few months back, in the course of reviewing the memcpy_nocache()
proposal from Brian, Linus proposed that the pmem specific
memcpy_to_pmem() routine be moved to be implemented at the driver level
[2]:

   "Quite frankly, the whole 'memcpy_nocache()' idea or (ab-)using
    copy_user_nocache() just needs to die. It's idiotic.

    As you point out, it's also fundamentally buggy crap.

    Throw it away. There is no possible way this is ever valid or
    portable. We're not going to lie and claim that it is.

    If some driver ends up using 'movnt' by hand, that is up to that
    *driver*. But no way in hell should we care about this one whit in
    the sense of <linux/uaccess.h>."

This feedback also dovetails with another fs/dax.c design wart of being
hard coded to assume the backing device is pmem. We call the pmem
specific copy, clear, and flush routines even if the backing device
driver is one of the other 3 dax drivers (axonram, dccssblk, or brd).
There is no reason to spend cpu cycles flushing the cache after writing
to brd, for example, since it is using volatile memory for storage.

Moreover, the pmem driver might be fronting a volatile memory range
published by the ACPI NFIT, or the platform might have arranged to flush
cpu caches on power fail. This latter capability is a feature that has
appeared in embedded storage appliances (pre-ACPI-NFIT nvdimm
platforms).

Now, the comment about completely avoiding uaccess.h is augmented by
Al's recent assertion:

   "And for !@#!@# sake, comments like this
    +        * On x86_64 __copy_from_user_nocache() uses non-temporal stores
    +        * for the bulk of the transfer, but we need to manually flush
    +        * if the transfer is unaligned. A cached memory copy is used
    +        * when destination or size is not naturally aligned. That is:
    +        *   - Require 8-byte alignment when size is 8 bytes or larger.
    +        *   - Require 4-byte alignment when size is 4 bytes.
    mean only one thing: this should live in arch/x86/lib/usercopy_64.c,
    right next to the actual function that does copying.  NOT in
    drivers/nvdimm/x86.c.  At the very least it needs a comment in usercopy_64.c
    with dire warnings along the lines of "don't touch that code without
    looking into <filename>:pmem_from_user().."

So, this series proceeds to keep all the usercopy code centralized. The
change set:

1/ Moves what was previously named "the pmem api" out of the global
   namespace and into the libnvdimm sub-system that needs to be
   concerned with architecture specific persistent memory considerations.

2/ Arranges for dax to stop abusing __copy_user_nocache() and implements
   formal _flushcache helpers that use 'movnt' on x86_64.

3/ Makes filesystem-dax cache maintenance optional by arranging for dax
   to call driver specific copy and flush operations only if the driver
   publishes them.

4/ Allows filesytem-dax cache-flushing to be controlled by a new
   'dax/write_cache' sysfs attribute. The pmem driver is updated to
   clear the flag by default when pmem is driving volatile memory. ACPI
   6.2 defines a mechanism to detect if the platform handles cpu cache
   flushing for pmem and will be used to set the default for this flag.

[2]: https://lists.01.org/pipermail/linux-nvdimm/2017-January/008364.html

This series is based on v4.12-rc4 and passes the current ndctl
regression suite.

---

Dan Williams (16):
      x86, uaccess: introduce copy_from_iter_flushcache for pmem / cache-bypass operations
      dm: add ->copy_from_iter() dax operation support
      filesystem-dax: convert to dax_copy_from_iter()
      dax, pmem: introduce an optional 'flush' dax_operation
      dm: add ->flush() dax operation support
      filesystem-dax: convert to dax_flush()
      x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush
      x86, dax, libnvdimm: remove wb_cache_pmem() indirection
      x86, libnvdimm, pmem: move arch_invalidate_pmem() to libnvdimm
      x86, libnvdimm, pmem: remove global pmem api
      libnvdimm, pmem: fix persistence warning
      libnvdimm, nfit: enable support for volatile ranges
      dax: remove default copy_from_iter fallback
      dax: convert to bitmask for flags
      libnvdimm, pmem, dax: export a cache control attribute
      libnvdimm, pmem: disable dax flushing when pmem is fronting a volatile region


 MAINTAINERS                       |    4 -
 arch/powerpc/sysdev/axonram.c     |    8 ++
 arch/x86/Kconfig                  |    1 
 arch/x86/include/asm/pmem.h       |  136 -----------------------------------
 arch/x86/include/asm/string_64.h  |    5 +
 arch/x86/include/asm/uaccess_64.h |   11 +++
 arch/x86/lib/usercopy_64.c        |  134 +++++++++++++++++++++++++++++++++++
 arch/x86/mm/pageattr.c            |    6 ++
 drivers/acpi/nfit/core.c          |   15 +++-
 drivers/block/brd.c               |    8 ++
 drivers/dax/super.c               |  118 +++++++++++++++++++++++++++++--
 drivers/md/dm-linear.c            |   30 ++++++++
 drivers/md/dm-stripe.c            |   40 ++++++++++
 drivers/md/dm.c                   |   45 ++++++++++++
 drivers/nvdimm/bus.c              |    8 +-
 drivers/nvdimm/claim.c            |    6 +-
 drivers/nvdimm/core.c             |    2 -
 drivers/nvdimm/dax_devs.c         |    2 -
 drivers/nvdimm/dimm_devs.c        |   10 ++-
 drivers/nvdimm/namespace_devs.c   |   14 +---
 drivers/nvdimm/nd-core.h          |    9 ++
 drivers/nvdimm/pfn_devs.c         |    4 +
 drivers/nvdimm/pmem.c             |   40 +++++++++-
 drivers/nvdimm/pmem.h             |   14 ++++
 drivers/nvdimm/region_devs.c      |   43 +++++++----
 drivers/s390/block/dcssblk.c      |    8 ++
 fs/dax.c                          |    9 +-
 include/linux/dax.h               |   12 +++
 include/linux/device-mapper.h     |    6 ++
 include/linux/libnvdimm.h         |    2 +
 include/linux/pmem.h              |  142 -------------------------------------
 include/linux/string.h            |    6 ++
 include/linux/uio.h               |   15 ++++
 lib/Kconfig                       |    3 +
 lib/iov_iter.c                    |   22 ++++++
 35 files changed, 597 insertions(+), 341 deletions(-)
 delete mode 100644 arch/x86/include/asm/pmem.h
 delete mode 100644 include/linux/pmem.h

[PATCH v4 01/16] x86, uaccess: introduce copy_from_iter_flushcache for pmem / cache-bypass operations

[Dan Williams]

The pmem driver has a need to transfer data with a persistent memory
destination and be able to rely on the fact that the destination writes are not
cached. It is sufficient for the writes to be flushed to a cpu-store-buffer
(non-temporal / "movnt" in x86 terms), as we expect userspace to call fsync()
to ensure data-writes have reached a power-fail-safe zone in the platform. The
fsync() triggers a REQ_FUA or REQ_FLUSH to the pmem driver which will turn
around and fence previous writes with an "sfence".

Implement a __copy_from_user_inatomic_flushcache, memcpy_page_flushcache, and
memcpy_flushcache, that guarantee that the destination buffer is not dirty in
the cpu cache on completion. The new copy_from_iter_flushcache and sub-routines
will be used to replace the "pmem api" (include/linux/pmem.h +
arch/x86/include/asm/pmem.h). The availability of copy_from_iter_flushcache()
and memcpy_flushcache() are gated by the CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
config symbol, and fallback to copy_from_iter_nocache() and plain memcpy()
otherwise.

This is meant to satisfy the concern from Linus that if a driver wants to do
something beyond the normal nocache semantics it should be something private to
that driver [1], and Al's concern that anything uaccess related belongs with
the rest of the uaccess code [2].

The first consumer of this interface is a new 'copy_from_iter' dax operation so
that pmem can inject cache maintenance operations without imposing this
overhead on other dax-capable drivers.

[1]: https://lists.01.org/pipermail/linux-nvdimm/2017-January/008364.html
[2]: https://lists.01.org/pipermail/linux-nvdimm/2017-April/009942.html

Cc: <x86@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/Kconfig                  |    1 
 arch/x86/include/asm/string_64.h  |    5 +
 arch/x86/include/asm/uaccess_64.h |   11 +++
 arch/x86/lib/usercopy_64.c        |  128 +++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/core.c          |    3 -
 drivers/nvdimm/claim.c            |    2 -
 drivers/nvdimm/pmem.c             |   13 +++-
 drivers/nvdimm/region_devs.c      |    4 +
 include/linux/dax.h               |    3 +
 include/linux/string.h            |    6 ++
 include/linux/uio.h               |   15 ++++
 lib/Kconfig                       |    3 +
 lib/iov_iter.c                    |   22 ++++++
 13 files changed, 209 insertions(+), 7 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4ccfacc7232a..bb273b2f50b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -54,6 +54,7 @@ config X86
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_PMEM_API		if X86_64
+	select ARCH_HAS_UACCESS_FLUSHCACHE	if X86_64
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_STRICT_KERNEL_RWX
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
index 733bae07fb29..1f22bc277c45 100644
--- a/arch/x86/include/asm/string_64.h
+++ b/arch/x86/include/asm/string_64.h
@@ -109,6 +109,11 @@ memcpy_mcsafe(void *dst, const void *src, size_t cnt)
 	return 0;
 }
 
+#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
+#define __HAVE_ARCH_MEMCPY_FLUSHCACHE 1
+void memcpy_flushcache(void *dst, const void *src, size_t cnt);
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_STRING_64_H */
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index c5504b9a472e..b16f6a1d8b26 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -171,6 +171,10 @@ unsigned long raw_copy_in_user(void __user *dst, const void __user *src, unsigne
 extern long __copy_user_nocache(void *dst, const void __user *src,
 				unsigned size, int zerorest);
 
+extern long __copy_user_flushcache(void *dst, const void __user *src, unsigned size);
+extern void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
+			   size_t len);
+
 static inline int
 __copy_from_user_inatomic_nocache(void *dst, const void __user *src,
 				  unsigned size)
@@ -179,6 +183,13 @@ __copy_from_user_inatomic_nocache(void *dst, const void __user *src,
 	return __copy_user_nocache(dst, src, size, 0);
 }
 
+static inline int
+__copy_from_user_flushcache(void *dst, const void __user *src, unsigned size)
+{
+	kasan_check_write(dst, size);
+	return __copy_user_flushcache(dst, src, size);
+}
+
 unsigned long
 copy_user_handle_tail(char *to, char *from, unsigned len);
 
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index 3b7c40a2e3e1..f42d2fd86ca3 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -7,6 +7,7 @@
  */
 #include <linux/export.h>
 #include <linux/uaccess.h>
+#include <linux/highmem.h>
 
 /*
  * Zero Userspace
@@ -73,3 +74,130 @@ copy_user_handle_tail(char *to, char *from, unsigned len)
 	clac();
 	return len;
 }
+
+#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
+/**
+ * clean_cache_range - write back a cache range with CLWB
+ * @vaddr:	virtual start address
+ * @size:	number of bytes to write back
+ *
+ * Write back a cache range using the CLWB (cache line write back)
+ * instruction. Note that @size is internally rounded up to be cache
+ * line size aligned.
+ */
+static void clean_cache_range(void *addr, size_t size)
+{
+	u16 x86_clflush_size = boot_cpu_data.x86_clflush_size;
+	unsigned long clflush_mask = x86_clflush_size - 1;
+	void *vend = addr + size;
+	void *p;
+
+	for (p = (void *)((unsigned long)addr & ~clflush_mask);
+	     p < vend; p += x86_clflush_size)
+		clwb(p);
+}
+
+long __copy_user_flushcache(void *dst, const void __user *src, unsigned size)
+{
+	unsigned long flushed, dest = (unsigned long) dst;
+	long rc = __copy_user_nocache(dst, src, size, 0);
+
+	/*
+	 * __copy_user_nocache() uses non-temporal stores for the bulk
+	 * of the transfer, but we need to manually flush if the
+	 * transfer is unaligned. A cached memory copy is used when
+	 * destination or size is not naturally aligned. That is:
+	 *   - Require 8-byte alignment when size is 8 bytes or larger.
+	 *   - Require 4-byte alignment when size is 4 bytes.
+	 */
+	if (size < 8) {
+		if (!IS_ALIGNED(dest, 4) || size != 4)
+			clean_cache_range(dst, 1);
+	} else {
+		if (!IS_ALIGNED(dest, 8)) {
+			dest = ALIGN(dest, boot_cpu_data.x86_clflush_size);
+			clean_cache_range(dst, 1);
+		}
+
+		flushed = dest - (unsigned long) dst;
+		if (size > flushed && !IS_ALIGNED(size - flushed, 8))
+			clean_cache_range(dst + size - 1, 1);
+	}
+
+	return rc;
+}
+
+void memcpy_flushcache(void *_dst, const void *_src, size_t size)
+{
+	unsigned long dest = (unsigned long) _dst;
+	unsigned long source = (unsigned long) _src;
+
+	/* cache copy and flush to align dest */
+	if (!IS_ALIGNED(dest, 8)) {
+		unsigned len = min_t(unsigned, size, ALIGN(dest, 8) - dest);
+
+		memcpy((void *) dest, (void *) source, len);
+		clean_cache_range((void *) dest, len);
+		dest += len;
+		source += len;
+		size -= len;
+		if (!size)
+			return;
+	}
+
+	/* 4x8 movnti loop */
+	while (size >= 32) {
+		asm("movq    (%0), %%r8\n"
+		    "movq   8(%0), %%r9\n"
+		    "movq  16(%0), %%r10\n"
+		    "movq  24(%0), %%r11\n"
+		    "movnti  %%r8,   (%1)\n"
+		    "movnti  %%r9,  8(%1)\n"
+		    "movnti %%r10, 16(%1)\n"
+		    "movnti %%r11, 24(%1)\n"
+		    :: "r" (source), "r" (dest)
+		    : "memory", "r8", "r9", "r10", "r11");
+		dest += 32;
+		source += 32;
+		size -= 32;
+	}
+
+	/* 1x8 movnti loop */
+	while (size >= 8) {
+		asm("movq    (%0), %%r8\n"
+		    "movnti  %%r8,   (%1)\n"
+		    :: "r" (source), "r" (dest)
+		    : "memory", "r8");
+		dest += 8;
+		source += 8;
+		size -= 8;
+	}
+
+	/* 1x4 movnti loop */
+	while (size >= 4) {
+		asm("movl    (%0), %%r8d\n"
+		    "movnti  %%r8d,   (%1)\n"
+		    :: "r" (source), "r" (dest)
+		    : "memory", "r8");
+		dest += 4;
+		source += 4;
+		size -= 4;
+	}
+
+	/* cache copy for remaining bytes */
+	if (size) {
+		memcpy((void *) dest, (void *) source, size);
+		clean_cache_range((void *) dest, size);
+	}
+}
+EXPORT_SYMBOL_GPL(memcpy_flushcache);
+
+void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
+		size_t len)
+{
+	char *from = kmap_atomic(page);
+
+	memcpy_flushcache(to, from + offset, len);
+	kunmap_atomic(from);
+}
+#endif
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 656acb5d7166..cbd5596e7562 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1842,8 +1842,7 @@ static int acpi_nfit_blk_single_io(struct nfit_blk *nfit_blk,
 		}
 
 		if (rw)
-			memcpy_to_pmem(mmio->addr.aperture + offset,
-					iobuf + copied, c);
+			memcpy_flushcache(mmio->addr.aperture + offset, iobuf + copied, c);
 		else {
 			if (nfit_blk->dimm_flags & NFIT_BLK_READ_FLUSH)
 				mmio_flush_range((void __force *)
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 7ceb5fa4f2a1..b8b9c8ca7862 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -277,7 +277,7 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns,
 			rc = -EIO;
 	}
 
-	memcpy_to_pmem(nsio->addr + offset, buf, size);
+	memcpy_flushcache(nsio->addr + offset, buf, size);
 	nvdimm_flush(to_nd_region(ndns->dev.parent));
 
 	return rc;
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index c544d466ea51..2f3aefe565c6 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -29,6 +29,7 @@
 #include <linux/pfn_t.h>
 #include <linux/slab.h>
 #include <linux/pmem.h>
+#include <linux/uio.h>
 #include <linux/dax.h>
 #include <linux/nd.h>
 #include "pmem.h"
@@ -80,7 +81,7 @@ static void write_pmem(void *pmem_addr, struct page *page,
 {
 	void *mem = kmap_atomic(page);
 
-	memcpy_to_pmem(pmem_addr, mem + off, len);
+	memcpy_flushcache(pmem_addr, mem + off, len);
 	kunmap_atomic(mem);
 }
 
@@ -235,8 +236,15 @@ static long pmem_dax_direct_access(struct dax_device *dax_dev,
 	return __pmem_direct_access(pmem, pgoff, nr_pages, kaddr, pfn);
 }
 
+static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	return copy_from_iter_flushcache(addr, bytes, i);
+}
+
 static const struct dax_operations pmem_dax_ops = {
 	.direct_access = pmem_dax_direct_access,
+	.copy_from_iter = pmem_copy_from_iter,
 };
 
 static void pmem_release_queue(void *q)
@@ -294,7 +302,8 @@ static int pmem_attach_disk(struct device *dev,
 	dev_set_drvdata(dev, pmem);
 	pmem->phys_addr = res->start;
 	pmem->size = resource_size(res);
-	if (nvdimm_has_flush(nd_region) < 0)
+	if (!IS_ENABLED(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE)
+			|| nvdimm_has_flush(nd_region) < 0)
 		dev_warn(dev, "unable to guarantee persistence of writes\n");
 
 	if (!devm_request_mem_region(dev, res->start, resource_size(res),
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index b550edf2571f..985b0e11bd73 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1015,8 +1015,8 @@ void nvdimm_flush(struct nd_region *nd_region)
 	 * The first wmb() is needed to 'sfence' all previous writes
 	 * such that they are architecturally visible for the platform
 	 * buffer flush.  Note that we've already arranged for pmem
-	 * writes to avoid the cache via arch_memcpy_to_pmem().  The
-	 * final wmb() ensures ordering for the NVDIMM flush write.
+	 * writes to avoid the cache via memcpy_flushcache().  The final
+	 * wmb() ensures ordering for the NVDIMM flush write.
 	 */
 	wmb();
 	for (i = 0; i < nd_region->ndr_mappings; i++)
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 5ec1f6c47716..bbe79ed90e2b 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -16,6 +16,9 @@ struct dax_operations {
 	 */
 	long (*direct_access)(struct dax_device *, pgoff_t, long,
 			void **, pfn_t *);
+	/* copy_from_iter: dax-driver override for default copy_from_iter */
+	size_t (*copy_from_iter)(struct dax_device *, pgoff_t, void *, size_t,
+			struct iov_iter *);
 };
 
 #if IS_ENABLED(CONFIG_DAX)
diff --git a/include/linux/string.h b/include/linux/string.h
index 537918f8a98e..7439d83eaa33 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -122,6 +122,12 @@ static inline __must_check int memcpy_mcsafe(void *dst, const void *src,
 	return 0;
 }
 #endif
+#ifndef __HAVE_ARCH_MEMCPY_FLUSHCACHE
+static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt)
+{
+	memcpy(dst, src, cnt);
+}
+#endif
 void *memchr_inv(const void *s, int c, size_t n);
 char *strreplace(char *s, char old, char new);
 
diff --git a/include/linux/uio.h b/include/linux/uio.h
index f2d36a3d3005..55cd54a0e941 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -95,6 +95,21 @@ size_t copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i);
 size_t copy_from_iter(void *addr, size_t bytes, struct iov_iter *i);
 bool copy_from_iter_full(void *addr, size_t bytes, struct iov_iter *i);
 size_t copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i);
+#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
+/*
+ * Note, users like pmem that depend on the stricter semantics of
+ * copy_from_iter_flushcache() than copy_from_iter_nocache() must check for
+ * IS_ENABLED(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) before assuming that the
+ * destination is flushed from the cache on return.
+ */
+size_t copy_from_iter_flushcache(void *addr, size_t bytes, struct iov_iter *i);
+#else
+static inline size_t copy_from_iter_flushcache(void *addr, size_t bytes,
+				       struct iov_iter *i)
+{
+	return copy_from_iter_nocache(addr, bytes, i);
+}
+#endif
 bool copy_from_iter_full_nocache(void *addr, size_t bytes, struct iov_iter *i);
 size_t iov_iter_zero(size_t bytes, struct iov_iter *);
 unsigned long iov_iter_alignment(const struct iov_iter *i);
diff --git a/lib/Kconfig b/lib/Kconfig
index 0c8b78a9ae2e..2d1c4b3a085c 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -548,6 +548,9 @@ config ARCH_HAS_SG_CHAIN
 config ARCH_HAS_PMEM_API
 	bool
 
+config ARCH_HAS_UACCESS_FLUSHCACHE
+	bool
+
 config ARCH_HAS_MMIO_FLUSH
 	bool
 
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index f835964c9485..c9a69064462f 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -615,6 +615,28 @@ size_t copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i)
 }
 EXPORT_SYMBOL(copy_from_iter_nocache);
 
+#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
+size_t copy_from_iter_flushcache(void *addr, size_t bytes, struct iov_iter *i)
+{
+	char *to = addr;
+	if (unlikely(i->type & ITER_PIPE)) {
+		WARN_ON(1);
+		return 0;
+	}
+	iterate_and_advance(i, bytes, v,
+		__copy_from_user_flushcache((to += v.iov_len) - v.iov_len,
+					 v.iov_base, v.iov_len),
+		memcpy_page_flushcache((to += v.bv_len) - v.bv_len, v.bv_page,
+				 v.bv_offset, v.bv_len),
+		memcpy_flushcache((to += v.iov_len) - v.iov_len, v.iov_base,
+			v.iov_len)
+	)
+
+	return bytes;
+}
+EXPORT_SYMBOL_GPL(copy_from_iter_flushcache);
+#endif
+
 bool copy_from_iter_full_nocache(void *addr, size_t bytes, struct iov_iter *i)
 {
 	char *to = addr;

[PATCH v4 02/16] dm: add ->copy_from_iter() dax operation support

[Dan Williams]

Allow device-mapper to route copy_from_iter operations to the
per-target implementation. In order for the device stacking to work we
need a dax_dev and a pgoff relative to that device. This gives each
layer of the stack the information it needs to look up the operation
pointer for the next level.

This conceptually allows for an array of mixed device drivers with
varying copy_from_iter implementations.

Reviewed-by: Toshi Kani <toshi.kani@hpe.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c           |   13 +++++++++++++
 drivers/md/dm-linear.c        |   15 +++++++++++++++
 drivers/md/dm-stripe.c        |   20 ++++++++++++++++++++
 drivers/md/dm.c               |   26 ++++++++++++++++++++++++++
 include/linux/dax.h           |    2 ++
 include/linux/device-mapper.h |    3 +++
 6 files changed, 79 insertions(+)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 6ed32aac8bbe..dd299e55f65d 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -18,6 +18,7 @@
 #include <linux/cdev.h>
 #include <linux/hash.h>
 #include <linux/slab.h>
+#include <linux/uio.h>
 #include <linux/dax.h>
 #include <linux/fs.h>
 
@@ -172,6 +173,18 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
 }
 EXPORT_SYMBOL_GPL(dax_direct_access);
 
+size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
+		size_t bytes, struct iov_iter *i)
+{
+	if (!dax_alive(dax_dev))
+		return 0;
+
+	if (!dax_dev->ops->copy_from_iter)
+		return copy_from_iter(addr, bytes, i);
+	return dax_dev->ops->copy_from_iter(dax_dev, pgoff, addr, bytes, i);
+}
+EXPORT_SYMBOL_GPL(dax_copy_from_iter);
+
 bool dax_alive(struct dax_device *dax_dev)
 {
 	lockdep_assert_held(&dax_srcu);
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 7d42a9d9f406..0841ec1bfbad 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -159,6 +159,20 @@ static long linear_dax_direct_access(struct dm_target *ti, pgoff_t pgoff,
 	return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn);
 }
 
+static size_t linear_dax_copy_from_iter(struct dm_target *ti, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	struct linear_c *lc = ti->private;
+	struct block_device *bdev = lc->dev->bdev;
+	struct dax_device *dax_dev = lc->dev->dax_dev;
+	sector_t dev_sector, sector = pgoff * PAGE_SECTORS;
+
+	dev_sector = linear_map_sector(ti, sector);
+	if (bdev_dax_pgoff(bdev, dev_sector, ALIGN(bytes, PAGE_SIZE), &pgoff))
+		return 0;
+	return dax_copy_from_iter(dax_dev, pgoff, addr, bytes, i);
+}
+
 static struct target_type linear_target = {
 	.name   = "linear",
 	.version = {1, 3, 0},
@@ -171,6 +185,7 @@ static struct target_type linear_target = {
 	.prepare_ioctl = linear_prepare_ioctl,
 	.iterate_devices = linear_iterate_devices,
 	.direct_access = linear_dax_direct_access,
+	.dax_copy_from_iter = linear_dax_copy_from_iter,
 };
 
 int __init dm_linear_init(void)
diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index 75152482f3ad..1ef914f9ca72 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -332,6 +332,25 @@ static long stripe_dax_direct_access(struct dm_target *ti, pgoff_t pgoff,
 	return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn);
 }
 
+static size_t stripe_dax_copy_from_iter(struct dm_target *ti, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	sector_t dev_sector, sector = pgoff * PAGE_SECTORS;
+	struct stripe_c *sc = ti->private;
+	struct dax_device *dax_dev;
+	struct block_device *bdev;
+	uint32_t stripe;
+
+	stripe_map_sector(sc, sector, &stripe, &dev_sector);
+	dev_sector += sc->stripe[stripe].physical_start;
+	dax_dev = sc->stripe[stripe].dev->dax_dev;
+	bdev = sc->stripe[stripe].dev->bdev;
+
+	if (bdev_dax_pgoff(bdev, dev_sector, ALIGN(bytes, PAGE_SIZE), &pgoff))
+		return 0;
+	return dax_copy_from_iter(dax_dev, pgoff, addr, bytes, i);
+}
+
 /*
  * Stripe status:
  *
@@ -451,6 +470,7 @@ static struct target_type stripe_target = {
 	.iterate_devices = stripe_iterate_devices,
 	.io_hints = stripe_io_hints,
 	.direct_access = stripe_dax_direct_access,
+	.dax_copy_from_iter = stripe_dax_copy_from_iter,
 };
 
 int __init dm_stripe_init(void)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 37ccd73c79ec..7faaceb52819 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -19,6 +19,7 @@
 #include <linux/dax.h>
 #include <linux/slab.h>
 #include <linux/idr.h>
+#include <linux/uio.h>
 #include <linux/hdreg.h>
 #include <linux/delay.h>
 #include <linux/wait.h>
@@ -969,6 +970,30 @@ static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
 	return ret;
 }
 
+static size_t dm_dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	struct mapped_device *md = dax_get_private(dax_dev);
+	sector_t sector = pgoff * PAGE_SECTORS;
+	struct dm_target *ti;
+	long ret = 0;
+	int srcu_idx;
+
+	ti = dm_dax_get_live_target(md, sector, &srcu_idx);
+
+	if (!ti)
+		goto out;
+	if (!ti->type->dax_copy_from_iter) {
+		ret = copy_from_iter(addr, bytes, i);
+		goto out;
+	}
+	ret = ti->type->dax_copy_from_iter(ti, pgoff, addr, bytes, i);
+ out:
+	dm_put_live_table(md, srcu_idx);
+
+	return ret;
+}
+
 /*
  * A target may call dm_accept_partial_bio only from the map routine.  It is
  * allowed for all bio types except REQ_PREFLUSH.
@@ -2859,6 +2884,7 @@ static const struct block_device_operations dm_blk_dops = {
 
 static const struct dax_operations dm_dax_ops = {
 	.direct_access = dm_dax_direct_access,
+	.copy_from_iter = dm_dax_copy_from_iter,
 };
 
 /*
diff --git a/include/linux/dax.h b/include/linux/dax.h
index bbe79ed90e2b..28e398f8c59e 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -78,6 +78,8 @@ void kill_dax(struct dax_device *dax_dev);
 void *dax_get_private(struct dax_device *dax_dev);
 long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
 		void **kaddr, pfn_t *pfn);
+size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
+		size_t bytes, struct iov_iter *i);
 
 /*
  * We use lowest available bit in exceptional entry for locking, one bit for
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index f4c639c0c362..11c8a0a92f9c 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -132,6 +132,8 @@ typedef int (*dm_busy_fn) (struct dm_target *ti);
  */
 typedef long (*dm_dax_direct_access_fn) (struct dm_target *ti, pgoff_t pgoff,
 		long nr_pages, void **kaddr, pfn_t *pfn);
+typedef size_t (*dm_dax_copy_from_iter_fn)(struct dm_target *ti, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i);
 #define PAGE_SECTORS (PAGE_SIZE / 512)
 
 void dm_error(const char *message);
@@ -181,6 +183,7 @@ struct target_type {
 	dm_iterate_devices_fn iterate_devices;
 	dm_io_hints_fn io_hints;
 	dm_dax_direct_access_fn direct_access;
+	dm_dax_copy_from_iter_fn dax_copy_from_iter;
 
 	/* For internal device-mapper use. */
 	struct list_head list;

[PATCH v4 03/16] filesystem-dax: convert to dax_copy_from_iter()

[Dan Williams]

Now that all possible providers of the dax_operations copy_from_iter
method are implemented, switch filesytem-dax to call the driver rather
than copy_to_iter_pmem.

Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/pmem.h |   50 -------------------------------------------
 fs/dax.c                    |    3 ++-
 include/linux/pmem.h        |   24 ---------------------
 3 files changed, 2 insertions(+), 75 deletions(-)

diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index 0ff8fe71b255..60e8edbe0205 100644
--- a/arch/x86/include/asm/pmem.h
+++ b/arch/x86/include/asm/pmem.h
@@ -66,56 +66,6 @@ static inline void arch_wb_cache_pmem(void *addr, size_t size)
 }
 
 /**
- * arch_copy_from_iter_pmem - copy data from an iterator to PMEM
- * @addr:	PMEM destination address
- * @bytes:	number of bytes to copy
- * @i:		iterator with source data
- *
- * Copy data from the iterator 'i' to the PMEM buffer starting at 'addr'.
- */
-static inline size_t arch_copy_from_iter_pmem(void *addr, size_t bytes,
-		struct iov_iter *i)
-{
-	size_t len;
-
-	/* TODO: skip the write-back by always using non-temporal stores */
-	len = copy_from_iter_nocache(addr, bytes, i);
-
-	/*
-	 * In the iovec case on x86_64 copy_from_iter_nocache() uses
-	 * non-temporal stores for the bulk of the transfer, but we need
-	 * to manually flush if the transfer is unaligned. A cached
-	 * memory copy is used when destination or size is not naturally
-	 * aligned. That is:
-	 *   - Require 8-byte alignment when size is 8 bytes or larger.
-	 *   - Require 4-byte alignment when size is 4 bytes.
-	 *
-	 * In the non-iovec case the entire destination needs to be
-	 * flushed.
-	 */
-	if (iter_is_iovec(i)) {
-		unsigned long flushed, dest = (unsigned long) addr;
-
-		if (bytes < 8) {
-			if (!IS_ALIGNED(dest, 4) || (bytes != 4))
-				arch_wb_cache_pmem(addr, bytes);
-		} else {
-			if (!IS_ALIGNED(dest, 8)) {
-				dest = ALIGN(dest, boot_cpu_data.x86_clflush_size);
-				arch_wb_cache_pmem(addr, 1);
-			}
-
-			flushed = dest - (unsigned long) addr;
-			if (bytes > flushed && !IS_ALIGNED(bytes - flushed, 8))
-				arch_wb_cache_pmem(addr + bytes - 1, 1);
-		}
-	} else
-		arch_wb_cache_pmem(addr, bytes);
-
-	return len;
-}
-
-/**
  * arch_clear_pmem - zero a PMEM memory range
  * @addr:	virtual start address
  * @size:	number of bytes to zero
diff --git a/fs/dax.c b/fs/dax.c
index 2a6889b3585f..b459948de427 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1054,7 +1054,8 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 			map_len = end - pos;
 
 		if (iov_iter_rw(iter) == WRITE)
-			map_len = copy_from_iter_pmem(kaddr, map_len, iter);
+			map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr,
+					map_len, iter);
 		else
 			map_len = copy_to_iter(kaddr, map_len, iter);
 		if (map_len <= 0) {
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
index 71ecf3d46aac..9d542a5600e4 100644
--- a/include/linux/pmem.h
+++ b/include/linux/pmem.h
@@ -31,13 +31,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 	BUG();
 }
 
-static inline size_t arch_copy_from_iter_pmem(void *addr, size_t bytes,
-		struct iov_iter *i)
-{
-	BUG();
-	return 0;
-}
-
 static inline void arch_clear_pmem(void *addr, size_t size)
 {
 	BUG();
@@ -80,23 +73,6 @@ static inline void memcpy_to_pmem(void *dst, const void *src, size_t n)
 }
 
 /**
- * copy_from_iter_pmem - copy data from an iterator to PMEM
- * @addr:	PMEM destination address
- * @bytes:	number of bytes to copy
- * @i:		iterator with source data
- *
- * Copy data from the iterator 'i' to the PMEM buffer starting at 'addr'.
- * See blkdev_issue_flush() note for memcpy_to_pmem().
- */
-static inline size_t copy_from_iter_pmem(void *addr, size_t bytes,
-		struct iov_iter *i)
-{
-	if (arch_has_pmem_api())
-		return arch_copy_from_iter_pmem(addr, bytes, i);
-	return copy_from_iter_nocache(addr, bytes, i);
-}
-
-/**
  * clear_pmem - zero a PMEM memory range
  * @addr:	virtual start address
  * @size:	number of bytes to zero

[PATCH v4 04/16] dax, pmem: introduce an optional 'flush' dax_operation

[Dan Williams]

Filesystem-DAX flushes caches whenever it writes to the address returned
through dax_direct_access() and when writing back dirty radix entries.
That flushing is only required in the pmem case, so add a dax operation
to allow pmem to take this extra action, but skip it for other dax
capable devices that do not provide a flush routine.

An example for this differentiation might be a volatile ram disk where
there is no expectation of persistence. In fact the pmem driver itself might
front such an address range specified by the NFIT. So, this "no flush"
property might be something passed down by the bus / libnvdimm.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/pmem.c |    7 +++++++
 include/linux/dax.h   |    2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 2f3aefe565c6..823b07774244 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -242,9 +242,16 @@ static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
 	return copy_from_iter_flushcache(addr, bytes, i);
 }
 
+static void pmem_dax_flush(struct dax_device *dax_dev, pgoff_t pgoff,
+		void *addr, size_t size)
+{
+	wb_cache_pmem(addr, size);
+}
+
 static const struct dax_operations pmem_dax_ops = {
 	.direct_access = pmem_dax_direct_access,
 	.copy_from_iter = pmem_copy_from_iter,
+	.flush = pmem_dax_flush,
 };
 
 static void pmem_release_queue(void *q)
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 28e398f8c59e..407dd3ff6e54 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -19,6 +19,8 @@ struct dax_operations {
 	/* copy_from_iter: dax-driver override for default copy_from_iter */
 	size_t (*copy_from_iter)(struct dax_device *, pgoff_t, void *, size_t,
 			struct iov_iter *);
+	/* flush: optional driver-specific cache management after writes */
+	void (*flush)(struct dax_device *, pgoff_t, void *, size_t);
 };
 
 #if IS_ENABLED(CONFIG_DAX)

[PATCH v4 05/16] dm: add ->flush() dax operation support

[Dan Williams]

Allow device-mapper to route flush operations to the
per-target implementation. In order for the device stacking to work we
need a dax_dev and a pgoff relative to that device. This gives each
layer of the stack the information it needs to look up the operation
pointer for the next level.

This conceptually allows for an array of mixed device drivers with
varying flush implementations.

Reviewed-by: Toshi Kani <toshi.kani@hpe.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c           |   11 +++++++++++
 drivers/md/dm-linear.c        |   15 +++++++++++++++
 drivers/md/dm-stripe.c        |   20 ++++++++++++++++++++
 drivers/md/dm.c               |   19 +++++++++++++++++++
 include/linux/dax.h           |    2 ++
 include/linux/device-mapper.h |    3 +++
 6 files changed, 70 insertions(+)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index dd299e55f65d..b7729e4d351a 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -185,6 +185,17 @@ size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 }
 EXPORT_SYMBOL_GPL(dax_copy_from_iter);
 
+void dax_flush(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
+		size_t size)
+{
+	if (!dax_alive(dax_dev))
+		return;
+
+	if (dax_dev->ops->flush)
+		dax_dev->ops->flush(dax_dev, pgoff, addr, size);
+}
+EXPORT_SYMBOL_GPL(dax_flush);
+
 bool dax_alive(struct dax_device *dax_dev)
 {
 	lockdep_assert_held(&dax_srcu);
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 0841ec1bfbad..25e661974319 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -173,6 +173,20 @@ static size_t linear_dax_copy_from_iter(struct dm_target *ti, pgoff_t pgoff,
 	return dax_copy_from_iter(dax_dev, pgoff, addr, bytes, i);
 }
 
+static void linear_dax_flush(struct dm_target *ti, pgoff_t pgoff, void *addr,
+		size_t size)
+{
+	struct linear_c *lc = ti->private;
+	struct block_device *bdev = lc->dev->bdev;
+	struct dax_device *dax_dev = lc->dev->dax_dev;
+	sector_t dev_sector, sector = pgoff * PAGE_SECTORS;
+
+	dev_sector = linear_map_sector(ti, sector);
+	if (bdev_dax_pgoff(bdev, dev_sector, ALIGN(size, PAGE_SIZE), &pgoff))
+		return;
+	dax_flush(dax_dev, pgoff, addr, size);
+}
+
 static struct target_type linear_target = {
 	.name   = "linear",
 	.version = {1, 3, 0},
@@ -186,6 +200,7 @@ static struct target_type linear_target = {
 	.iterate_devices = linear_iterate_devices,
 	.direct_access = linear_dax_direct_access,
 	.dax_copy_from_iter = linear_dax_copy_from_iter,
+	.dax_flush = linear_dax_flush,
 };
 
 int __init dm_linear_init(void)
diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index 1ef914f9ca72..8e73517967b6 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -351,6 +351,25 @@ static size_t stripe_dax_copy_from_iter(struct dm_target *ti, pgoff_t pgoff,
 	return dax_copy_from_iter(dax_dev, pgoff, addr, bytes, i);
 }
 
+static void stripe_dax_flush(struct dm_target *ti, pgoff_t pgoff, void *addr,
+		size_t size)
+{
+	sector_t dev_sector, sector = pgoff * PAGE_SECTORS;
+	struct stripe_c *sc = ti->private;
+	struct dax_device *dax_dev;
+	struct block_device *bdev;
+	uint32_t stripe;
+
+	stripe_map_sector(sc, sector, &stripe, &dev_sector);
+	dev_sector += sc->stripe[stripe].physical_start;
+	dax_dev = sc->stripe[stripe].dev->dax_dev;
+	bdev = sc->stripe[stripe].dev->bdev;
+
+	if (bdev_dax_pgoff(bdev, dev_sector, ALIGN(size, PAGE_SIZE), &pgoff))
+		return;
+	dax_flush(dax_dev, pgoff, addr, size);
+}
+
 /*
  * Stripe status:
  *
@@ -471,6 +490,7 @@ static struct target_type stripe_target = {
 	.io_hints = stripe_io_hints,
 	.direct_access = stripe_dax_direct_access,
 	.dax_copy_from_iter = stripe_dax_copy_from_iter,
+	.dax_flush = stripe_dax_flush,
 };
 
 int __init dm_stripe_init(void)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7faaceb52819..09b3efdc8abf 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -994,6 +994,24 @@ static size_t dm_dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
 	return ret;
 }
 
+static void dm_dax_flush(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
+		size_t size)
+{
+	struct mapped_device *md = dax_get_private(dax_dev);
+	sector_t sector = pgoff * PAGE_SECTORS;
+	struct dm_target *ti;
+	int srcu_idx;
+
+	ti = dm_dax_get_live_target(md, sector, &srcu_idx);
+
+	if (!ti)
+		goto out;
+	if (ti->type->dax_flush)
+		ti->type->dax_flush(ti, pgoff, addr, size);
+ out:
+	dm_put_live_table(md, srcu_idx);
+}
+
 /*
  * A target may call dm_accept_partial_bio only from the map routine.  It is
  * allowed for all bio types except REQ_PREFLUSH.
@@ -2885,6 +2903,7 @@ static const struct block_device_operations dm_blk_dops = {
 static const struct dax_operations dm_dax_ops = {
 	.direct_access = dm_dax_direct_access,
 	.copy_from_iter = dm_dax_copy_from_iter,
+	.flush = dm_dax_flush,
 };
 
 /*
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 407dd3ff6e54..1f6b6072af64 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -82,6 +82,8 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
 		void **kaddr, pfn_t *pfn);
 size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 		size_t bytes, struct iov_iter *i);
+void dax_flush(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
+		size_t size);
 
 /*
  * We use lowest available bit in exceptional entry for locking, one bit for
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 11c8a0a92f9c..67bfe8ddcb32 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -134,6 +134,8 @@ typedef long (*dm_dax_direct_access_fn) (struct dm_target *ti, pgoff_t pgoff,
 		long nr_pages, void **kaddr, pfn_t *pfn);
 typedef size_t (*dm_dax_copy_from_iter_fn)(struct dm_target *ti, pgoff_t pgoff,
 		void *addr, size_t bytes, struct iov_iter *i);
+typedef void (*dm_dax_flush_fn)(struct dm_target *ti, pgoff_t pgoff, void *addr,
+		size_t size);
 #define PAGE_SECTORS (PAGE_SIZE / 512)
 
 void dm_error(const char *message);
@@ -184,6 +186,7 @@ struct target_type {
 	dm_io_hints_fn io_hints;
 	dm_dax_direct_access_fn direct_access;
 	dm_dax_copy_from_iter_fn dax_copy_from_iter;
+	dm_dax_flush_fn dax_flush;
 
 	/* For internal device-mapper use. */
 	struct list_head list;

[PATCH v4 06/16] filesystem-dax: convert to dax_flush()

[Dan Williams]

Filesystem-DAX flushes caches whenever it writes to the address returned
through dax_direct_access() and when writing back dirty radix entries.
That flushing is only required in the pmem case, so the dax_flush()
helper skips cache management work when the underlying driver does not
specify a flush method.

We still do all the dirty tracking since the radix entry will already be
there for locking purposes. However, the work to clean the entry will be
a nop for some dax drivers.

Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/dax.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dax.c b/fs/dax.c
index b459948de427..0933fc460ada 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -784,7 +784,7 @@ static int dax_writeback_one(struct block_device *bdev,
 	}
 
 	dax_mapping_entry_mkclean(mapping, index, pfn_t_to_pfn(pfn));
-	wb_cache_pmem(kaddr, size);
+	dax_flush(dax_dev, pgoff, kaddr, size);
 	/*
 	 * After we have flushed the cache, we can clear the dirty tag. There
 	 * cannot be new dirty data in the pfn after the flush has completed as

[PATCH v4 07/16] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush

[Dan Williams]

The clear_pmem() helper simply combines a memset() plus a cache flush.
Now that the flush routine is optionally provided by the dax device
driver we can avoid unnecessary cache management on dax devices fronting
volatile memory.

With clear_pmem() gone we can follow on with a patch to make pmem cache
management completely defined within the pmem driver.

Cc: <x86@kernel.org>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/pmem.h |   13 -------------
 fs/dax.c                    |    3 ++-
 include/linux/pmem.h        |   21 ---------------------
 3 files changed, 2 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index 60e8edbe0205..f4c119d253f3 100644
--- a/arch/x86/include/asm/pmem.h
+++ b/arch/x86/include/asm/pmem.h
@@ -65,19 +65,6 @@ static inline void arch_wb_cache_pmem(void *addr, size_t size)
 		clwb(p);
 }
 
-/**
- * arch_clear_pmem - zero a PMEM memory range
- * @addr:	virtual start address
- * @size:	number of bytes to zero
- *
- * Write zeros into the memory range starting at 'addr' for 'size' bytes.
- */
-static inline void arch_clear_pmem(void *addr, size_t size)
-{
-	memset(addr, 0, size);
-	arch_wb_cache_pmem(addr, size);
-}
-
 static inline void arch_invalidate_pmem(void *addr, size_t size)
 {
 	clflush_cache_range(addr, size);
diff --git a/fs/dax.c b/fs/dax.c
index 0933fc460ada..554b8e7d921c 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -975,7 +975,8 @@ int __dax_zero_page_range(struct block_device *bdev,
 			dax_read_unlock(id);
 			return rc;
 		}
-		clear_pmem(kaddr + offset, size);
+		memset(kaddr + offset, 0, size);
+		dax_flush(dax_dev, pgoff, kaddr + offset, size);
 		dax_read_unlock(id);
 	}
 	return 0;
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
index 9d542a5600e4..772bd02a5b52 100644
--- a/include/linux/pmem.h
+++ b/include/linux/pmem.h
@@ -31,11 +31,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 	BUG();
 }
 
-static inline void arch_clear_pmem(void *addr, size_t size)
-{
-	BUG();
-}
-
 static inline void arch_wb_cache_pmem(void *addr, size_t size)
 {
 	BUG();
@@ -73,22 +68,6 @@ static inline void memcpy_to_pmem(void *dst, const void *src, size_t n)
 }
 
 /**
- * clear_pmem - zero a PMEM memory range
- * @addr:	virtual start address
- * @size:	number of bytes to zero
- *
- * Write zeros into the memory range starting at 'addr' for 'size' bytes.
- * See blkdev_issue_flush() note for memcpy_to_pmem().
- */
-static inline void clear_pmem(void *addr, size_t size)
-{
-	if (arch_has_pmem_api())
-		arch_clear_pmem(addr, size);
-	else
-		memset(addr, 0, size);
-}
-
-/**
  * invalidate_pmem - flush a pmem range from the cache hierarchy
  * @addr:	virtual start address
  * @size:	bytes to invalidate (internally aligned to cache line size)

[PATCH v4 08/16] x86, dax, libnvdimm: remove wb_cache_pmem() indirection

[Dan Williams]

With all handling of the CONFIG_ARCH_HAS_PMEM_API case being moved to
libnvdimm and the pmem driver directly we do not need to provide global
wrappers and fallbacks in the CONFIG_ARCH_HAS_PMEM_API=n case. The pmem
driver will simply not link to arch_wb_cache_pmem() in that case.  Same
as before, pmem flushing is only defined for x86_64, via
clean_cache_range(), but it is straightforward to add other archs in the
future.

arch_wb_cache_pmem() is an exported function since the pmem module needs
to find it, but it is privately declared in drivers/nvdimm/pmem.h because
there are no consumers outside of the pmem driver.

Cc: <x86@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/pmem.h |   21 ---------------------
 arch/x86/lib/usercopy_64.c  |    6 ++++++
 drivers/nvdimm/pmem.c       |    2 +-
 drivers/nvdimm/pmem.h       |    8 ++++++++
 include/linux/pmem.h        |   19 -------------------
 5 files changed, 15 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index f4c119d253f3..4759a179aa52 100644
--- a/arch/x86/include/asm/pmem.h
+++ b/arch/x86/include/asm/pmem.h
@@ -44,27 +44,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 		BUG();
 }
 
-/**
- * arch_wb_cache_pmem - write back a cache range with CLWB
- * @vaddr:	virtual start address
- * @size:	number of bytes to write back
- *
- * Write back a cache range using the CLWB (cache line write back)
- * instruction. Note that @size is internally rounded up to be cache
- * line size aligned.
- */
-static inline void arch_wb_cache_pmem(void *addr, size_t size)
-{
-	u16 x86_clflush_size = boot_cpu_data.x86_clflush_size;
-	unsigned long clflush_mask = x86_clflush_size - 1;
-	void *vend = addr + size;
-	void *p;
-
-	for (p = (void *)((unsigned long)addr & ~clflush_mask);
-	     p < vend; p += x86_clflush_size)
-		clwb(p);
-}
-
 static inline void arch_invalidate_pmem(void *addr, size_t size)
 {
 	clflush_cache_range(addr, size);
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index f42d2fd86ca3..75d3776123cc 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -97,6 +97,12 @@ static void clean_cache_range(void *addr, size_t size)
 		clwb(p);
 }
 
+void arch_wb_cache_pmem(void *addr, size_t size)
+{
+	clean_cache_range(addr, size);
+}
+EXPORT_SYMBOL_GPL(arch_wb_cache_pmem);
+
 long __copy_user_flushcache(void *dst, const void __user *src, unsigned size)
 {
 	unsigned long flushed, dest = (unsigned long) dst;
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 823b07774244..3b87702d46bb 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -245,7 +245,7 @@ static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
 static void pmem_dax_flush(struct dax_device *dax_dev, pgoff_t pgoff,
 		void *addr, size_t size)
 {
-	wb_cache_pmem(addr, size);
+	arch_wb_cache_pmem(addr, size);
 }
 
 static const struct dax_operations pmem_dax_ops = {
diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h
index 7f4dbd72a90a..c4b3371c7f88 100644
--- a/drivers/nvdimm/pmem.h
+++ b/drivers/nvdimm/pmem.h
@@ -5,6 +5,14 @@
 #include <linux/pfn_t.h>
 #include <linux/fs.h>
 
+#ifdef CONFIG_ARCH_HAS_PMEM_API
+void arch_wb_cache_pmem(void *addr, size_t size);
+#else
+static inline void arch_wb_cache_pmem(void *addr, size_t size)
+{
+}
+#endif
+
 /* this definition is in it's own header for tools/testing/nvdimm to consume */
 struct pmem_device {
 	/* One contiguous memory region per device */
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
index 772bd02a5b52..33ae761f010a 100644
--- a/include/linux/pmem.h
+++ b/include/linux/pmem.h
@@ -31,11 +31,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 	BUG();
 }
 
-static inline void arch_wb_cache_pmem(void *addr, size_t size)
-{
-	BUG();
-}
-
 static inline void arch_invalidate_pmem(void *addr, size_t size)
 {
 	BUG();
@@ -80,18 +75,4 @@ static inline void invalidate_pmem(void *addr, size_t size)
 	if (arch_has_pmem_api())
 		arch_invalidate_pmem(addr, size);
 }
-
-/**
- * wb_cache_pmem - write back processor cache for PMEM memory range
- * @addr:	virtual start address
- * @size:	number of bytes to write back
- *
- * Write back the processor cache range starting at 'addr' for 'size' bytes.
- * See blkdev_issue_flush() note for memcpy_to_pmem().
- */
-static inline void wb_cache_pmem(void *addr, size_t size)
-{
-	if (arch_has_pmem_api())
-		arch_wb_cache_pmem(addr, size);
-}
 #endif /* __PMEM_H__ */

[PATCH v4 09/16] x86, libnvdimm, pmem: move arch_invalidate_pmem() to libnvdimm

[Dan Williams]

Kill this globally defined wrapper and move to libnvdimm so that we can
ultimately remove include/linux/pmem.h and asm/pmem.h.

Cc: <x86@kernel.org>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/pmem.h |    5 -----
 arch/x86/mm/pageattr.c      |    6 ++++++
 drivers/nvdimm/claim.c      |    3 ++-
 drivers/nvdimm/pmem.c       |    2 +-
 drivers/nvdimm/pmem.h       |    4 ++++
 include/linux/pmem.h        |   19 -------------------
 6 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index 4759a179aa52..b61a25a895a7 100644
--- a/arch/x86/include/asm/pmem.h
+++ b/arch/x86/include/asm/pmem.h
@@ -43,10 +43,5 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 				__func__, dst, src, rem))
 		BUG();
 }
-
-static inline void arch_invalidate_pmem(void *addr, size_t size)
-{
-	clflush_cache_range(addr, size);
-}
 #endif /* CONFIG_ARCH_HAS_PMEM_API */
 #endif /* __ASM_X86_PMEM_H__ */
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index c8520b2c62d2..757b0bcdf712 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -150,6 +150,12 @@ void clflush_cache_range(void *vaddr, unsigned int size)
 }
 EXPORT_SYMBOL_GPL(clflush_cache_range);
 
+void arch_invalidate_pmem(void *addr, size_t size)
+{
+	clflush_cache_range(addr, size);
+}
+EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
+
 static void __cpa_flush_all(void *arg)
 {
 	unsigned long cache = (unsigned long)arg;
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index b8b9c8ca7862..d2e16c0401df 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -14,6 +14,7 @@
 #include <linux/sizes.h>
 #include <linux/pmem.h>
 #include "nd-core.h"
+#include "pmem.h"
 #include "pfn.h"
 #include "btt.h"
 #include "nd.h"
@@ -272,7 +273,7 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns,
 				cleared /= 512;
 				badblocks_clear(&nsio->bb, sector, cleared);
 			}
-			invalidate_pmem(nsio->addr + offset, size);
+			arch_invalidate_pmem(nsio->addr + offset, size);
 		} else
 			rc = -EIO;
 	}
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 3b87702d46bb..68737bc68a07 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -71,7 +71,7 @@ static int pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
 		badblocks_clear(&pmem->bb, sector, cleared);
 	}
 
-	invalidate_pmem(pmem->virt_addr + offset, len);
+	arch_invalidate_pmem(pmem->virt_addr + offset, len);
 
 	return rc;
 }
diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h
index c4b3371c7f88..00005900c1b7 100644
--- a/drivers/nvdimm/pmem.h
+++ b/drivers/nvdimm/pmem.h
@@ -7,10 +7,14 @@
 
 #ifdef CONFIG_ARCH_HAS_PMEM_API
 void arch_wb_cache_pmem(void *addr, size_t size);
+void arch_invalidate_pmem(void *addr, size_t size);
 #else
 static inline void arch_wb_cache_pmem(void *addr, size_t size)
 {
 }
+static inline void arch_invalidate_pmem(void *addr, size_t size)
+{
+}
 #endif
 
 /* this definition is in it's own header for tools/testing/nvdimm to consume */
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
index 33ae761f010a..559c00848583 100644
--- a/include/linux/pmem.h
+++ b/include/linux/pmem.h
@@ -30,11 +30,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
 {
 	BUG();
 }
-
-static inline void arch_invalidate_pmem(void *addr, size_t size)
-{
-	BUG();
-}
 #endif
 
 static inline bool arch_has_pmem_api(void)
@@ -61,18 +56,4 @@ static inline void memcpy_to_pmem(void *dst, const void *src, size_t n)
 	else
 		memcpy(dst, src, n);
 }
-
-/**
- * invalidate_pmem - flush a pmem range from the cache hierarchy
- * @addr:	virtual start address
- * @size:	bytes to invalidate (internally aligned to cache line size)
- *
- * For platforms that support clearing poison this flushes any poisoned
- * ranges out of the cache
- */
-static inline void invalidate_pmem(void *addr, size_t size)
-{
-	if (arch_has_pmem_api())
-		arch_invalidate_pmem(addr, size);
-}
 #endif /* __PMEM_H__ */

[PATCH v4 10/16] x86, libnvdimm, pmem: remove global pmem api

[Dan Williams]

Now that all callers of the pmem api have been converted to dax helpers that
call back to the pmem driver, we can remove include/linux/pmem.h and
asm/pmem.h.

Cc: <x86@kernel.org>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 MAINTAINERS                     |    4 +--
 arch/x86/include/asm/pmem.h     |   47 -------------------------------
 drivers/acpi/nfit/core.c        |    3 +-
 drivers/nvdimm/claim.c          |    1 -
 drivers/nvdimm/dimm_devs.c      |    8 +++++
 drivers/nvdimm/namespace_devs.c |    6 +---
 drivers/nvdimm/pmem.c           |    1 -
 drivers/nvdimm/pmem.h           |    2 +
 drivers/nvdimm/region_devs.c    |    1 -
 fs/dax.c                        |    1 -
 include/linux/libnvdimm.h       |    1 +
 include/linux/pmem.h            |   59 ---------------------------------------
 12 files changed, 14 insertions(+), 120 deletions(-)
 delete mode 100644 arch/x86/include/asm/pmem.h
 delete mode 100644 include/linux/pmem.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a28acd7f525..1636ce420251 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7592,9 +7592,7 @@ M:	Ross Zwisler <ross.zwisler@linux.intel.com>
 L:	linux-nvdimm@lists.01.org
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
-F:	drivers/nvdimm/pmem.c
-F:	include/linux/pmem.h
-F:	arch/*/include/asm/pmem.h
+F:	drivers/nvdimm/pmem*
 
 LIGHTNVM PLATFORM SUPPORT
 M:	Matias Bjorling <mb@lightnvm.io>
diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
deleted file mode 100644
index b61a25a895a7..000000000000
--- a/arch/x86/include/asm/pmem.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Copyright(c) 2015 Intel Corporation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- */
-#ifndef __ASM_X86_PMEM_H__
-#define __ASM_X86_PMEM_H__
-
-#include <linux/uaccess.h>
-#include <asm/cacheflush.h>
-#include <asm/cpufeature.h>
-#include <asm/special_insns.h>
-
-#ifdef CONFIG_ARCH_HAS_PMEM_API
-/**
- * arch_memcpy_to_pmem - copy data to persistent memory
- * @dst: destination buffer for the copy
- * @src: source buffer for the copy
- * @n: length of the copy in bytes
- *
- * Copy data to persistent memory media via non-temporal stores so that
- * a subsequent pmem driver flush operation will drain posted write queues.
- */
-static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
-{
-	int rem;
-
-	/*
-	 * We are copying between two kernel buffers, if
-	 * __copy_from_user_inatomic_nocache() returns an error (page
-	 * fault) we would have already reported a general protection fault
-	 * before the WARN+BUG.
-	 */
-	rem = __copy_from_user_inatomic_nocache(dst, (void __user *) src, n);
-	if (WARN(rem, "%s: fault copying %p <- %p unwritten: %d\n",
-				__func__, dst, src, rem))
-		BUG();
-}
-#endif /* CONFIG_ARCH_HAS_PMEM_API */
-#endif /* __ASM_X86_PMEM_H__ */
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index cbd5596e7562..ac2436538b7e 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -20,7 +20,6 @@
 #include <linux/list.h>
 #include <linux/acpi.h>
 #include <linux/sort.h>
-#include <linux/pmem.h>
 #include <linux/io.h>
 #include <linux/nd.h>
 #include <asm/cacheflush.h>
@@ -1956,7 +1955,7 @@ static int acpi_nfit_blk_region_enable(struct nvdimm_bus *nvdimm_bus,
 	nfit_blk->bdw_offset = nfit_mem->bdw->offset;
 	mmio = &nfit_blk->mmio[BDW];
 	mmio->addr.base = devm_nvdimm_memremap(dev, nfit_mem->spa_bdw->address,
-                        nfit_mem->spa_bdw->length, ARCH_MEMREMAP_PMEM);
+                        nfit_mem->spa_bdw->length, nd_blk_memremap_flags(ndbr));
 	if (!mmio->addr.base) {
 		dev_dbg(dev, "%s: %s failed to map bdw\n", __func__,
 				nvdimm_name(nvdimm));
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index d2e16c0401df..3beedf173902 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -12,7 +12,6 @@
  */
 #include <linux/device.h>
 #include <linux/sizes.h>
-#include <linux/pmem.h>
 #include "nd-core.h"
 #include "pmem.h"
 #include "pfn.h"
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 9852a3355509..6a1e7a3c0c17 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -20,6 +20,7 @@
 #include <linux/mm.h>
 #include "nd-core.h"
 #include "label.h"
+#include "pmem.h"
 #include "nd.h"
 
 static DEFINE_IDA(dimm_ida);
@@ -235,6 +236,13 @@ struct nvdimm *nd_blk_region_to_dimm(struct nd_blk_region *ndbr)
 }
 EXPORT_SYMBOL_GPL(nd_blk_region_to_dimm);
 
+unsigned long nd_blk_memremap_flags(struct nd_blk_region *ndbr)
+{
+	/* pmem mapping properties are private to libnvdimm */
+	return ARCH_MEMREMAP_PMEM;
+}
+EXPORT_SYMBOL_GPL(nd_blk_memremap_flags);
+
 struct nvdimm_drvdata *to_ndd(struct nd_mapping *nd_mapping)
 {
 	struct nvdimm *nvdimm = nd_mapping->nvdimm;
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 2f9dfbd2dbec..4e9261ef8a95 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -14,10 +14,10 @@
 #include <linux/device.h>
 #include <linux/sort.h>
 #include <linux/slab.h>
-#include <linux/pmem.h>
 #include <linux/list.h>
 #include <linux/nd.h>
 #include "nd-core.h"
+#include "pmem.h"
 #include "nd.h"
 
 static void namespace_io_release(struct device *dev)
@@ -155,11 +155,7 @@ bool pmem_should_map_pages(struct device *dev)
 				IORES_DESC_NONE) == REGION_MIXED)
 		return false;
 
-#ifdef ARCH_MEMREMAP_PMEM
 	return ARCH_MEMREMAP_PMEM == MEMREMAP_WB;
-#else
-	return false;
-#endif
 }
 EXPORT_SYMBOL(pmem_should_map_pages);
 
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 68737bc68a07..06f6c27ec1e9 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -28,7 +28,6 @@
 #include <linux/blk-mq.h>
 #include <linux/pfn_t.h>
 #include <linux/slab.h>
-#include <linux/pmem.h>
 #include <linux/uio.h>
 #include <linux/dax.h>
 #include <linux/nd.h>
diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h
index 00005900c1b7..fce248a1fc87 100644
--- a/drivers/nvdimm/pmem.h
+++ b/drivers/nvdimm/pmem.h
@@ -6,9 +6,11 @@
 #include <linux/fs.h>
 
 #ifdef CONFIG_ARCH_HAS_PMEM_API
+#define ARCH_MEMREMAP_PMEM MEMREMAP_WB
 void arch_wb_cache_pmem(void *addr, size_t size);
 void arch_invalidate_pmem(void *addr, size_t size);
 #else
+#define ARCH_MEMREMAP_PMEM MEMREMAP_WT
 static inline void arch_wb_cache_pmem(void *addr, size_t size)
 {
 }
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 985b0e11bd73..3c06a6ea6958 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -15,7 +15,6 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/hash.h>
-#include <linux/pmem.h>
 #include <linux/sort.h>
 #include <linux/io.h>
 #include <linux/nd.h>
diff --git a/fs/dax.c b/fs/dax.c
index 554b8e7d921c..6d8699feae2e 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -25,7 +25,6 @@
 #include <linux/mm.h>
 #include <linux/mutex.h>
 #include <linux/pagevec.h>
-#include <linux/pmem.h>
 #include <linux/sched.h>
 #include <linux/sched/signal.h>
 #include <linux/uio.h>
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 6c807017128d..b2f659bd661d 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -159,6 +159,7 @@ void *nd_region_provider_data(struct nd_region *nd_region);
 void *nd_blk_region_provider_data(struct nd_blk_region *ndbr);
 void nd_blk_region_set_provider_data(struct nd_blk_region *ndbr, void *data);
 struct nvdimm *nd_blk_region_to_dimm(struct nd_blk_region *ndbr);
+unsigned long nd_blk_memremap_flags(struct nd_blk_region *ndbr);
 unsigned int nd_region_acquire_lane(struct nd_region *nd_region);
 void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane);
 u64 nd_fletcher64(void *addr, size_t len, bool le);
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
deleted file mode 100644
index 559c00848583..000000000000
--- a/include/linux/pmem.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright(c) 2015 Intel Corporation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- */
-#ifndef __PMEM_H__
-#define __PMEM_H__
-
-#include <linux/io.h>
-#include <linux/uio.h>
-
-#ifdef CONFIG_ARCH_HAS_PMEM_API
-#define ARCH_MEMREMAP_PMEM MEMREMAP_WB
-#include <asm/pmem.h>
-#else
-#define ARCH_MEMREMAP_PMEM MEMREMAP_WT
-/*
- * These are simply here to enable compilation, all call sites gate
- * calling these symbols with arch_has_pmem_api() and redirect to the
- * implementation in asm/pmem.h.
- */
-static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n)
-{
-	BUG();
-}
-#endif
-
-static inline bool arch_has_pmem_api(void)
-{
-	return IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API);
-}
-
-/**
- * memcpy_to_pmem - copy data to persistent memory
- * @dst: destination buffer for the copy
- * @src: source buffer for the copy
- * @n: length of the copy in bytes
- *
- * Perform a memory copy that results in the destination of the copy
- * being effectively evicted from, or never written to, the processor
- * cache hierarchy after the copy completes.  After memcpy_to_pmem()
- * data may still reside in cpu or platform buffers, so this operation
- * must be followed by a blkdev_issue_flush() on the pmem block device.
- */
-static inline void memcpy_to_pmem(void *dst, const void *src, size_t n)
-{
-	if (arch_has_pmem_api())
-		arch_memcpy_to_pmem(dst, src, n);
-	else
-		memcpy(dst, src, n);
-}
-#endif /* __PMEM_H__ */

[PATCH v4 11/16] libnvdimm, pmem: fix persistence warning

[Dan Williams]

The pmem driver assumes if platform firmware describes the memory
devices associated with a persistent memory range and
CONFIG_ARCH_HAS_PMEM_API=y that it has all the mechanism necessary to
flush data to a power-fail safe zone. We warn if the firmware does not
describe memory devices, but we also need to warn if the architecture
does not claim pmem support.

Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/region_devs.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 3c06a6ea6958..41b4cdf5dea8 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1037,8 +1037,9 @@ int nvdimm_has_flush(struct nd_region *nd_region)
 {
 	int i;
 
-	/* no nvdimm == flushing capability unknown */
-	if (nd_region->ndr_mappings == 0)
+	/* no nvdimm or pmem api == flushing capability unknown */
+	if (nd_region->ndr_mappings == 0
+			|| !IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API))
 		return -ENXIO;
 
 	for (i = 0; i < nd_region->ndr_mappings; i++) {

[PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

Allow volatile nfit ranges to participate in all the same infrastructure
provided for persistent memory regions. A resulting resulting namespace
device will still be called "pmem", but the parent region type will be
"nd_volatile". This is in preparation for disabling the dax ->flush()
operation in the pmem driver when it is hosted on a volatile range.

Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/core.c        |    9 ++++++++-
 drivers/nvdimm/bus.c            |    8 ++++----
 drivers/nvdimm/core.c           |    2 +-
 drivers/nvdimm/dax_devs.c       |    2 +-
 drivers/nvdimm/dimm_devs.c      |    2 +-
 drivers/nvdimm/namespace_devs.c |    8 ++++----
 drivers/nvdimm/nd-core.h        |    9 +++++++++
 drivers/nvdimm/pfn_devs.c       |    4 ++--
 drivers/nvdimm/region_devs.c    |   27 ++++++++++++++-------------
 9 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index ac2436538b7e..60d1ca149cc1 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2227,6 +2227,13 @@ static bool nfit_spa_is_virtual(struct acpi_nfit_system_address *spa)
 		nfit_spa_type(spa) == NFIT_SPA_PCD);
 }
 
+static bool nfit_spa_is_volatile(struct acpi_nfit_system_address *spa)
+{
+	return (nfit_spa_type(spa) == NFIT_SPA_VDISK ||
+		nfit_spa_type(spa) == NFIT_SPA_VCD   ||
+		nfit_spa_type(spa) == NFIT_SPA_VOLATILE);
+}
+
 static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
 		struct nfit_spa *nfit_spa)
 {
@@ -2301,7 +2308,7 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
 				ndr_desc);
 		if (!nfit_spa->nd_region)
 			rc = -ENOMEM;
-	} else if (nfit_spa_type(spa) == NFIT_SPA_VOLATILE) {
+	} else if (nfit_spa_is_volatile(spa)) {
 		nfit_spa->nd_region = nvdimm_volatile_region_create(nvdimm_bus,
 				ndr_desc);
 		if (!nfit_spa->nd_region)
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index e9361bffe5ee..4cfba534814b 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -38,13 +38,13 @@ static int to_nd_device_type(struct device *dev)
 {
 	if (is_nvdimm(dev))
 		return ND_DEVICE_DIMM;
-	else if (is_nd_pmem(dev))
+	else if (is_memory(dev))
 		return ND_DEVICE_REGION_PMEM;
 	else if (is_nd_blk(dev))
 		return ND_DEVICE_REGION_BLK;
 	else if (is_nd_dax(dev))
 		return ND_DEVICE_DAX_PMEM;
-	else if (is_nd_pmem(dev->parent) || is_nd_blk(dev->parent))
+	else if (is_nd_region(dev->parent))
 		return nd_region_to_nstype(to_nd_region(dev->parent));
 
 	return 0;
@@ -56,7 +56,7 @@ static int nvdimm_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 	 * Ensure that region devices always have their numa node set as
 	 * early as possible.
 	 */
-	if (is_nd_pmem(dev) || is_nd_blk(dev))
+	if (is_nd_region(dev))
 		set_dev_node(dev, to_nd_region(dev)->numa_node);
 	return add_uevent_var(env, "MODALIAS=" ND_DEVICE_MODALIAS_FMT,
 			to_nd_device_type(dev));
@@ -65,7 +65,7 @@ static int nvdimm_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 static struct module *to_bus_provider(struct device *dev)
 {
 	/* pin bus providers while regions are enabled */
-	if (is_nd_pmem(dev) || is_nd_blk(dev)) {
+	if (is_nd_region(dev)) {
 		struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
 
 		return nvdimm_bus->nd_desc->module;
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 2dee908e4bae..22e3ef463401 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -504,7 +504,7 @@ void nvdimm_badblocks_populate(struct nd_region *nd_region,
 	struct nvdimm_bus *nvdimm_bus;
 	struct list_head *poison_list;
 
-	if (!is_nd_pmem(&nd_region->dev)) {
+	if (!is_memory(&nd_region->dev)) {
 		dev_WARN_ONCE(&nd_region->dev, 1,
 				"%s only valid for pmem regions\n", __func__);
 		return;
diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c
index c1b6556aea6e..a304983ac417 100644
--- a/drivers/nvdimm/dax_devs.c
+++ b/drivers/nvdimm/dax_devs.c
@@ -89,7 +89,7 @@ struct device *nd_dax_create(struct nd_region *nd_region)
 	struct device *dev = NULL;
 	struct nd_dax *nd_dax;
 
-	if (!is_nd_pmem(&nd_region->dev))
+	if (!is_memory(&nd_region->dev))
 		return NULL;
 
 	nd_dax = nd_dax_alloc(nd_region);
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 6a1e7a3c0c17..f0d1b7e5de01 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -419,7 +419,7 @@ int alias_dpa_busy(struct device *dev, void *data)
 	struct resource *res;
 	int i;
 
-	if (!is_nd_pmem(dev))
+	if (!is_memory(dev))
 		return 0;
 
 	nd_region = to_nd_region(dev);
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 4e9261ef8a95..57724da484d0 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -112,7 +112,7 @@ static int is_uuid_busy(struct device *dev, void *data)
 
 static int is_namespace_uuid_busy(struct device *dev, void *data)
 {
-	if (is_nd_pmem(dev) || is_nd_blk(dev))
+	if (is_nd_region(dev))
 		return device_for_each_child(dev, data, is_uuid_busy);
 	return 0;
 }
@@ -783,7 +783,7 @@ static int __reserve_free_pmem(struct device *dev, void *data)
 	struct nd_label_id label_id;
 	int i;
 
-	if (!is_nd_pmem(dev))
+	if (!is_memory(dev))
 		return 0;
 
 	nd_region = to_nd_region(dev);
@@ -1872,7 +1872,7 @@ static struct device *nd_namespace_pmem_create(struct nd_region *nd_region)
 	struct resource *res;
 	struct device *dev;
 
-	if (!is_nd_pmem(&nd_region->dev))
+	if (!is_memory(&nd_region->dev))
 		return NULL;
 
 	nspm = kzalloc(sizeof(*nspm), GFP_KERNEL);
@@ -2152,7 +2152,7 @@ static struct device **scan_labels(struct nd_region *nd_region)
 		}
 		dev->parent = &nd_region->dev;
 		devs[count++] = dev;
-	} else if (is_nd_pmem(&nd_region->dev)) {
+	} else if (is_memory(&nd_region->dev)) {
 		/* clean unselected labels */
 		for (i = 0; i < nd_region->ndr_mappings; i++) {
 			struct list_head *l, *e;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 4c4bd209e725..86bc19ae30da 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -64,7 +64,16 @@ struct blk_alloc_info {
 
 bool is_nvdimm(struct device *dev);
 bool is_nd_pmem(struct device *dev);
+bool is_nd_volatile(struct device *dev);
 bool is_nd_blk(struct device *dev);
+static inline bool is_nd_region(struct device *dev)
+{
+	return is_nd_pmem(dev) || is_nd_blk(dev) || is_nd_volatile(dev);
+}
+static inline bool is_memory(struct device *dev)
+{
+	return is_nd_pmem(dev) || is_nd_volatile(dev);
+}
 struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev);
 int __init nvdimm_bus_init(void);
 void nvdimm_bus_exit(void);
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index a6c403600d19..5929eb65cee3 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -331,7 +331,7 @@ struct device *nd_pfn_create(struct nd_region *nd_region)
 	struct nd_pfn *nd_pfn;
 	struct device *dev;
 
-	if (!is_nd_pmem(&nd_region->dev))
+	if (!is_memory(&nd_region->dev))
 		return NULL;
 
 	nd_pfn = nd_pfn_alloc(nd_region);
@@ -354,7 +354,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
 	if (!pfn_sb || !ndns)
 		return -ENODEV;
 
-	if (!is_nd_pmem(nd_pfn->dev.parent))
+	if (!is_memory(nd_pfn->dev.parent))
 		return -ENODEV;
 
 	if (nvdimm_read_bytes(ndns, SZ_4K, pfn_sb, sizeof(*pfn_sb), 0))
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 41b4cdf5dea8..53a64a16aba4 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -168,6 +168,11 @@ bool is_nd_blk(struct device *dev)
 	return dev ? dev->type == &nd_blk_device_type : false;
 }
 
+bool is_nd_volatile(struct device *dev)
+{
+	return dev ? dev->type == &nd_volatile_device_type : false;
+}
+
 struct nd_region *to_nd_region(struct device *dev)
 {
 	struct nd_region *nd_region = container_of(dev, struct nd_region, dev);
@@ -214,7 +219,7 @@ EXPORT_SYMBOL_GPL(nd_blk_region_set_provider_data);
  */
 int nd_region_to_nstype(struct nd_region *nd_region)
 {
-	if (is_nd_pmem(&nd_region->dev)) {
+	if (is_memory(&nd_region->dev)) {
 		u16 i, alias;
 
 		for (i = 0, alias = 0; i < nd_region->ndr_mappings; i++) {
@@ -242,7 +247,7 @@ static ssize_t size_show(struct device *dev,
 	struct nd_region *nd_region = to_nd_region(dev);
 	unsigned long long size = 0;
 
-	if (is_nd_pmem(dev)) {
+	if (is_memory(dev)) {
 		size = nd_region->ndr_size;
 	} else if (nd_region->ndr_mappings == 1) {
 		struct nd_mapping *nd_mapping = &nd_region->mapping[0];
@@ -307,7 +312,7 @@ static ssize_t set_cookie_show(struct device *dev,
 	struct nd_region *nd_region = to_nd_region(dev);
 	struct nd_interleave_set *nd_set = nd_region->nd_set;
 
-	if (is_nd_pmem(dev) && nd_set)
+	if (is_memory(dev) && nd_set)
 		/* pass, should be precluded by region_visible */;
 	else
 		return -ENXIO;
@@ -334,7 +339,7 @@ resource_size_t nd_region_available_dpa(struct nd_region *nd_region)
 		if (!ndd)
 			return 0;
 
-		if (is_nd_pmem(&nd_region->dev)) {
+		if (is_memory(&nd_region->dev)) {
 			available += nd_pmem_available_dpa(nd_region,
 					nd_mapping, &overlap);
 			if (overlap > blk_max_overlap) {
@@ -520,10 +525,10 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
 	struct nd_interleave_set *nd_set = nd_region->nd_set;
 	int type = nd_region_to_nstype(nd_region);
 
-	if (!is_nd_pmem(dev) && a == &dev_attr_pfn_seed.attr)
+	if (!is_memory(dev) && a == &dev_attr_pfn_seed.attr)
 		return 0;
 
-	if (!is_nd_pmem(dev) && a == &dev_attr_dax_seed.attr)
+	if (!is_memory(dev) && a == &dev_attr_dax_seed.attr)
 		return 0;
 
 	if (!is_nd_pmem(dev) && a == &dev_attr_badblocks.attr)
@@ -551,7 +556,7 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
 				|| type == ND_DEVICE_NAMESPACE_BLK)
 			&& a == &dev_attr_available_size.attr)
 		return a->mode;
-	else if (is_nd_pmem(dev) && nd_set)
+	else if (is_memory(dev) && nd_set)
 		return a->mode;
 
 	return 0;
@@ -603,7 +608,7 @@ static void nd_region_notify_driver_action(struct nvdimm_bus *nvdimm_bus,
 {
 	struct nd_region *nd_region;
 
-	if (!probe && (is_nd_pmem(dev) || is_nd_blk(dev))) {
+	if (!probe && is_nd_region(dev)) {
 		int i;
 
 		nd_region = to_nd_region(dev);
@@ -621,12 +626,8 @@ static void nd_region_notify_driver_action(struct nvdimm_bus *nvdimm_bus,
 			if (ndd)
 				atomic_dec(&nvdimm->busy);
 		}
-
-		if (is_nd_pmem(dev))
-			return;
 	}
-	if (dev->parent && (is_nd_blk(dev->parent) || is_nd_pmem(dev->parent))
-			&& probe) {
+	if (dev->parent && is_nd_region(dev->parent) && probe) {
 		nd_region = to_nd_region(dev->parent);
 		nvdimm_bus_lock(dev);
 		if (nd_region->ns_seed == dev)

[PATCH v4 13/16] dax: remove default copy_from_iter fallback

[Dan Williams]

Require all dax-drivers to register a ->copy_from_iter() operation so
that it is clear which dax_operations are optional and which must be
implemented for filesystem-dax to operate.

Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/powerpc/sysdev/axonram.c |    8 ++++++++
 drivers/block/brd.c           |    8 ++++++++
 drivers/dax/super.c           |    2 --
 drivers/s390/block/dcssblk.c  |    8 ++++++++
 include/linux/dax.h           |    2 +-
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/axonram.c b/arch/powerpc/sysdev/axonram.c
index a7fe5fee744f..2799706106c6 100644
--- a/arch/powerpc/sysdev/axonram.c
+++ b/arch/powerpc/sysdev/axonram.c
@@ -45,6 +45,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/pfn_t.h>
+#include <linux/uio.h>
 
 #include <asm/page.h>
 #include <asm/prom.h>
@@ -163,8 +164,15 @@ axon_ram_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pa
 	return __axon_ram_direct_access(bank, pgoff, nr_pages, kaddr, pfn);
 }
 
+static size_t axon_ram_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	return copy_from_iter(addr, bytes, i);
+}
+
 static const struct dax_operations axon_ram_dax_ops = {
 	.direct_access = axon_ram_dax_direct_access,
+	.copy_from_iter = axon_ram_copy_from_iter,
 };
 
 /**
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 57b574f2f66a..f2a7ac350f6a 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -22,6 +22,7 @@
 #ifdef CONFIG_BLK_DEV_RAM_DAX
 #include <linux/pfn_t.h>
 #include <linux/dax.h>
+#include <linux/uio.h>
 #endif
 
 #include <linux/uaccess.h>
@@ -354,8 +355,15 @@ static long brd_dax_direct_access(struct dax_device *dax_dev,
 	return __brd_direct_access(brd, pgoff, nr_pages, kaddr, pfn);
 }
 
+static size_t brd_dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
+		void *addr, size_t bytes, struct iov_iter *i)
+{
+	return copy_from_iter(addr, bytes, i);
+}
+
 static const struct dax_operations brd_dax_ops = {
 	.direct_access = brd_dax_direct_access,
+	.copy_from_iter = brd_dax_copy_from_iter,
 };
 #endif
 
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index b7729e4d351a..9e0160b950d7 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -179,8 +179,6 @@ size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 	if (!dax_alive(dax_dev))
 		return 0;
 
-	if (!dax_dev->ops->copy_from_iter)
-		return copy_from_iter(addr, bytes, i);
 	return dax_dev->ops->copy_from_iter(dax_dev, pgoff, addr, bytes, i);
 }
 EXPORT_SYMBOL_GPL(dax_copy_from_iter);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 36e5280af3e4..88fa7b3f7a9d 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/pfn_t.h>
+#include <linux/uio.h>
 #include <linux/dax.h>
 #include <asm/extmem.h>
 #include <asm/io.h>
@@ -43,8 +44,15 @@ static const struct block_device_operations dcssblk_devops = {
 	.release 	= dcssblk_release,
 };
 
+static size_t dcssblk_dax_copy_from_iter(struct dax_device *dax_dev,
+		pgoff_t pgoff, void *addr, size_t bytes, struct iov_iter *i)
+{
+	return copy_from_iter(addr, bytes, i);
+}
+
 static const struct dax_operations dcssblk_dax_ops = {
 	.direct_access = dcssblk_dax_direct_access,
+	.copy_from_iter = dcssblk_dax_copy_from_iter,
 };
 
 struct dcssblk_dev_info {
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 1f6b6072af64..73fca1bebaf3 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -16,7 +16,7 @@ struct dax_operations {
 	 */
 	long (*direct_access)(struct dax_device *, pgoff_t, long,
 			void **, pfn_t *);
-	/* copy_from_iter: dax-driver override for default copy_from_iter */
+	/* copy_from_iter: required operation for fs-dax direct-i/o */
 	size_t (*copy_from_iter)(struct dax_device *, pgoff_t, void *, size_t,
 			struct iov_iter *);
 	/* flush: optional driver-specific cache management after writes */

[PATCH v4 14/16] dax: convert to bitmask for flags

[Dan Williams]

In preparation for adding more flags, convert the existing flag to a
bit-flag.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 9e0160b950d7..8bf71195921b 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -116,13 +116,18 @@ int __bdev_dax_supported(struct super_block *sb, int blocksize)
 EXPORT_SYMBOL_GPL(__bdev_dax_supported);
 #endif
 
+enum dax_device_flags {
+	/* !alive + rcu grace period == no new operations / mappings */
+	DAXDEV_ALIVE,
+};
+
 /**
  * struct dax_device - anchor object for dax services
  * @inode: core vfs
  * @cdev: optional character interface for "device dax"
  * @host: optional name for lookups where the device path is not available
  * @private: dax driver private data
- * @alive: !alive + rcu grace period == no new operations / mappings
+ * @flags: state and boolean properties
  */
 struct dax_device {
 	struct hlist_node list;
@@ -130,7 +135,7 @@ struct dax_device {
 	struct cdev cdev;
 	const char *host;
 	void *private;
-	bool alive;
+	unsigned long flags;
 	const struct dax_operations *ops;
 };
 
@@ -197,7 +202,7 @@ EXPORT_SYMBOL_GPL(dax_flush);
 bool dax_alive(struct dax_device *dax_dev)
 {
 	lockdep_assert_held(&dax_srcu);
-	return dax_dev->alive;
+	return test_bit(DAXDEV_ALIVE, &dax_dev->flags);
 }
 EXPORT_SYMBOL_GPL(dax_alive);
 
@@ -217,7 +222,7 @@ void kill_dax(struct dax_device *dax_dev)
 	if (!dax_dev)
 		return;
 
-	dax_dev->alive = false;
+	clear_bit(DAXDEV_ALIVE, &dax_dev->flags);
 
 	synchronize_srcu(&dax_srcu);
 
@@ -257,7 +262,7 @@ static void dax_destroy_inode(struct inode *inode)
 {
 	struct dax_device *dax_dev = to_dax_dev(inode);
 
-	WARN_ONCE(dax_dev->alive,
+	WARN_ONCE(test_bit(DAXDEV_ALIVE, &dax_dev->flags),
 			"kill_dax() must be called before final iput()\n");
 	call_rcu(&inode->i_rcu, dax_i_callback);
 }
@@ -309,7 +314,7 @@ static struct dax_device *dax_dev_get(dev_t devt)
 
 	dax_dev = to_dax_dev(inode);
 	if (inode->i_state & I_NEW) {
-		dax_dev->alive = true;
+		set_bit(DAXDEV_ALIVE, &dax_dev->flags);
 		inode->i_cdev = &dax_dev->cdev;
 		inode->i_mode = S_IFCHR;
 		inode->i_flags = S_DAX;

[PATCH v4 15/16] libnvdimm, pmem, dax: export a cache control attribute

[Dan Williams]

The dax_flush() operation can be turned into a nop on platforms where
firmware arranges for cpu caches to be flushed on a power-fail event.
The ACPI 6.2 specification defines a mechanism for the platform to
indicate this capability so the kernel can select the proper default.
However, for other platforms, the administrator must toggle this setting
manually.

Given this flush setting is a dax-specific mechanism we advertise it
through a 'dax' attribute group hanging off a host device. For example,
a 'pmem0' block-device gets a 'dax' sysfs-subdirectory with a
'write_cache' attribute to control response to dax cache flush requests.
This is similar to the 'queue/write_cache' attribute that appears under
block devices.

Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c   |   79 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/pmem.c |   10 ++++++
 include/linux/dax.h   |    3 ++
 3 files changed, 92 insertions(+)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 8bf71195921b..4827251782a1 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -119,6 +119,8 @@ EXPORT_SYMBOL_GPL(__bdev_dax_supported);
 enum dax_device_flags {
 	/* !alive + rcu grace period == no new operations / mappings */
 	DAXDEV_ALIVE,
+	/* gate whether dax_flush() calls the low level flush routine */
+	DAXDEV_WRITE_CACHE,
 };
 
 /**
@@ -139,6 +141,71 @@ struct dax_device {
 	const struct dax_operations *ops;
 };
 
+static ssize_t write_cache_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
+	ssize_t rc;
+
+	WARN_ON_ONCE(!dax_dev);
+	if (!dax_dev)
+		return -ENXIO;
+
+	rc = sprintf(buf, "%d\n", !!test_bit(DAXDEV_WRITE_CACHE,
+				&dax_dev->flags));
+	put_dax(dax_dev);
+	return rc;
+}
+
+static ssize_t write_cache_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	bool write_cache;
+	int rc = strtobool(buf, &write_cache);
+	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
+
+	WARN_ON_ONCE(!dax_dev);
+	if (!dax_dev)
+		return -ENXIO;
+
+	if (rc)
+		len = rc;
+	else if (write_cache)
+		set_bit(DAXDEV_WRITE_CACHE, &dax_dev->flags);
+	else
+		clear_bit(DAXDEV_WRITE_CACHE, &dax_dev->flags);
+
+	put_dax(dax_dev);
+	return len;
+}
+static DEVICE_ATTR_RW(write_cache);
+
+static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);
+	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
+
+	WARN_ON_ONCE(!dax_dev);
+	if (!dax_dev)
+		return 0;
+
+	if (a == &dev_attr_write_cache.attr && !dax_dev->ops->flush)
+		return 0;
+	return a->mode;
+}
+
+static struct attribute *dax_attributes[] = {
+	&dev_attr_write_cache.attr,
+	NULL,
+};
+
+struct attribute_group dax_attribute_group = {
+	.name = "dax",
+	.attrs = dax_attributes,
+	.is_visible = dax_visible,
+};
+EXPORT_SYMBOL_GPL(dax_attribute_group);
+
 /**
  * dax_direct_access() - translate a device pgoff to an absolute pfn
  * @dax_dev: a dax_device instance representing the logical memory range
@@ -194,11 +261,23 @@ void dax_flush(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 	if (!dax_alive(dax_dev))
 		return;
 
+	if (!test_bit(DAXDEV_WRITE_CACHE, &dax_dev->flags))
+		return;
+
 	if (dax_dev->ops->flush)
 		dax_dev->ops->flush(dax_dev, pgoff, addr, size);
 }
 EXPORT_SYMBOL_GPL(dax_flush);
 
+void dax_write_cache(struct dax_device *dax_dev, bool wc)
+{
+	if (wc)
+		set_bit(DAXDEV_WRITE_CACHE, &dax_dev->flags);
+	else
+		clear_bit(DAXDEV_WRITE_CACHE, &dax_dev->flags);
+}
+EXPORT_SYMBOL_GPL(dax_write_cache);
+
 bool dax_alive(struct dax_device *dax_dev)
 {
 	lockdep_assert_held(&dax_srcu);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 06f6c27ec1e9..7339d184070e 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -253,6 +253,11 @@ static const struct dax_operations pmem_dax_ops = {
 	.flush = pmem_dax_flush,
 };
 
+static const struct attribute_group *pmem_attribute_groups[] = {
+	&dax_attribute_group,
+	NULL,
+};
+
 static void pmem_release_queue(void *q)
 {
 	blk_cleanup_queue(q);
@@ -287,6 +292,7 @@ static int pmem_attach_disk(struct device *dev,
 	struct pmem_device *pmem;
 	struct resource pfn_res;
 	struct request_queue *q;
+	struct device *gendev;
 	struct gendisk *disk;
 	void *addr;
 
@@ -384,8 +390,12 @@ static int pmem_attach_disk(struct device *dev,
 		put_disk(disk);
 		return -ENOMEM;
 	}
+	dax_write_cache(dax_dev, true);
 	pmem->dax_dev = dax_dev;
 
+	gendev = disk_to_dev(disk);
+	gendev->groups = pmem_attribute_groups;
+
 	device_add_disk(dev, disk);
 	if (devm_add_action_or_reset(dev, pmem_release_disk, pmem))
 		return -ENOMEM;
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 73fca1bebaf3..8f39db7439c3 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -23,6 +23,8 @@ struct dax_operations {
 	void (*flush)(struct dax_device *, pgoff_t, void *, size_t);
 };
 
+extern struct attribute_group dax_attribute_group;
+
 #if IS_ENABLED(CONFIG_DAX)
 struct dax_device *dax_get_by_host(const char *host);
 void put_dax(struct dax_device *dax_dev);
@@ -84,6 +86,7 @@ size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 		size_t bytes, struct iov_iter *i);
 void dax_flush(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
 		size_t size);
+void dax_write_cache(struct dax_device *dax_dev, bool wc);
 
 /*
  * We use lowest available bit in exceptional entry for locking, one bit for

[PATCH v4 16/16] libnvdimm, pmem: disable dax flushing when pmem is fronting a volatile region

[Dan Williams]

The pmem driver attaches to both persistent and volatile memory ranges
advertised by the ACPI NFIT. When the region is volatile it is redundant
to spend cycles flushing caches at fsync(). Check if the hosting region
is volatile and do not set dax_write_cache() if it is.

Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/pmem.c        |   13 ++++++++-----
 drivers/nvdimm/region_devs.c |    6 ++++++
 include/linux/libnvdimm.h    |    1 +
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 7339d184070e..e7a40f77f729 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -284,10 +284,10 @@ static int pmem_attach_disk(struct device *dev,
 	struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
 	struct nd_region *nd_region = to_nd_region(dev->parent);
 	struct vmem_altmap __altmap, *altmap = NULL;
+	int nid = dev_to_node(dev), fua, wbc;
 	struct resource *res = &nsio->res;
 	struct nd_pfn *nd_pfn = NULL;
 	struct dax_device *dax_dev;
-	int nid = dev_to_node(dev);
 	struct nd_pfn_sb *pfn_sb;
 	struct pmem_device *pmem;
 	struct resource pfn_res;
@@ -314,9 +314,12 @@ static int pmem_attach_disk(struct device *dev,
 	dev_set_drvdata(dev, pmem);
 	pmem->phys_addr = res->start;
 	pmem->size = resource_size(res);
-	if (!IS_ENABLED(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE)
-			|| nvdimm_has_flush(nd_region) < 0)
+	fua = nvdimm_has_flush(nd_region);
+	if (!IS_ENABLED(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) || fua < 0) {
 		dev_warn(dev, "unable to guarantee persistence of writes\n");
+		fua = 0;
+	}
+	wbc = nvdimm_has_cache(nd_region);
 
 	if (!devm_request_mem_region(dev, res->start, resource_size(res),
 				dev_name(&ndns->dev))) {
@@ -360,7 +363,7 @@ static int pmem_attach_disk(struct device *dev,
 		return PTR_ERR(addr);
 	pmem->virt_addr = addr;
 
-	blk_queue_write_cache(q, true, true);
+	blk_queue_write_cache(q, wbc, fua);
 	blk_queue_make_request(q, pmem_make_request);
 	blk_queue_physical_block_size(q, PAGE_SIZE);
 	blk_queue_max_hw_sectors(q, UINT_MAX);
@@ -390,7 +393,7 @@ static int pmem_attach_disk(struct device *dev,
 		put_disk(disk);
 		return -ENOMEM;
 	}
-	dax_write_cache(dax_dev, true);
+	dax_write_cache(dax_dev, wbc);
 	pmem->dax_dev = dax_dev;
 
 	gendev = disk_to_dev(disk);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 53a64a16aba4..0c3b089b280a 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1060,6 +1060,12 @@ int nvdimm_has_flush(struct nd_region *nd_region)
 }
 EXPORT_SYMBOL_GPL(nvdimm_has_flush);
 
+int nvdimm_has_cache(struct nd_region *nd_region)
+{
+	return is_nd_pmem(&nd_region->dev);
+}
+EXPORT_SYMBOL_GPL(nvdimm_has_cache);
+
 void __exit nd_region_devs_exit(void)
 {
 	ida_destroy(&region_ida);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index b2f659bd661d..a8ee1d0afd70 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -165,4 +165,5 @@ void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane);
 u64 nd_fletcher64(void *addr, size_t len, bool le);
 void nvdimm_flush(struct nd_region *nd_region);
 int nvdimm_has_flush(struct nd_region *nd_region);
+int nvdimm_has_cache(struct nd_region *nd_region);
 #endif /* __LIBNVDIMM_H__ */

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 06/29/2017 01:54 PM, Dan Williams wrote:
> Allow volatile nfit ranges to participate in all the same infrastructure
> provided for persistent memory regions. 

This seems to be a bit more than "other rework".

> A resulting resulting namespace
> device will still be called "pmem", but the parent region type will be
> "nd_volatile". 

What does this look like to a user or admin?  How does someone know that
/dev/pmemX is persistent memory and /dev/pmemY isn't?  Someone shouldn't
have to weed through /sys or ndctl some other interface to figure that out
in the future if they don't have to do that today.  We have different
names for BTT namespaces.  Is there a different name for volatile ranges?

-- ljk

> This is in preparation for disabling the dax ->flush()
> operation in the pmem driver when it is hosted on a volatile range.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Wilcox <mawilcox@microsoft.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/acpi/nfit/core.c        |    9 ++++++++-
>  drivers/nvdimm/bus.c            |    8 ++++----
>  drivers/nvdimm/core.c           |    2 +-
>  drivers/nvdimm/dax_devs.c       |    2 +-
>  drivers/nvdimm/dimm_devs.c      |    2 +-
>  drivers/nvdimm/namespace_devs.c |    8 ++++----
>  drivers/nvdimm/nd-core.h        |    9 +++++++++
>  drivers/nvdimm/pfn_devs.c       |    4 ++--
>  drivers/nvdimm/region_devs.c    |   27 ++++++++++++++-------------
>  9 files changed, 44 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index ac2436538b7e..60d1ca149cc1 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2227,6 +2227,13 @@ static bool nfit_spa_is_virtual(struct acpi_nfit_system_address *spa)
>  		nfit_spa_type(spa) == NFIT_SPA_PCD);
>  }
>  
> +static bool nfit_spa_is_volatile(struct acpi_nfit_system_address *spa)
> +{
> +	return (nfit_spa_type(spa) == NFIT_SPA_VDISK ||
> +		nfit_spa_type(spa) == NFIT_SPA_VCD   ||
> +		nfit_spa_type(spa) == NFIT_SPA_VOLATILE);
> +}
> +
>  static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
>  		struct nfit_spa *nfit_spa)
>  {
> @@ -2301,7 +2308,7 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
>  				ndr_desc);
>  		if (!nfit_spa->nd_region)
>  			rc = -ENOMEM;
> -	} else if (nfit_spa_type(spa) == NFIT_SPA_VOLATILE) {
> +	} else if (nfit_spa_is_volatile(spa)) {
>  		nfit_spa->nd_region = nvdimm_volatile_region_create(nvdimm_bus,
>  				ndr_desc);
>  		if (!nfit_spa->nd_region)
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index e9361bffe5ee..4cfba534814b 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -38,13 +38,13 @@ static int to_nd_device_type(struct device *dev)
>  {
>  	if (is_nvdimm(dev))
>  		return ND_DEVICE_DIMM;
> -	else if (is_nd_pmem(dev))
> +	else if (is_memory(dev))
>  		return ND_DEVICE_REGION_PMEM;
>  	else if (is_nd_blk(dev))
>  		return ND_DEVICE_REGION_BLK;
>  	else if (is_nd_dax(dev))
>  		return ND_DEVICE_DAX_PMEM;
> -	else if (is_nd_pmem(dev->parent) || is_nd_blk(dev->parent))
> +	else if (is_nd_region(dev->parent))
>  		return nd_region_to_nstype(to_nd_region(dev->parent));
>  
>  	return 0;
> @@ -56,7 +56,7 @@ static int nvdimm_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	 * Ensure that region devices always have their numa node set as
>  	 * early as possible.
>  	 */
> -	if (is_nd_pmem(dev) || is_nd_blk(dev))
> +	if (is_nd_region(dev))
>  		set_dev_node(dev, to_nd_region(dev)->numa_node);
>  	return add_uevent_var(env, "MODALIAS=" ND_DEVICE_MODALIAS_FMT,
>  			to_nd_device_type(dev));
> @@ -65,7 +65,7 @@ static int nvdimm_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
>  static struct module *to_bus_provider(struct device *dev)
>  {
>  	/* pin bus providers while regions are enabled */
> -	if (is_nd_pmem(dev) || is_nd_blk(dev)) {
> +	if (is_nd_region(dev)) {
>  		struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
>  
>  		return nvdimm_bus->nd_desc->module;
> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
> index 2dee908e4bae..22e3ef463401 100644
> --- a/drivers/nvdimm/core.c
> +++ b/drivers/nvdimm/core.c
> @@ -504,7 +504,7 @@ void nvdimm_badblocks_populate(struct nd_region *nd_region,
>  	struct nvdimm_bus *nvdimm_bus;
>  	struct list_head *poison_list;
>  
> -	if (!is_nd_pmem(&nd_region->dev)) {
> +	if (!is_memory(&nd_region->dev)) {
>  		dev_WARN_ONCE(&nd_region->dev, 1,
>  				"%s only valid for pmem regions\n", __func__);
>  		return;
> diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c
> index c1b6556aea6e..a304983ac417 100644
> --- a/drivers/nvdimm/dax_devs.c
> +++ b/drivers/nvdimm/dax_devs.c
> @@ -89,7 +89,7 @@ struct device *nd_dax_create(struct nd_region *nd_region)
>  	struct device *dev = NULL;
>  	struct nd_dax *nd_dax;
>  
> -	if (!is_nd_pmem(&nd_region->dev))
> +	if (!is_memory(&nd_region->dev))
>  		return NULL;
>  
>  	nd_dax = nd_dax_alloc(nd_region);
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index 6a1e7a3c0c17..f0d1b7e5de01 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -419,7 +419,7 @@ int alias_dpa_busy(struct device *dev, void *data)
>  	struct resource *res;
>  	int i;
>  
> -	if (!is_nd_pmem(dev))
> +	if (!is_memory(dev))
>  		return 0;
>  
>  	nd_region = to_nd_region(dev);
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 4e9261ef8a95..57724da484d0 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -112,7 +112,7 @@ static int is_uuid_busy(struct device *dev, void *data)
>  
>  static int is_namespace_uuid_busy(struct device *dev, void *data)
>  {
> -	if (is_nd_pmem(dev) || is_nd_blk(dev))
> +	if (is_nd_region(dev))
>  		return device_for_each_child(dev, data, is_uuid_busy);
>  	return 0;
>  }
> @@ -783,7 +783,7 @@ static int __reserve_free_pmem(struct device *dev, void *data)
>  	struct nd_label_id label_id;
>  	int i;
>  
> -	if (!is_nd_pmem(dev))
> +	if (!is_memory(dev))
>  		return 0;
>  
>  	nd_region = to_nd_region(dev);
> @@ -1872,7 +1872,7 @@ static struct device *nd_namespace_pmem_create(struct nd_region *nd_region)
>  	struct resource *res;
>  	struct device *dev;
>  
> -	if (!is_nd_pmem(&nd_region->dev))
> +	if (!is_memory(&nd_region->dev))
>  		return NULL;
>  
>  	nspm = kzalloc(sizeof(*nspm), GFP_KERNEL);
> @@ -2152,7 +2152,7 @@ static struct device **scan_labels(struct nd_region *nd_region)
>  		}
>  		dev->parent = &nd_region->dev;
>  		devs[count++] = dev;
> -	} else if (is_nd_pmem(&nd_region->dev)) {
> +	} else if (is_memory(&nd_region->dev)) {
>  		/* clean unselected labels */
>  		for (i = 0; i < nd_region->ndr_mappings; i++) {
>  			struct list_head *l, *e;
> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
> index 4c4bd209e725..86bc19ae30da 100644
> --- a/drivers/nvdimm/nd-core.h
> +++ b/drivers/nvdimm/nd-core.h
> @@ -64,7 +64,16 @@ struct blk_alloc_info {
>  
>  bool is_nvdimm(struct device *dev);
>  bool is_nd_pmem(struct device *dev);
> +bool is_nd_volatile(struct device *dev);
>  bool is_nd_blk(struct device *dev);
> +static inline bool is_nd_region(struct device *dev)
> +{
> +	return is_nd_pmem(dev) || is_nd_blk(dev) || is_nd_volatile(dev);
> +}
> +static inline bool is_memory(struct device *dev)
> +{
> +	return is_nd_pmem(dev) || is_nd_volatile(dev);
> +}
>  struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev);
>  int __init nvdimm_bus_init(void);
>  void nvdimm_bus_exit(void);
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index a6c403600d19..5929eb65cee3 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -331,7 +331,7 @@ struct device *nd_pfn_create(struct nd_region *nd_region)
>  	struct nd_pfn *nd_pfn;
>  	struct device *dev;
>  
> -	if (!is_nd_pmem(&nd_region->dev))
> +	if (!is_memory(&nd_region->dev))
>  		return NULL;
>  
>  	nd_pfn = nd_pfn_alloc(nd_region);
> @@ -354,7 +354,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
>  	if (!pfn_sb || !ndns)
>  		return -ENODEV;
>  
> -	if (!is_nd_pmem(nd_pfn->dev.parent))
> +	if (!is_memory(nd_pfn->dev.parent))
>  		return -ENODEV;
>  
>  	if (nvdimm_read_bytes(ndns, SZ_4K, pfn_sb, sizeof(*pfn_sb), 0))
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index 41b4cdf5dea8..53a64a16aba4 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -168,6 +168,11 @@ bool is_nd_blk(struct device *dev)
>  	return dev ? dev->type == &nd_blk_device_type : false;
>  }
>  
> +bool is_nd_volatile(struct device *dev)
> +{
> +	return dev ? dev->type == &nd_volatile_device_type : false;
> +}
> +
>  struct nd_region *to_nd_region(struct device *dev)
>  {
>  	struct nd_region *nd_region = container_of(dev, struct nd_region, dev);
> @@ -214,7 +219,7 @@ EXPORT_SYMBOL_GPL(nd_blk_region_set_provider_data);
>   */
>  int nd_region_to_nstype(struct nd_region *nd_region)
>  {
> -	if (is_nd_pmem(&nd_region->dev)) {
> +	if (is_memory(&nd_region->dev)) {
>  		u16 i, alias;
>  
>  		for (i = 0, alias = 0; i < nd_region->ndr_mappings; i++) {
> @@ -242,7 +247,7 @@ static ssize_t size_show(struct device *dev,
>  	struct nd_region *nd_region = to_nd_region(dev);
>  	unsigned long long size = 0;
>  
> -	if (is_nd_pmem(dev)) {
> +	if (is_memory(dev)) {
>  		size = nd_region->ndr_size;
>  	} else if (nd_region->ndr_mappings == 1) {
>  		struct nd_mapping *nd_mapping = &nd_region->mapping[0];
> @@ -307,7 +312,7 @@ static ssize_t set_cookie_show(struct device *dev,
>  	struct nd_region *nd_region = to_nd_region(dev);
>  	struct nd_interleave_set *nd_set = nd_region->nd_set;
>  
> -	if (is_nd_pmem(dev) && nd_set)
> +	if (is_memory(dev) && nd_set)
>  		/* pass, should be precluded by region_visible */;
>  	else
>  		return -ENXIO;
> @@ -334,7 +339,7 @@ resource_size_t nd_region_available_dpa(struct nd_region *nd_region)
>  		if (!ndd)
>  			return 0;
>  
> -		if (is_nd_pmem(&nd_region->dev)) {
> +		if (is_memory(&nd_region->dev)) {
>  			available += nd_pmem_available_dpa(nd_region,
>  					nd_mapping, &overlap);
>  			if (overlap > blk_max_overlap) {
> @@ -520,10 +525,10 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
>  	struct nd_interleave_set *nd_set = nd_region->nd_set;
>  	int type = nd_region_to_nstype(nd_region);
>  
> -	if (!is_nd_pmem(dev) && a == &dev_attr_pfn_seed.attr)
> +	if (!is_memory(dev) && a == &dev_attr_pfn_seed.attr)
>  		return 0;
>  
> -	if (!is_nd_pmem(dev) && a == &dev_attr_dax_seed.attr)
> +	if (!is_memory(dev) && a == &dev_attr_dax_seed.attr)
>  		return 0;
>  
>  	if (!is_nd_pmem(dev) && a == &dev_attr_badblocks.attr)
> @@ -551,7 +556,7 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
>  				|| type == ND_DEVICE_NAMESPACE_BLK)
>  			&& a == &dev_attr_available_size.attr)
>  		return a->mode;
> -	else if (is_nd_pmem(dev) && nd_set)
> +	else if (is_memory(dev) && nd_set)
>  		return a->mode;
>  
>  	return 0;
> @@ -603,7 +608,7 @@ static void nd_region_notify_driver_action(struct nvdimm_bus *nvdimm_bus,
>  {
>  	struct nd_region *nd_region;
>  
> -	if (!probe && (is_nd_pmem(dev) || is_nd_blk(dev))) {
> +	if (!probe && is_nd_region(dev)) {
>  		int i;
>  
>  		nd_region = to_nd_region(dev);
> @@ -621,12 +626,8 @@ static void nd_region_notify_driver_action(struct nvdimm_bus *nvdimm_bus,
>  			if (ndd)
>  				atomic_dec(&nvdimm->busy);
>  		}
> -
> -		if (is_nd_pmem(dev))
> -			return;
>  	}
> -	if (dev->parent && (is_nd_blk(dev->parent) || is_nd_pmem(dev->parent))
> -			&& probe) {
> +	if (dev->parent && is_nd_region(dev->parent) && probe) {
>  		nd_region = to_nd_region(dev->parent);
>  		nvdimm_bus_lock(dev);
>  		if (nd_region->ns_seed == dev)
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
> 

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 12:20 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> On 06/29/2017 01:54 PM, Dan Williams wrote:
>> Allow volatile nfit ranges to participate in all the same infrastructure
>> provided for persistent memory regions.
>
> This seems to be a bit more than "other rework".

It's part of the rationale for having a "write_cache" control
attribute. There's only so much I can squeeze into the subject line,
but it is mentioned in the cover letter.

>> A resulting resulting namespace
>> device will still be called "pmem", but the parent region type will be
>> "nd_volatile".
>
> What does this look like to a user or admin?  How does someone know that
> /dev/pmemX is persistent memory and /dev/pmemY isn't?  Someone shouldn't
> have to weed through /sys or ndctl some other interface to figure that out
> in the future if they don't have to do that today.  We have different
> names for BTT namespaces.  Is there a different name for volatile ranges?

No, the block device name is still /dev/pmem. It's already the case
that you need to check behind just the name of the device to figure
out if something is actually volatile or not (see memmap=ss!nn
configurations), so I would not be in favor of changing the device
name if we think the memory might not be persistent. Moreover, I think
it was a mistake that we change the device name for btt or not, and
I'm glad Matthew talked me out of making the same mistake with
memory-mode vs raw-mode pmem namespaces. So, the block device name
just reflects the driver of the block device, not the properties of
the device, just like all other block device instances.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 06/29/2017 04:42 PM, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 12:20 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>> On 06/29/2017 01:54 PM, Dan Williams wrote:
>>> Allow volatile nfit ranges to participate in all the same infrastructure
>>> provided for persistent memory regions.
>>
>> This seems to be a bit more than "other rework".
> 
> It's part of the rationale for having a "write_cache" control
> attribute. There's only so much I can squeeze into the subject line,
> but it is mentioned in the cover letter.
> 
>>> A resulting resulting namespace
>>> device will still be called "pmem", but the parent region type will be
>>> "nd_volatile".
>>
>> What does this look like to a user or admin?  How does someone know that
>> /dev/pmemX is persistent memory and /dev/pmemY isn't?  Someone shouldn't
>> have to weed through /sys or ndctl some other interface to figure that out
>> in the future if they don't have to do that today.  We have different
>> names for BTT namespaces.  Is there a different name for volatile ranges?
> 
> No, the block device name is still /dev/pmem. It's already the case
> that you need to check behind just the name of the device to figure
> out if something is actually volatile or not (see memmap=ss!nn
> configurations),

I don't have any experience with using memmap but if it's primarily used
by developers without NVDIMMs, they'd know it's not persistent.  Or is it
primarily used by administrators using non-NFIT NVDIMMs, in which case it
is persistent?

In any case, how exactly does one determine whether the device is volatile
or not?  I'm dumb so tell me the command line or API.

> so I would not be in favor of changing the device
> name if we think the memory might not be persistent. Moreover, I think
> it was a mistake that we change the device name for btt or not, and
> I'm glad Matthew talked me out of making the same mistake with
> memory-mode vs raw-mode pmem namespaces. So, the block device name
> just reflects the driver of the block device, not the properties of
> the device, just like all other block device instances.

I agree that creating a new device name for BTT was perhaps a mistake,
although it would be good to know how to query a device property for
sector atomicity.  The difference between BTT vs. non-BTT seems less
critical to me than knowing in an obvious way whether the device is
actually persistent.

-- ljk

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 2:16 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> On 06/29/2017 04:42 PM, Dan Williams wrote:
>> On Thu, Jun 29, 2017 at 12:20 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>>> On 06/29/2017 01:54 PM, Dan Williams wrote:
>>>> Allow volatile nfit ranges to participate in all the same infrastructure
>>>> provided for persistent memory regions.
>>>
>>> This seems to be a bit more than "other rework".
>>
>> It's part of the rationale for having a "write_cache" control
>> attribute. There's only so much I can squeeze into the subject line,
>> but it is mentioned in the cover letter.
>>
>>>> A resulting resulting namespace
>>>> device will still be called "pmem", but the parent region type will be
>>>> "nd_volatile".
>>>
>>> What does this look like to a user or admin?  How does someone know that
>>> /dev/pmemX is persistent memory and /dev/pmemY isn't?  Someone shouldn't
>>> have to weed through /sys or ndctl some other interface to figure that out
>>> in the future if they don't have to do that today.  We have different
>>> names for BTT namespaces.  Is there a different name for volatile ranges?
>>
>> No, the block device name is still /dev/pmem. It's already the case
>> that you need to check behind just the name of the device to figure
>> out if something is actually volatile or not (see memmap=ss!nn
>> configurations),
>
> I don't have any experience with using memmap but if it's primarily used
> by developers without NVDIMMs, they'd know it's not persistent.  Or is it
> primarily used by administrators using non-NFIT NVDIMMs, in which case it
> is persistent?
>
> In any case, how exactly does one determine whether the device is volatile
> or not?  I'm dumb so tell me the command line or API.

Especially with memmap= or e820-defined memory it's unknowable from
the kernel. We don't know if the user is using it to cover for a
platform where there is no BIOS support for advertising persistent
memory, or if they have a BIOS that does not produce an NFIT as is the
case here [1], or if it is some developer just testing with no
expectation of persistence.

[1]: https://github.com/pmem/ndctl/issues/21

>> so I would not be in favor of changing the device
>> name if we think the memory might not be persistent. Moreover, I think
>> it was a mistake that we change the device name for btt or not, and
>> I'm glad Matthew talked me out of making the same mistake with
>> memory-mode vs raw-mode pmem namespaces. So, the block device name
>> just reflects the driver of the block device, not the properties of
>> the device, just like all other block device instances.
>
> I agree that creating a new device name for BTT was perhaps a mistake,
> although it would be good to know how to query a device property for
> sector atomicity.  The difference between BTT vs. non-BTT seems less
> critical to me than knowing in an obvious way whether the device is
> actually persistent.

We don't have a good way to answer "actually persistent" in the
general case. I'm thinking of cases where the energy source on the
DIMM has died, or we trigger one of the conditions that leads to the
""unable to guarantee persistence of writes" message. The /dev/pmem
device name just tells you that your block device is hosted by a
driver that knows how to handle persistent memory constraints, but any
other details about the nature of the address range need to come from
other sources of information, and potentially information sources that
the kernel does not know about.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 6/29/2017 5:50 PM, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 2:16 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>> On 06/29/2017 04:42 PM, Dan Williams wrote:
>>> On Thu, Jun 29, 2017 at 12:20 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>>>> On 06/29/2017 01:54 PM, Dan Williams wrote:
>>>>> Allow volatile nfit ranges to participate in all the same infrastructure
>>>>> provided for persistent memory regions.
>>>>
>>>> This seems to be a bit more than "other rework".
>>>
>>> It's part of the rationale for having a "write_cache" control
>>> attribute. There's only so much I can squeeze into the subject line,
>>> but it is mentioned in the cover letter.
>>>
>>>>> A resulting resulting namespace
>>>>> device will still be called "pmem", but the parent region type will be
>>>>> "nd_volatile".
>>>>
>>>> What does this look like to a user or admin?  How does someone know that
>>>> /dev/pmemX is persistent memory and /dev/pmemY isn't?  Someone shouldn't
>>>> have to weed through /sys or ndctl some other interface to figure that out
>>>> in the future if they don't have to do that today.  We have different
>>>> names for BTT namespaces.  Is there a different name for volatile ranges?
>>>
>>> No, the block device name is still /dev/pmem. It's already the case
>>> that you need to check behind just the name of the device to figure
>>> out if something is actually volatile or not (see memmap=ss!nn
>>> configurations),
>>
>> I don't have any experience with using memmap but if it's primarily used
>> by developers without NVDIMMs, they'd know it's not persistent.  Or is it
>> primarily used by administrators using non-NFIT NVDIMMs, in which case it
>> is persistent?
>>
>> In any case, how exactly does one determine whether the device is volatile
>> or not?  I'm dumb so tell me the command line or API.
>
> Especially with memmap= or e820-defined memory it's unknowable from
> the kernel. We don't know if the user is using it to cover for a
> platform where there is no BIOS support for advertising persistent
> memory, or if they have a BIOS that does not produce an NFIT as is the
> case here [1], or if it is some developer just testing with no
> expectation of persistence.
>
> [1]: https://github.com/pmem/ndctl/issues/21

Ok.  I'm not really concerned about those cases but was asking since
you mentioned memmap as an example.

In any case, how does someone, like a system administrator, confirm that
a /dev/pmem device is a device that claims to be persistent?  Is there
a specific ndctl command line that would make it obvious what the Linux
device is on a device that claims to be persistent?

>>> so I would not be in favor of changing the device
>>> name if we think the memory might not be persistent. Moreover, I think
>>> it was a mistake that we change the device name for btt or not, and
>>> I'm glad Matthew talked me out of making the same mistake with
>>> memory-mode vs raw-mode pmem namespaces. So, the block device name
>>> just reflects the driver of the block device, not the properties of
>>> the device, just like all other block device instances.
>>
>> I agree that creating a new device name for BTT was perhaps a mistake,
>> although it would be good to know how to query a device property for
>> sector atomicity.  The difference between BTT vs. non-BTT seems less
>> critical to me than knowing in an obvious way whether the device is
>> actually persistent.
>
> We don't have a good way to answer "actually persistent" in the
> general case. I'm thinking of cases where the energy source on the
> DIMM has died, or we trigger one of the conditions that leads to the
> ""unable to guarantee persistence of writes" message.

There are certainly error conditions that can happen and we've talked
about that bit in our health status discussions.  I think the question
of whether the device is healthy enough to be persistent right now
is different from whether the device is never ever going to be persistent.

> The /dev/pmem
> device name just tells you that your block device is hosted by a
> driver that knows how to handle persistent memory constraints, but any
> other details about the nature of the address range need to come from
> other sources of information, and potentially information sources that
> the kernel does not know about.

I'm asking about the other source of information in this specific case
where we're exposing pmem devices that will never ever be persistent.
Before we add these devices, I think we should be able to tell the user
how they can know the properties of the underlying device.

-- ljk

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 3:12 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
[..]
>> The /dev/pmem
>> device name just tells you that your block device is hosted by a
>> driver that knows how to handle persistent memory constraints, but any
>> other details about the nature of the address range need to come from
>> other sources of information, and potentially information sources that
>> the kernel does not know about.
>
>
> I'm asking about the other source of information in this specific case
> where we're exposing pmem devices that will never ever be persistent.
> Before we add these devices, I think we should be able to tell the user
> how they can know the properties of the underlying device.

The only way I can think to indicate this is with a platform + device
whitelist in a tool like ndctl. Where the tool says "yes, these
xyz-vendor DIMMs on this abc-vendor platform with this 123-version
BIOS" is a known good persistent configuration.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 06/29/2017 06:28 PM, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 3:12 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> [..]
>>> The /dev/pmem
>>> device name just tells you that your block device is hosted by a
>>> driver that knows how to handle persistent memory constraints, but any
>>> other details about the nature of the address range need to come from
>>> other sources of information, and potentially information sources that
>>> the kernel does not know about.
>>
>>
>> I'm asking about the other source of information in this specific case
>> where we're exposing pmem devices that will never ever be persistent.
>> Before we add these devices, I think we should be able to tell the user
>> how they can know the properties of the underlying device.
> 
> The only way I can think to indicate this is with a platform + device
> whitelist in a tool like ndctl. Where the tool says "yes, these
> xyz-vendor DIMMs on this abc-vendor platform with this 123-version
> BIOS" is a known good persistent configuration.

Doesn't the kernel know that something will never ever be persistent
because the NFIT type says NFIT_SPA_VDISK, NFIT_SPA_VCD, or NFIT_SPA_VOLATILE?
That's the case I'm asking about here.   In this patch, you're adding support
for creating /dev/pmem devices for those address ranges.  My question is
how the admin/user knows that those devices will never ever be persistent.

I don't think we need ndctl to know which vendors' hardware/firmware
actually works as advertised.

-- ljk

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 3:35 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> On 06/29/2017 06:28 PM, Dan Williams wrote:
>> On Thu, Jun 29, 2017 at 3:12 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>> [..]
>>>> The /dev/pmem
>>>> device name just tells you that your block device is hosted by a
>>>> driver that knows how to handle persistent memory constraints, but any
>>>> other details about the nature of the address range need to come from
>>>> other sources of information, and potentially information sources that
>>>> the kernel does not know about.
>>>
>>>
>>> I'm asking about the other source of information in this specific case
>>> where we're exposing pmem devices that will never ever be persistent.
>>> Before we add these devices, I think we should be able to tell the user
>>> how they can know the properties of the underlying device.
>>
>> The only way I can think to indicate this is with a platform + device
>> whitelist in a tool like ndctl. Where the tool says "yes, these
>> xyz-vendor DIMMs on this abc-vendor platform with this 123-version
>> BIOS" is a known good persistent configuration.
>
> Doesn't the kernel know that something will never ever be persistent
> because the NFIT type says NFIT_SPA_VDISK, NFIT_SPA_VCD, or NFIT_SPA_VOLATILE?
> That's the case I'm asking about here.   In this patch, you're adding support
> for creating /dev/pmem devices for those address ranges.  My question is
> how the admin/user knows that those devices will never ever be persistent.

The parent region of the namespace will have a 'volatile' type:

# cat /sys/bus/nd/devices/region0/devtype
nd_volatile

> I don't think we need ndctl to know which vendors' hardware/firmware
> actually works as advertised.

Certification stickers is what I was thinking, but I was missing your
direction question.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 6/29/2017 6:43 PM, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 3:35 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>> On 06/29/2017 06:28 PM, Dan Williams wrote:
>>> On Thu, Jun 29, 2017 at 3:12 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>>> [..]
>>>>> The /dev/pmem
>>>>> device name just tells you that your block device is hosted by a
>>>>> driver that knows how to handle persistent memory constraints, but any
>>>>> other details about the nature of the address range need to come from
>>>>> other sources of information, and potentially information sources that
>>>>> the kernel does not know about.
>>>>
>>>>
>>>> I'm asking about the other source of information in this specific case
>>>> where we're exposing pmem devices that will never ever be persistent.
>>>> Before we add these devices, I think we should be able to tell the user
>>>> how they can know the properties of the underlying device.
>>>
>>> The only way I can think to indicate this is with a platform + device
>>> whitelist in a tool like ndctl. Where the tool says "yes, these
>>> xyz-vendor DIMMs on this abc-vendor platform with this 123-version
>>> BIOS" is a known good persistent configuration.
>>
>> Doesn't the kernel know that something will never ever be persistent
>> because the NFIT type says NFIT_SPA_VDISK, NFIT_SPA_VCD, or NFIT_SPA_VOLATILE?
>> That's the case I'm asking about here.   In this patch, you're adding support
>> for creating /dev/pmem devices for those address ranges.  My question is
>> how the admin/user knows that those devices will never ever be persistent.
>
> The parent region of the namespace will have a 'volatile' type:
>
> # cat /sys/bus/nd/devices/region0/devtype
> nd_volatile

If all I know is the /dev/pmem device name, how do I find that?

-- ljk
>
>> I don't think we need ndctl to know which vendors' hardware/firmware
>> actually works as advertised.
>
> Certification stickers is what I was thinking, but I was missing your
> direction question.
>

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 3:49 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>> The parent region of the namespace will have a 'volatile' type:
>>
>> # cat /sys/bus/nd/devices/region0/devtype
>> nd_volatile
>
>
> If all I know is the /dev/pmem device name, how do I find that?
>

    cat $(readlink -f /sys/block/pmem0/device)/../devtype

...this is where 'ndctl list' will get the information.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Linda Knippers]

On 06/29/2017 06:58 PM, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 3:49 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>>> The parent region of the namespace will have a 'volatile' type:
>>>
>>> # cat /sys/bus/nd/devices/region0/devtype
>>> nd_volatile
>>
>>
>> If all I know is the /dev/pmem device name, how do I find that?
>>
> 
>     cat $(readlink -f /sys/block/pmem0/device)/../devtype
> 
> ...this is where 'ndctl list' will get the information.
> 

Thanks.

I think we need a section 4 pmem manpage like exists for
mem, sd, fd, md, etc., where we can put stuff like this, as well
as providing some overview information that will point people to
other resources.  I'll give that some thought unless there is one
already that I'm not finding.

-- ljk

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Thu, Jun 29, 2017 at 4:14 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> On 06/29/2017 06:58 PM, Dan Williams wrote:
>> On Thu, Jun 29, 2017 at 3:49 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
>>>> The parent region of the namespace will have a 'volatile' type:
>>>>
>>>> # cat /sys/bus/nd/devices/region0/devtype
>>>> nd_volatile
>>>
>>>
>>> If all I know is the /dev/pmem device name, how do I find that?
>>>
>>
>>     cat $(readlink -f /sys/block/pmem0/device)/../devtype
>>
>> ...this is where 'ndctl list' will get the information.
>>
>
> Thanks.
>
> I think we need a section 4 pmem manpage like exists for
> mem, sd, fd, md, etc., where we can put stuff like this, as well
> as providing some overview information that will point people to
> other resources.  I'll give that some thought unless there is one
> already that I'm not finding.
>

A "pmem" man page sounds like a great idea, I wasn't aware we even had
an sd man page.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Kani, Toshimitsu]

On Thu, 2017-06-29 at 18:28 -0700, Dan Williams wrote:
> On Thu, Jun 29, 2017 at 4:14 PM, Linda Knippers <linda.knippers@hpe.c
> om> wrote:
> > On 06/29/2017 06:58 PM, Dan Williams wrote:
> > > On Thu, Jun 29, 2017 at 3:49 PM, Linda Knippers <linda.knippers@h
> > > pe.com> wrote:
> > > > > The parent region of the namespace will have a 'volatile'
> > > > > type:
> > > > > 
> > > > > # cat /sys/bus/nd/devices/region0/devtype
> > > > > nd_volatile
> > > > 
> > > > 
> > > > If all I know is the /dev/pmem device name, how do I find that?
> > > > 
> > > 
> > >     cat $(readlink -f /sys/block/pmem0/device)/../devtype
> > > 
> > > ...this is where 'ndctl list' will get the information.
> > > 
> > 
> > Thanks.
> > 
> > I think we need a section 4 pmem manpage like exists for
> > mem, sd, fd, md, etc., where we can put stuff like this, as well
> > as providing some overview information that will point people to
> > other resources.  I'll give that some thought unless there is one
> > already that I'm not finding.
> > 
> 
> A "pmem" man page sounds like a great idea, I wasn't aware we even
> had an sd man page.

Sorry for being late to respond, but I agree with Linda that this 
naming policy is likely to confuse users.  I also care less about the
current users who use memmap option.  This case is pmem-emulation and
they know what they are doing.

Assuming block device interface is needed (in addition to device-dax)
for volatile range for use-cases like swap device, I wonder if user can
actually specify a right pmem device for swap from OS-install GUI when
both volatile and persistent block devices are listed as /dev/pmemN. 
Sometimes we are restricted with GUI menu.  Some users use GUI all the
time like Windows as well.

Can we differentiate the naming by adding 'v' like 'pmemNv' (if you
can't go with 'vmemN')?  I don't think having 's' for BTT was that bad.
 It's been helpful to tell users that these pmem devices are not byte-
addressable.  I also think that BTT for volatile range makes no sense
(unless emulated as persistent memory by memmap option).

Thanks,
-Toshi

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

On Wed, Jul 5, 2017 at 4:46 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> On Thu, 2017-06-29 at 18:28 -0700, Dan Williams wrote:
>> On Thu, Jun 29, 2017 at 4:14 PM, Linda Knippers <linda.knippers@hpe.c
>> om> wrote:
>> > On 06/29/2017 06:58 PM, Dan Williams wrote:
>> > > On Thu, Jun 29, 2017 at 3:49 PM, Linda Knippers <linda.knippers@h
>> > > pe.com> wrote:
>> > > > > The parent region of the namespace will have a 'volatile'
>> > > > > type:
>> > > > >
>> > > > > # cat /sys/bus/nd/devices/region0/devtype
>> > > > > nd_volatile
>> > > >
>> > > >
>> > > > If all I know is the /dev/pmem device name, how do I find that?
>> > > >
>> > >
>> > >     cat $(readlink -f /sys/block/pmem0/device)/../devtype
>> > >
>> > > ...this is where 'ndctl list' will get the information.
>> > >
>> >
>> > Thanks.
>> >
>> > I think we need a section 4 pmem manpage like exists for
>> > mem, sd, fd, md, etc., where we can put stuff like this, as well
>> > as providing some overview information that will point people to
>> > other resources.  I'll give that some thought unless there is one
>> > already that I'm not finding.
>> >
>>
>> A "pmem" man page sounds like a great idea, I wasn't aware we even
>> had an sd man page.
>
> Sorry for being late to respond, but I agree with Linda that this
> naming policy is likely to confuse users.  I also care less about the
> current users who use memmap option.  This case is pmem-emulation and
> they know what they are doing.
>
> Assuming block device interface is needed (in addition to device-dax)
> for volatile range for use-cases like swap device, I wonder if user can
> actually specify a right pmem device for swap from OS-install GUI when
> both volatile and persistent block devices are listed as /dev/pmemN.
> Sometimes we are restricted with GUI menu.  Some users use GUI all the
> time like Windows as well.
>
> Can we differentiate the naming by adding 'v' like 'pmemNv' (if you
> can't go with 'vmemN')?  I don't think having 's' for BTT was that bad.
>  It's been helpful to tell users that these pmem devices are not byte-
> addressable.  I also think that BTT for volatile range makes no sense
> (unless emulated as persistent memory by memmap option).

I'm more worried about sending the wrong signal the other way. That
users believe that the 'p' means definitely "persistent" when we have
no way to guarantee that.

If it was only memmap= that we had to worry about that would be one
thing, but we apparently have vendors that are shipping "e820-type-12
memory" as their NVDIMM solution [1].

We've also been shipping the policy that 'pmem' may front a volatile
range ever since v4.8 (commit c2f32acdf848 "acpi, nfit: treat virtual
ramdisk SPA as pmem region"). At least now we have the "nd_volatile"
region type. Any change of the device name now is potentially a
regression for environments that are already expecting /dev/pmemX.

As far as I know there are no OS installers that understand pmem. When
they do add support I think it would be straightforward to avoid
confusion and filter "volatile" hosted pmem devices from the install
target list. I don't see this being much different from the confusion
when users can not differentiate their 'sd' device between USB and
SATA. We have symlinks in /dev/disk/by* to make it easier to identify
storage devices, I think it makes sense to add udev rules for
identifying volatile pmem and not try to differentiate this in the
default kernel device name.

[1]: https://github.com/pmem/ndctl/issues/21

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Kani, Toshimitsu]

On Wed, 2017-07-05 at 17:07 -0700, Dan Williams wrote:
> On Wed, Jul 5, 2017 at 4:46 PM, Kani, Toshimitsu <toshi.kani@hpe.com>
> wrote:
> > On Thu, 2017-06-29 at 18:28 -0700, Dan Williams wrote:
 :
> > 
> > Sorry for being late to respond, but I agree with Linda that this
> > naming policy is likely to confuse users.  I also care less about
> > the current users who use memmap option.  This case is pmem-
> > emulation and they know what they are doing.
> > 
> > Assuming block device interface is needed (in addition to device-
> > dax) for volatile range for use-cases like swap device, I wonder if
> > user can actually specify a right pmem device for swap from OS-
> > install GUI when both volatile and persistent block devices are
> > listed as /dev/pmemN.  Sometimes we are restricted with GUI
> > menu.  Some users use GUI all the time like Windows as well.
> > 
> > Can we differentiate the naming by adding 'v' like 'pmemNv' (if you
> > can't go with 'vmemN')?  I don't think having 's' for BTT was that
> > bad.  It's been helpful to tell users that these pmem devices are
> > not byte-addressable.  I also think that BTT for volatile range
> > makes no sense (unless emulated as persistent memory by memmap
> > option).
> 
> I'm more worried about sending the wrong signal the other way. That
> users believe that the 'p' means definitely "persistent" when we have
> no way to guarantee that.

That's a valid point.  But isn't it vendors' responsibility to
guarantee it when their devices are described as persistent in one way
or the other in FW?

> If it was only memmap= that we had to worry about that would be one
> thing, but we apparently have vendors that are shipping "e820-type-12
> memory" as their NVDIMM solution [1].

Type-12 is persistent memory in a non-standard FW interface.  So, it
makes sense to show it as pmem.

> We've also been shipping the policy that 'pmem' may front a volatile
> range ever since v4.8 (commit c2f32acdf848 "acpi, nfit: treat virtual
> ramdisk SPA as pmem region"). At least now we have the "nd_volatile"
> region type. Any change of the device name now is potentially a
> regression for environments that are already expecting /dev/pmemX.

Hmm... I thought this was for mapping ISO image for booting.  Does it
get listed as a regular pmem device and allow user to modify its
content?  I doubt this case being used today, though.  (It was
prototyped on an HPE box and I can check the status if needed.)

> As far as I know there are no OS installers that understand pmem. 

It's actually the other way around.  It was changed not to list pmem
devices since OS cannot boot from a pmem yet...

> When they do add support I think it would be straightforward to avoid
> confusion and filter "volatile" hosted pmem devices from the install
> target list. I don't see this being much different from the confusion
> when users can not differentiate their 'sd' device between USB and
> SATA. 

Right, such changes can be made.  It was just an example that typical
use-cases today do not require additional step to check persistence of
block devices.

> We have symlinks in /dev/disk/by* to make it easier to identify
> storage devices, I think it makes sense to add udev rules for
> identifying volatile pmem and not try to differentiate this in the
> default kernel device name.

I am not sure what might be a good way, but I am concerned because a
single block device naming do not represent both volatile and
persistent media today.

Thanks,
-Toshi

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Dan Williams]

[ adding Jeff, and Johannes ]

On Wed, Jul 5, 2017 at 6:17 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> On Wed, 2017-07-05 at 17:07 -0700, Dan Williams wrote:
[..]
>> We have symlinks in /dev/disk/by* to make it easier to identify
>> storage devices, I think it makes sense to add udev rules for
>> identifying volatile pmem and not try to differentiate this in the
>> default kernel device name.
>
> I am not sure what might be a good way, but I am concerned because a
> single block device naming do not represent both volatile and
> persistent media today.

We do have time to changes this if we find out this is critical. Maybe
it's best to ask Linux distro folks what would be easier for them?

Jeff, Johannes, any thoughts on whether we should produce a
"/dev/vmemX" device when we know the backing memory range is volatile?
In this patch everything shows up as /dev/pmemX and you need to look
elsewhere in sysfs to find that the memory range is defined as
volatile by the NFIT.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[hch]

On Wed, Jul 05, 2017 at 07:08:54PM -0700, Dan Williams wrote:
> [ adding Jeff, and Johannes ]
> 
> On Wed, Jul 5, 2017 at 6:17 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > On Wed, 2017-07-05 at 17:07 -0700, Dan Williams wrote:
> [..]
> >> We have symlinks in /dev/disk/by* to make it easier to identify
> >> storage devices, I think it makes sense to add udev rules for
> >> identifying volatile pmem and not try to differentiate this in the
> >> default kernel device name.
> >
> > I am not sure what might be a good way, but I am concerned because a
> > single block device naming do not represent both volatile and
> > persistent media today.
> 
> We do have time to changes this if we find out this is critical. Maybe
> it's best to ask Linux distro folks what would be easier for them?

I'm not really concerned about it, because SCSI devices for example
might not be persistent as well with ѕcsi_debug, target_core_rd or
volatile qemu devices.

That being said I really don't understand the purpose of these volatile
nfit ranges.  Are they seen in the wild?  If yes what's the use case?
If not why do we even need to support them?

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[Oliver]

On Thu, Jul 6, 2017 at 12:11 PM, hch@lst.de <hch@lst.de> wrote:
> On Wed, Jul 05, 2017 at 07:08:54PM -0700, Dan Williams wrote:
>> [ adding Jeff, and Johannes ]
>>
>> On Wed, Jul 5, 2017 at 6:17 PM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
>> > On Wed, 2017-07-05 at 17:07 -0700, Dan Williams wrote:
>> [..]
>> >> We have symlinks in /dev/disk/by* to make it easier to identify
>> >> storage devices, I think it makes sense to add udev rules for
>> >> identifying volatile pmem and not try to differentiate this in the
>> >> default kernel device name.
>> >
>> > I am not sure what might be a good way, but I am concerned because a
>> > single block device naming do not represent both volatile and
>> > persistent media today.
>>
>> We do have time to changes this if we find out this is critical. Maybe
>> it's best to ask Linux distro folks what would be easier for them?
>
> I'm not really concerned about it, because SCSI devices for example
> might not be persistent as well with ѕcsi_debug, target_core_rd or
> volatile qemu devices.
>
> That being said I really don't understand the purpose of these volatile
> nfit ranges.  Are they seen in the wild?  If yes what's the use case?
> If not why do we even need to support them?

The main use case is provisioning install media for bare metal
servers. Traditionally that's been handled by having the BMC emulate a
USB CD drive. Unfortunately, most BMCs have limited CPU, limited
memory and a wet-string network connection so a host based alternative
is nice to have.

Re: [PATCH v4 12/16] libnvdimm, nfit: enable support for volatile ranges

[hch]

On Thu, Jul 06, 2017 at 12:53:13PM +1000, Oliver wrote:
> The main use case is provisioning install media for bare metal
> servers. Traditionally that's been handled by having the BMC emulate a
> USB CD drive. Unfortunately, most BMCs have limited CPU, limited
> memory and a wet-string network connection so a host based alternative
> is nice to have.

If they are CD replacement they should be marked as read-only, which
would solve any concerns about them being volatile or not.