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

[Dan Williams]

On Fri, Mar 11, 2016 at 2:50 PM, Ross Zwisler
<ross.zwisler@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
>> Cc: Tony Luck <tony.luck@xxxxxxxxx>
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
>> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx>
>> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>> ---
>> 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.