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

Groups > linux.kernel > #1355334 > unrolled thread

[PATCH] x86, pmem: use memcpy_mcsafe() for memcpy_from_pmem()

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86, pmem: use memcpy_mcsafe() for memcpy_from_pmem() Dan Williams <dan.j.williams@intel.com> - 2016-03-10 20:20 +0100
    Re: [PATCH] x86, pmem: use memcpy_mcsafe() for memcpy_from_pmem() Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-03-12 00:00 +0100
      Re: [PATCH] x86, pmem: use memcpy_mcsafe() for memcpy_from_pmem() Dan Williams <dan.j.williams@intel.com> - 2016-03-12 00:10 +0100

#1355334 — [PATCH] x86, pmem: use memcpy_mcsafe() for memcpy_from_pmem()

Update the definition of memcpy_from_pmem() to return 0 or -EIO on
error.  Implement x86::arch_memcpy_from_pmem() with memcpy_mcsafe().

Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Andrew, now that all the pre-requisites for this patch are in -next
(tip/core/ras, tip/x86/asm, nvdimm/libnvdimm-for-next) may I ask you to
carry it in -mm?

Alternatively I can do an octopus merge and post a branch, but that
seems messy/risky for me to be merging 3 branches that are still subject
to a merge window disposition.

 arch/x86/include/asm/pmem.h |    9 +++++++++
 drivers/nvdimm/pmem.c       |    4 ++--
 include/linux/pmem.h        |   14 ++++++++------
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index bf8b35d2035a..4df3820535c6 100644
--- a/arch/x86/include/asm/pmem.h
+++ b/arch/x86/include/asm/pmem.h
@@ -47,6 +47,15 @@ static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src,
 		BUG();
 }
 
+static inline int arch_memcpy_from_pmem(void *dst, const void __pmem *src,
+		size_t n)
+{
+	if (static_cpu_has(X86_FEATURE_MCE_RECOVERY))
+		return memcpy_mcsafe(dst, (void __force *) src, n) ? 0 : -EIO;
+	memcpy(dst, (void __force *) src, n);
+	return 0;
+}
+
 /**
  * arch_wmb_pmem - synchronize writes to persistent memory
  *
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index adc387236fe7..2022d08c60ce 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -98,7 +98,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
 		if (unlikely(bad_pmem))
 			rc = -EIO;
 		else {
-			memcpy_from_pmem(mem + off, pmem_addr, len);
+			rc = memcpy_from_pmem(mem + off, pmem_addr, len);
 			flush_dcache_page(page);
 		}
 	} else {
@@ -295,7 +295,7 @@ static int pmem_rw_bytes(struct nd_namespace_common *ndns,
 
 		if (unlikely(is_bad_pmem(&pmem->bb, offset / 512, sz_align)))
 			return -EIO;
-		memcpy_from_pmem(buf, pmem->virt_addr + offset, size);
+		return memcpy_from_pmem(buf, pmem->virt_addr + offset, size);
 	} else {
 		memcpy_to_pmem(pmem->virt_addr + offset, buf, size);
 		wmb_pmem();
diff --git a/include/linux/pmem.h b/include/linux/pmem.h
index 3ec5309e29f3..c46c5cf6538e 100644
--- a/include/linux/pmem.h
+++ b/include/linux/pmem.h
@@ -66,14 +66,16 @@ static inline void arch_invalidate_pmem(void __pmem *addr, size_t size)
 #endif
 
 /*
- * Architectures that define ARCH_HAS_PMEM_API must provide
- * implementations for arch_memcpy_to_pmem(), arch_wmb_pmem(),
- * arch_copy_from_iter_pmem(), arch_clear_pmem(), arch_wb_cache_pmem()
- * and arch_has_wmb_pmem().
+ * memcpy_from_pmem - read from persistent memory with error handling
+ * @dst: destination buffer
+ * @src: source buffer
+ *
+ * Returns 0 on success -EIO on failure.
  */
-static inline void memcpy_from_pmem(void *dst, void __pmem const *src, size_t size)
+static inline int memcpy_from_pmem(void *dst, void __pmem const *src,
+		size_t size)
 {
-	memcpy(dst, (void __force const *) src, size);
+	return arch_memcpy_from_pmem(dst, src, size);
 }
 
 static inline bool arch_has_pmem_api(void)

[toc] | [next] | [standalone]

