From: Ross Zwisler <ross.zwisler@linux.intel.com> To: Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk> Cc: linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org, Dan Williams <dan.j.williams@intel.com>, Matthew Wilcox <matthew.r.wilcox@intel.com>, Dave Chinner <david@fromorbit.com>, "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org Subject: Re: [PATCH v3 5/7] pmem: add copy_from_iter_pmem() and clear_pmem() Date: Mon, 17 Aug 2015 16:23:18 -0600 [thread overview] Message-ID: <1439850198.11296.5.camel@linux.intel.com> (raw) In-Reply-To: <20150817191016.GC6752@lst.de> On Mon, 2015-08-17 at 21:10 +0200, Christoph Hellwig wrote: > > #include <linux/uaccess.h> > > +#include <linux/uio.h> > > + > > Can we keep this in linux/pmem.h? I'm pretty sure the stubs would need > it as well, and even if they don't it'll keep the includes consistent. Sure. > > +{ > > + size_t len; > > + > > + len = copy_from_iter_nocache((void __force *)addr, bytes, i); > > + > > + /* > > + * 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. > > Shouldn't we fi that? I'm not sure - When Al make copy_from_iter_nocache() it was just a copy of copy_from_iter(), with the iovec case changed to use __copy_from_user_nocache(). The other cases use memcpy_from_page() and memcpy(). To have everything do non-temporal stores we'd probably need to make non-temporal versions of each of those (alluded to by Al's comment in the copy_from_iter_nocache() commit: "BTW, do we want memcpy_nocache()?"). > > + */ > > + if (iter_is_iovec(i) == false) > > + __arch_wb_cache_pmem(addr, bytes); > > And if not and iter_needs_pmem_wb helper to encode this knowledge would > be useful. Maybe this should be the short-term solution, and I'll add a TODO to fix the copy_from_iter_nocache() implementation as described above so we can always have non-temporal stores? > > +static inline void arch_clear_pmem(void __pmem *addr, size_t size) > > +{ > > + /* TODO: implement the zeroing via non-temporal writes */ > > + if (size == PAGE_SIZE && ((unsigned long)addr & ~PAGE_MASK) == 0) > > + clear_page((void __force *)addr); > > + else > > + memset((void __force *)addr, 0, size); > > + > > + __arch_wb_cache_pmem(addr, size); > > Please add a local vaiable so that the __force casting is only needed > once. Same for other functions with this pattern. Sure.
WARNING: multiple messages have this Message-ID (diff)
From: Ross Zwisler <ross.zwisler@linux.intel.com> To: Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk> Cc: linux-kernel@vger.kernel.org, linux-nvdimm@ml01.01.org, Dan Williams <dan.j.williams@intel.com>, Matthew Wilcox <matthew.r.wilcox@intel.com>, Dave Chinner <david@fromorbit.com>, "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org Subject: Re: [PATCH v3 5/7] pmem: add copy_from_iter_pmem() and clear_pmem() Date: Mon, 17 Aug 2015 16:23:18 -0600 [thread overview] Message-ID: <1439850198.11296.5.camel@linux.intel.com> (raw) In-Reply-To: <20150817191016.GC6752@lst.de> On Mon, 2015-08-17 at 21:10 +0200, Christoph Hellwig wrote: > > #include <linux/uaccess.h> > > +#include <linux/uio.h> > > + > > Can we keep this in linux/pmem.h? I'm pretty sure the stubs would need > it as well, and even if they don't it'll keep the includes consistent. Sure. > > +{ > > + size_t len; > > + > > + len = copy_from_iter_nocache((void __force *)addr, bytes, i); > > + > > + /* > > + * 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. > > Shouldn't we fi that? I'm not sure - When Al make copy_from_iter_nocache() it was just a copy of copy_from_iter(), with the iovec case changed to use __copy_from_user_nocache(). The other cases use memcpy_from_page() and memcpy(). To have everything do non-temporal stores we'd probably need to make non-temporal versions of each of those (alluded to by Al's comment in the copy_from_iter_nocache() commit: "BTW, do we want memcpy_nocache()?"). > > + */ > > + if (iter_is_iovec(i) == false) > > + __arch_wb_cache_pmem(addr, bytes); > > And if not and iter_needs_pmem_wb helper to encode this knowledge would > be useful. Maybe this should be the short-term solution, and I'll add a TODO to fix the copy_from_iter_nocache() implementation as described above so we can always have non-temporal stores? > > +static inline void arch_clear_pmem(void __pmem *addr, size_t size) > > +{ > > + /* TODO: implement the zeroing via non-temporal writes */ > > + if (size == PAGE_SIZE && ((unsigned long)addr & ~PAGE_MASK) == 0) > > + clear_page((void __force *)addr); > > + else > > + memset((void __force *)addr, 0, size); > > + > > + __arch_wb_cache_pmem(addr, size); > > Please add a local vaiable so that the __force casting is only needed > once. Same for other functions with this pattern. Sure.
next prev parent reply other threads:[~2015-08-17 22:23 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-08-17 18:30 [PATCH v3 0/7] dax: I/O path enhancements Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 18:30 ` [PATCH v3 1/7] brd: make rd_size static Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 19:03 ` Christoph Hellwig 2015-08-17 19:03 ` Christoph Hellwig 2015-08-17 18:30 ` [PATCH v3 2/7] pmem, x86: move x86 PMEM API to new pmem.h header Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 18:30 ` [PATCH v3 3/7] pmem: remove layer when calling arch_has_wmb_pmem() Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 18:30 ` [PATCH v3 4/7] pmem, x86: clean up conditional pmem includes Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 19:04 ` Christoph Hellwig 2015-08-17 19:04 ` Christoph Hellwig 2015-08-17 18:30 ` [PATCH v3 5/7] pmem: add copy_from_iter_pmem() and clear_pmem() Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 19:10 ` Christoph Hellwig 2015-08-17 19:10 ` Christoph Hellwig 2015-08-17 22:23 ` Ross Zwisler [this message] 2015-08-17 22:23 ` Ross Zwisler 2015-08-17 18:30 ` [PATCH v3 6/7] dax: update I/O path to do proper PMEM flushing Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 19:11 ` Christoph Hellwig 2015-08-17 19:11 ` Christoph Hellwig 2015-08-17 18:30 ` [PATCH v3 7/7] pmem, dax: have direct_access use __pmem annotation Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 18:30 ` Ross Zwisler 2015-08-17 19:11 ` Christoph Hellwig 2015-08-17 19:11 ` Christoph Hellwig 2015-08-17 19:11 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1439850198.11296.5.camel@linux.intel.com \ --to=ross.zwisler@linux.intel.com \ --cc=dan.j.williams@intel.com \ --cc=david@fromorbit.com \ --cc=hch@lst.de \ --cc=hpa@zytor.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ --cc=matthew.r.wilcox@intel.com \ --cc=mingo@redhat.com \ --cc=tglx@linutronix.de \ --cc=viro@zeniv.linux.org.uk \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.