| Message ID | 149187075199.40875.9829505688202257056.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2017-04-10 at 17:35 -0700, Dan Williams wrote: > Before we rework the "pmem api" to stop abusing __copy_user_nocache() > for memcpy_to_pmem() we need to fix cases where we may strand dirty > data in the cpu cache. The problem occurs when copy_from_iter_pmem() > is used for arbitrary data transfers from userspace. There is no > guarantee that these transfers, performed by dax_iomap_actor(), will > have aligned destinations or aligned transfer lengths. Backstop the > usage __copy_user_nocache() with explicit cache management in these > unaligned cases. > > Yes, copy_from_iter_pmem() is now too big for an inline, but > addressing that is saved for a later patch that moves the entirety of > the "pmem api" into the pmem driver directly. > > Fixes: 5de490daec8b ("pmem: add copy_from_iter_pmem() and > clear_pmem()") > Cc: <stable@vger.kernel.org> > Cc: <x86@kernel.org> > Cc: Jan Kara <jack@suse.cz> > Cc: Jeff Moyer <jmoyer@redhat.com> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Matthew Wilcox <mawilcox@microsoft.com> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Changes in v3: > * match the implementation to the notes at the top of > __copy_user_nocache (Toshi) > > * Switch to using the IS_ALIGNED() macro to make alignment checks > easier to read and harder to get wrong like they were in v2. (Toshi, > Dan) Thanks Dan! It looks good. -Toshi
On Mon, Apr 10, 2017 at 05:35:02PM -0700, Dan Williams wrote: > Before we rework the "pmem api" to stop abusing __copy_user_nocache() > for memcpy_to_pmem() we need to fix cases where we may strand dirty data > in the cpu cache. The problem occurs when copy_from_iter_pmem() is used > for arbitrary data transfers from userspace. There is no guarantee that > these transfers, performed by dax_iomap_actor(), will have aligned > destinations or aligned transfer lengths. Backstop the usage > __copy_user_nocache() with explicit cache management in these unaligned > cases. > > Yes, copy_from_iter_pmem() is now too big for an inline, but addressing > that is saved for a later patch that moves the entirety of the "pmem > api" into the pmem driver directly. > > Fixes: 5de490daec8b ("pmem: add copy_from_iter_pmem() and clear_pmem()") > Cc: <stable@vger.kernel.org> > Cc: <x86@kernel.org> > Cc: Jan Kara <jack@suse.cz> > Cc: Jeff Moyer <jmoyer@redhat.com> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Matthew Wilcox <mawilcox@microsoft.com> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> This looks good to me, thanks for fixing this. Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h index 2c1ebeb4d737..529bb4a6487a 100644 --- a/arch/x86/include/asm/pmem.h +++ b/arch/x86/include/asm/pmem.h @@ -55,7 +55,8 @@ static inline int arch_memcpy_from_pmem(void *dst, const void *src, size_t n) * @size: number of bytes to write back * * Write back a cache range using the CLWB (cache line write back) - * instruction. + * instruction. Note that @size is internally rounded up to be cache + * line size aligned. */ static inline void arch_wb_cache_pmem(void *addr, size_t size) { @@ -69,15 +70,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 @@ -94,7 +86,35 @@ static inline size_t arch_copy_from_iter_pmem(void *addr, size_t bytes, /* 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)) + /* + * In the iovec case on x86_64 copy_from_iter_nocache() uses + * non-temporal stores for the bulk of the transfer, but we need + * to manually flush if the transfer is unaligned. A cached + * memory copy is used when destination or size is not naturally + * aligned. That is: + * - Require 8-byte alignment when size is 8 bytes or larger. + * - Require 4-byte alignment when size is 4 bytes. + * + * In the non-iovec case the entire destination needs to be + * flushed. + */ + if (iter_is_iovec(i)) { + unsigned long flushed, dest = (unsigned long) addr; + + if (bytes < 8) { + if (!IS_ALIGNED(dest, 4) || (bytes != 4)) + arch_wb_cache_pmem(addr, 1); + } else { + if (!IS_ALIGNED(dest, 8)) { + dest = ALIGN(dest, boot_cpu_data.x86_clflush_size); + arch_wb_cache_pmem(addr, 1); + } + + flushed = dest - (unsigned long) addr; + if (bytes > flushed && !IS_ALIGNED(bytes - flushed, 8)) + arch_wb_cache_pmem(addr + bytes - 1, 1); + } + } else arch_wb_cache_pmem(addr, bytes); return len;
Before we rework the "pmem api" to stop abusing __copy_user_nocache() for memcpy_to_pmem() we need to fix cases where we may strand dirty data in the cpu cache. The problem occurs when copy_from_iter_pmem() is used for arbitrary data transfers from userspace. There is no guarantee that these transfers, performed by dax_iomap_actor(), will have aligned destinations or aligned transfer lengths. Backstop the usage __copy_user_nocache() with explicit cache management in these unaligned cases. Yes, copy_from_iter_pmem() is now too big for an inline, but addressing that is saved for a later patch that moves the entirety of the "pmem api" into the pmem driver directly. Fixes: 5de490daec8b ("pmem: add copy_from_iter_pmem() and clear_pmem()") Cc: <stable@vger.kernel.org> Cc: <x86@kernel.org> Cc: Jan Kara <jack@suse.cz> Cc: Jeff Moyer <jmoyer@redhat.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Matthew Wilcox <mawilcox@microsoft.com> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> Signed-off-by: Toshi Kani <toshi.kani@hpe.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Changes in v3: * match the implementation to the notes at the top of __copy_user_nocache (Toshi) * Switch to using the IS_ALIGNED() macro to make alignment checks easier to read and harder to get wrong like they were in v2. (Toshi, Dan) arch/x86/include/asm/pmem.h | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-)