#1356247

On Thu, Mar 10, 2016 at 11:15:53AM -0800, Dan Williams wrote:
> Update the definition of memcpy_from_pmem() to return 0 or -EIO on
> error.  Implement x86::arch_memcpy_from_pmem() with memcpy_mcsafe().
> 
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Andrew, now that all the pre-requisites for this patch are in -next
> (tip/core/ras, tip/x86/asm, nvdimm/libnvdimm-for-next) may I ask you to
> carry it in -mm?
> 
> Alternatively I can do an octopus merge and post a branch, but that
> seems messy/risky for me to be merging 3 branches that are still subject
> to a merge window disposition.
> 
>  arch/x86/include/asm/pmem.h |    9 +++++++++
>  drivers/nvdimm/pmem.c       |    4 ++--
>  include/linux/pmem.h        |   14 ++++++++------
>  3 files changed, 19 insertions(+), 8 deletions(-)
<>
> diff --git a/include/linux/pmem.h b/include/linux/pmem.h
> index 3ec5309e29f3..c46c5cf6538e 100644
> --- a/include/linux/pmem.h
> +++ b/include/linux/pmem.h
> @@ -66,14 +66,16 @@ static inline void arch_invalidate_pmem(void __pmem *addr, size_t size)
>  #endif
>  
>  /*
> - * Architectures that define ARCH_HAS_PMEM_API must provide
> - * implementations for arch_memcpy_to_pmem(), arch_wmb_pmem(),
> - * arch_copy_from_iter_pmem(), arch_clear_pmem(), arch_wb_cache_pmem()
> - * and arch_has_wmb_pmem().

Why did you delete the above comment?  I believe it adds value?  Or do you
think the fact that another architecture will get compile errors if the arch_*
functions aren't defined is documentation enough?

> + * memcpy_from_pmem - read from persistent memory with error handling
> + * @dst: destination buffer
> + * @src: source buffer

Missing kerneldoc for @size?

Aside from those tiny nits:
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

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

#1356274

On Fri, Mar 11, 2016 at 2:50 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Thu, Mar 10, 2016 at 11:15:53AM -0800, Dan Williams wrote:
>> Update the definition of memcpy_from_pmem() to return 0 or -EIO on
>> error.  Implement x86::arch_memcpy_from_pmem() with memcpy_mcsafe().
>>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> Andrew, now that all the pre-requisites for this patch are in -next
>> (tip/core/ras, tip/x86/asm, nvdimm/libnvdimm-for-next) may I ask you to
>> carry it in -mm?
>>
>> Alternatively I can do an octopus merge and post a branch, but that
>> seems messy/risky for me to be merging 3 branches that are still subject
>> to a merge window disposition.
>>
>>  arch/x86/include/asm/pmem.h |    9 +++++++++
>>  drivers/nvdimm/pmem.c       |    4 ++--
>>  include/linux/pmem.h        |   14 ++++++++------
>>  3 files changed, 19 insertions(+), 8 deletions(-)
> <>
>> diff --git a/include/linux/pmem.h b/include/linux/pmem.h
>> index 3ec5309e29f3..c46c5cf6538e 100644
>> --- a/include/linux/pmem.h
>> +++ b/include/linux/pmem.h
>> @@ -66,14 +66,16 @@ static inline void arch_invalidate_pmem(void __pmem *addr, size_t size)
>>  #endif
>>
>>  /*
>> - * Architectures that define ARCH_HAS_PMEM_API must provide
>> - * implementations for arch_memcpy_to_pmem(), arch_wmb_pmem(),
>> - * arch_copy_from_iter_pmem(), arch_clear_pmem(), arch_wb_cache_pmem()
>> - * and arch_has_wmb_pmem().
>
> Why did you delete the above comment?  I believe it adds value?  Or do you
> think the fact that another architecture will get compile errors if the arch_*
> functions aren't defined is documentation enough?

That and this line-wrapped function-list caused merge conflicts across
the past couple development cycles.  The maintenance overhead to
continue to maintain it didn't seem worth it especially since we have
the compiler to keep people honest.

>> + * memcpy_from_pmem - read from persistent memory with error handling
>> + * @dst: destination buffer
>> + * @src: source buffer
>
> Missing kerneldoc for @size?
>

I'll fix that up in v2 when Tony reworks the memcpy_mcsafe() return value.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web