* [PATCH] x86, pmem: fix broken __copy_user_nocache cache-bypass assumptions
@ 2017-04-06 20:59 Dan Williams
2017-04-07 17:41 ` Kani, Toshimitsu
0 siblings, 1 reply; 3+ messages in thread
From: Dan Williams @ 2017-04-06 20:59 UTC (permalink / raw)
To: linux-nvdimm
Cc: Jan Kara, Toshi Kani, Matthew Wilcox, x86, linux-kernel, stable,
Christoph Hellwig, Jeff Moyer, Ingo Molnar, Al Viro,
H. Peter Anvin, Thomas Gleixner, Ross Zwisler
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: Toshi Kani <toshi.kani@hpe.com>
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: Dan Williams <dan.j.williams@intel.com>
---
I am looking to take this through nvdimm.git along with some other
pending fixes for 4.11.
arch/x86/include/asm/pmem.h | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
index 2c1ebeb4d737..d4d488980bd4 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,34 @@ 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. In the
+ * non-iovec case the entire destination needs to be flushed.
+ */
+ if (iter_is_iovec(i)) {
+ unsigned long dest = (unsigned long) addr;
+
+ /*
+ * If the destination is not 8-byte aligned then
+ * __copy_user_nocache (on x86_64) uses cached copies
+ */
+ if (dest & 8) {
+ arch_wb_cache_pmem(addr, 1);
+ dest = ALIGN(dest, 8);
+ }
+
+ /*
+ * If the remaining transfer length, after accounting
+ * for destination alignment, is not 8-byte aligned
+ * then __copy_user_nocache() falls back to cached
+ * copies for the trailing bytes in the final cacheline
+ * of the transfer.
+ */
+ if ((bytes - (dest - (unsigned long) addr)) & 8)
+ arch_wb_cache_pmem(addr + bytes - 1, 1);
+ } else
arch_wb_cache_pmem(addr, bytes);
return len;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86, pmem: fix broken __copy_user_nocache cache-bypass assumptions
2017-04-06 20:59 [PATCH] x86, pmem: fix broken __copy_user_nocache cache-bypass assumptions Dan Williams
@ 2017-04-07 17:41 ` Kani, Toshimitsu
2017-04-07 23:51 ` Dan Williams
0 siblings, 1 reply; 3+ messages in thread
From: Kani, Toshimitsu @ 2017-04-07 17:41 UTC (permalink / raw)
To: dan.j.williams, linux-nvdimm@lists.01.org
Cc: linux-kernel, jmoyer, tglx, hch, stable, viro, x86, mawilcox,
hpa, mingo, ross.zwisler, jack
On Thu, 2017-04-06 at 13:59 -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.
The change looks good to me. Should we also avoid cache flushing in
the case of size=4B & dest aligned by 4B?
Thanks,
-Toshi
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86, pmem: fix broken __copy_user_nocache cache-bypass assumptions
2017-04-07 17:41 ` Kani, Toshimitsu
@ 2017-04-07 23:51 ` Dan Williams
0 siblings, 0 replies; 3+ messages in thread
From: Dan Williams @ 2017-04-07 23:51 UTC (permalink / raw)
To: Kani, Toshimitsu
Cc: linux-nvdimm@lists.01.org, linux-kernel, jmoyer, tglx, hch,
stable, viro, x86, mawilcox, hpa, mingo, ross.zwisler, jack
On Fri, Apr 7, 2017 at 10:41 AM, Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> On Thu, 2017-04-06 at 13:59 -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.
>
> The change looks good to me. Should we also avoid cache flushing in
> the case of size=4B & dest aligned by 4B?
Yes, since you fixed the 4B aligned case we should skip cache flushing
in that case. I'll send a v2.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-07 23:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 20:59 [PATCH] x86, pmem: fix broken __copy_user_nocache cache-bypass assumptions Dan Williams
2017-04-07 17:41 ` Kani, Toshimitsu
2017-04-07 23:51 ` Dan Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).