diff mbox

[v9,10/10] drivers/block/pmem: Map NVDIMM with ioremap_wt()

Message ID 1431551151-19124-11-git-send-email-toshi.kani@hp.com (mailing list archive)
State Superseded
Headers show

Commit Message

Toshi Kani May 13, 2015, 9:05 p.m. UTC
The pmem driver maps NVDIMM with ioremap_nocache() as we cannot
write back the contents of the CPU caches in case of a crash.

This patch changes to use ioremap_wt(), which provides uncached
writes but cached reads, for improving read performance.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/block/pmem.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dan Williams May 14, 2015, 9:52 p.m. UTC | #1
On Wed, May 13, 2015 at 2:05 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> The pmem driver maps NVDIMM with ioremap_nocache() as we cannot
> write back the contents of the CPU caches in case of a crash.
>
> This patch changes to use ioremap_wt(), which provides uncached
> writes but cached reads, for improving read performance.

I'm thinking that for the libnd integration we don't want the pmem
driver hard coding the cache-policy decision.  This is something that
should be specified to nd_pmem_region_create().  Especially
considering that platform firmware tables (NFIT) may specify the cache
policy for the range.  As Matthew Wilcox mentioned offline we also
must match the DAX-to-mmap cache policy with the policy for the driver
mapping  for architectures that are not capable of multiple mappings
of the same physical address with different policies.
Toshi Kani May 14, 2015, 10:20 p.m. UTC | #2
On Thu, 2015-05-14 at 14:52 -0700, Dan Williams wrote:
> On Wed, May 13, 2015 at 2:05 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > The pmem driver maps NVDIMM with ioremap_nocache() as we cannot
> > write back the contents of the CPU caches in case of a crash.
> >
> > This patch changes to use ioremap_wt(), which provides uncached
> > writes but cached reads, for improving read performance.
> 
> I'm thinking that for the libnd integration we don't want the pmem
> driver hard coding the cache-policy decision.  This is something that
> should be specified to nd_pmem_region_create().  Especially
> considering that platform firmware tables (NFIT) may specify the cache
> policy for the range.  As Matthew Wilcox mentioned offline we also
> must match the DAX-to-mmap cache policy with the policy for the driver
> mapping  for architectures that are not capable of multiple mappings
> of the same physical address with different policies.

Agreed.  I believe this hardcoded ioremap is temporary (either UC- or
WT), and we need to allow NFIT or user to specify a map type, such as
WB.

Thanks,
-Toshi
diff mbox

Patch

diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
index eabf4a8..095dfaa 100644
--- a/drivers/block/pmem.c
+++ b/drivers/block/pmem.c
@@ -139,11 +139,11 @@  static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res)
 	}
 
 	/*
-	 * Map the memory as non-cachable, as we can't write back the contents
+	 * Map the memory as write-through, as we can't write back the contents
 	 * of the CPU caches in case of a crash.
 	 */
 	err = -ENOMEM;
-	pmem->virt_addr = ioremap_nocache(pmem->phys_addr, pmem->size);
+	pmem->virt_addr = ioremap_wt(pmem->phys_addr, pmem->size);
 	if (!pmem->virt_addr)
 		goto out_release_region;