From: "Boylston, Brian" <brian.boylston@hpe.com>
To: Boaz Harrosh <boaz@plexistor.com>,
        "linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>
CC: "Moreno, Oliver" <oliver.moreno@hpe.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@redhat.com>, Al Viro <viro@ZenIV.linux.org.uk>,
        "H. Peter Anvin" <hpa@zytor.com>, Thomas Gleixner <tglx@linutronix.de>,
        "boylston@burromesa.net" <boylston@burromesa.net>
Subject: RE: [PATCH v2 1/3] introduce memcpy_nocache()
Thread-Topic: [PATCH v2 1/3] introduce memcpy_nocache()
Thread-Index: AQHSL6D1lerMctkfoESlwuQONZJ2RqC7JYqAgAHydHA=
Date: Fri, 28 Oct 2016 01:54:24 +0000
Message-ID: <CS1PR84MB011975E5DCF3B091E53A3FB38EAD0@CS1PR84MB0119.NAMPRD84.PROD.OUTLOOK.COM>
References: <20161026155021.20892-1-brian.boylston@hpe.com>
 <20161026155021.20892-2-brian.boylston@hpe.com>
 <58110959.90901@plexistor.com>
In-Reply-To: <58110959.90901@plexistor.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Oct 2016 01:54:24.2085
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0293
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u9S2Ugjh011808
Content-Length: 4527
Lines: 124

Boaz Harrosh wrote on 2016-10-26:
> On 10/26/2016 06:50 PM, Brian Boylston wrote:
>> Introduce memcpy_nocache() as a memcpy() that avoids the processor cache
>> if possible.  Without arch-specific support, this defaults to just
>> memcpy().  For now, include arch-specific support for x86.
>> 
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: <x86@kernel.org>
>> Cc: Al Viro <viro@ZenIV.linux.org.uk>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Brian Boylston <brian.boylston@hpe.com>
>> Reviewed-by: Toshi Kani <toshi.kani@hpe.com>
>> Reported-by: Oliver Moreno <oliver.moreno@hpe.com>
>> ---
>>  arch/x86/include/asm/string_32.h |  3 +++
>>  arch/x86/include/asm/string_64.h |  3 +++
>>  arch/x86/lib/misc.c              | 12 ++++++++++++
>>  include/linux/string.h           | 15 +++++++++++++++
>>  4 files changed, 33 insertions(+)
>> diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
>> index 3d3e835..64f80c0 100644
>> --- a/arch/x86/include/asm/string_32.h
>> +++ b/arch/x86/include/asm/string_32.h
>> @@ -196,6 +196,9 @@ static inline void *__memcpy3d(void *to, const void *from, size_t len)
>> 
>>  #endif
>> +#define __HAVE_ARCH_MEMCPY_NOCACHE
>> +extern void *memcpy_nocache(void *dest, const void *src, size_t count);
>> +
>>  #define __HAVE_ARCH_MEMMOVE
>>  void *memmove(void *dest, const void *src, size_t n);
>> diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
>> index 90dbbd9..a8fdd55 100644
>> --- a/arch/x86/include/asm/string_64.h
>> +++ b/arch/x86/include/asm/string_64.h
>> @@ -51,6 +51,9 @@ extern void *__memcpy(void *to, const void *from, size_t len);
>>  #define memcpy(dst, src, len) __inline_memcpy((dst), (src), (len))
>>  #endif
>> +#define __HAVE_ARCH_MEMCPY_NOCACHE
>> +extern void *memcpy_nocache(void *dest, const void *src, size_t count);
>> +
>>  #define __HAVE_ARCH_MEMSET
>>  void *memset(void *s, int c, size_t n);
>>  void *__memset(void *s, int c, size_t n);
>> diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
>> index 76b373a..c993ab3 100644
>> --- a/arch/x86/lib/misc.c
>> +++ b/arch/x86/lib/misc.c
>> @@ -1,3 +1,6 @@
>> +#include <linux/export.h>
>> +#include <linux/uaccess.h>
>> +
>>  /*
>>   * Count the digits of @val including a possible sign.
>>   *
>> @@ -19,3 +22,12 @@ int num_digits(int val)
>>  	}
>>  	return d;
>>  }
>> +
>> +#ifdef __HAVE_ARCH_MEMCPY_NOCACHE
>> +void *memcpy_nocache(void *dest, const void *src, size_t count)
>> +{
>> +	__copy_from_user_inatomic_nocache(dest, src, count);
>> +	return dest;
>> +}
>> +EXPORT_SYMBOL(memcpy_nocache);
>> +#endif
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 26b6f6a..7f40c41 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -102,6 +102,21 @@ extern void * memset(void *,int,__kernel_size_t);
>>  #ifndef __HAVE_ARCH_MEMCPY
>>  extern void * memcpy(void *,const void *,__kernel_size_t);
>>  #endif
>> +
>> +#ifndef __HAVE_ARCH_MEMCPY_NOCACHE
>> +/**
>> + * memcpy_nocache - Copy one area of memory to another, avoiding the
>> + * processor cache if possible
>> + * @dest: Where to copy to
>> + * @src: Where to copy from
>> + * @count: The size of the area.
>> + */
>> +static inline void *memcpy_nocache(void *dest, const void *src, size_t count)
>> +{
>> +	return memcpy(dest, src, count);
>> +}
> 
> What about memcpy_to_pmem() in linux/pmem.h it already has all the arch switches.
> 
> Feels bad to add yet just another arch switch over __copy_user_nocache
> 
> Just feels like too many things that do the same thing. Sigh

I agree that this looks like a nicer path.

I had considered adjusting copy_from_iter_nocache() to use memcpy_to_pmem(),
but lib/iov_iter.c doesn't currently #include linux/pmem.h.  Would it be
acceptable to add it?  Also, I wasn't sure if memcpy_to_pmem() would always
mean exactly "memcpy nocache".

I had also considered adjusting copy_from_iter_pmem() (also in linux/pmem.h)
to just use memcpy_to_pmem() directly, but then it can't use the goodness
that is the iterate_and_advance() macro in iov_iter.c.

So, I took a shot with a possibly ill-fated memcpy_nocache().  Thoughts on
either of the above two?  Are these even in line with what you were thinking?

Thanks!
Brian

> 
> Boaz
> 
>> +#endif
>> +
>>  #ifndef __HAVE_ARCH_MEMMOVE
>>  extern void * memmove(void *,const void *,__kernel_size_t);
>>  #endif