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 3/3] x86: remove unneeded flush in
 arch_copy_from_iter_pmem()
Thread-Topic: [PATCH v2 3/3] x86: remove unneeded flush in
 arch_copy_from_iter_pmem()
Thread-Index: AQHSL6D0q3Z//bg+lk6VAbyBMEa1Z6C7Jv0AgAH2QVA=
Date: Fri, 28 Oct 2016 01:58:29 +0000
Message-ID: <CS1PR84MB0119143471FF0B524DA10EC08EAD0@CS1PR84MB0119.NAMPRD84.PROD.OUTLOOK.COM>
References: <20161026155021.20892-1-brian.boylston@hpe.com>
 <20161026155021.20892-4-brian.boylston@hpe.com>
 <58110A90.9070900@plexistor.com>
In-Reply-To: <58110A90.9070900@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:58:29.5979
 (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 u9S1wbK4011765
Content-Length: 2577
Lines: 71

Boaz Harrosh wrote on 2016-10-26:
> On 10/26/2016 06:50 PM, Brian Boylston wrote:
>> copy_from_iter_nocache() now uses nocache copies for all types of iovecs
>> on x86, so the flush in arch_copy_from_iter_pmem() is no longer needed.
>> 
>> 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/pmem.h | 19 +------------------
>>  1 file changed, 1 insertion(+), 18 deletions(-)
>> diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
>> index 643eba4..2fbf4ae 100644
>> --- a/arch/x86/include/asm/pmem.h
>> +++ b/arch/x86/include/asm/pmem.h
>> @@ -72,15 +72,6 @@ static inline void arch_wb_cache_pmem(void *addr, size_t size)
>>  		clwb(p);
>>  }
>> -/*
>> - * copy_from_iter_nocache() on x86 only uses non-temporal stores for iovec
>> - * iterators, so for other types (bvec & kvec) we must do a cache write-back.
>> - */
>> -static inline bool __iter_needs_pmem_wb(struct iov_iter *i)
>> -{
>> -	return iter_is_iovec(i) == false;
>> -}
>> -
>>  /**
>>   * arch_copy_from_iter_pmem - copy data from an iterator to PMEM
>>   * @addr:	PMEM destination address
>> @@ -92,15 +83,7 @@ static inline bool __iter_needs_pmem_wb(struct iov_iter *i)
>>  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);
>> -
>> -	if (__iter_needs_pmem_wb(i))
>> -		arch_wb_cache_pmem(addr, bytes);
>> -
>> -	return len;
>> +	return copy_from_iter_nocache(addr, bytes, i);
> 
> I wish you would remove all this completely. Don't see the point for it anymore
> 

At first, I was just targeting bvecs for the splice() case, so I had left the
check and the flush in there for kvecs.  When doing this v2 of the patch set,
I made kvecs nocache as well, so removed the check and flush, but didn't follow
through completely as you suggest.  Will revisit.

Thanks!
Brian

> Thanks a lot for doing this. I have this patch for ages in my trees and was too
> lasy to fight them through. Yes it is a big boost for many workloads.
> 
> Lots of gratitude
> Boaz
> 
>>  }
>>  /**