On Tue, May 22, 2018 at 12:19 PM, Mike Snitzer <snitzer(a)redhat.com> wrote:
On Tue, May 22 2018 at 3:00pm -0400,
Dan Williams <dan.j.williams(a)intel.com> wrote:
> On Tue, May 22, 2018 at 11:41 AM, Mike Snitzer <snitzer(a)redhat.com> wrote:
> > On Tue, May 22 2018 at 2:39am -0400,
> > Christoph Hellwig <hch(a)infradead.org> wrote:
> >> On Sat, May 19, 2018 at 07:25:07AM +0200, Mikulas Patocka wrote:
> >> > Use new API for flushing persistent memory.
> >> The sentence doesnt make much sense. 'A new API', 'A better
> >> abstraction' maybe?
> >> >
> >> > The problem is this:
> >> > * on X86-64, non-temporal stores have the best performance
> >> > * ARM64 doesn't have non-temporal stores, so we must flush cache.
> >> > should flush cache as late as possible, because it performs better
> >> > way.
> >> >
> >> > We introduce functions pmem_memcpy, pmem_flush and pmem_commit. To
> >> > data persistently, all three functions must be called.
> >> >
> >> > The macro pmem_assign may be used instead of pmem_memcpy. pmem_assign
> >> > (unlike pmem_memcpy) guarantees that 8-byte values are written
> >> >
> >> > On X86, pmem_memcpy is memcpy_flushcache, pmem_flush is empty and
> >> > pmem_commit is wmb.
> >> >
> >> > On ARM64, pmem_memcpy is memcpy, pmem_flush is arch_wb_cache_pmem and
> >> > pmem_commit is empty.
> >> All these should be provided by the pmem layer, and be properly
> >> documented. And be sorted before adding your new target that uses
> >> them.
> > I don't see that as a hard requirement. Mikulas did the work to figure
> > out what is more optimal on x86_64 vs amd64. It makes a difference for
> > his target and that is sufficient to carry it locally until/when it is
> > either elevated to pmem.
> > We cannot even get x86 and swait maintainers to reply to repeat requests
> > for review. Stacking up further deps on pmem isn't high on my list.
> Except I'm being responsive.
Except you're looking to immediately punt to linux-arm-kernel ;)
Well, I'm not, not really. I'm saying drop ARM support, it's not ready.
> I agree with Christoph that we should
> build pmem helpers at an architecture level and not per-driver. Let's
> make this driver depend on ARCH_HAS_PMEM_API and require ARM to catch
> up to x86 in this space. We already have PowerPC enabling PMEM API, so
> I don't see an unreasonable barrier to ask the same of ARM. This patch
> is not even cc'd to linux-arm-kernel. Has the subject been broached
> with them?
No idea. Not by me.
The thing is, I'm no expert in pmem. You are. Coordinating the change
with ARM et al feels unnecessarily limiting and quicky moves outside my
Serious question: Why can't this code land in this dm-writecache target
and then be lifted (or obsoleted)?
Because we already have an API, and we don't want to promote local
solutions to global problems, or carry unnecessary technical debt.
But if you think it worthwhile to force ARM to step up then fine. That
does limit the availability of using writecache on ARM while they get
the PMEM API together.
I'll do whatever you want.. just put the smack down and tell me how it
I'd say just control the variables you can control. Drop the ARM
support if you want to move forward and propose extensions / updates
to the pmem api for x86 and I'll help push those since I was involved
in pushing the x86 pmem api in the first instance. That way you don't
need to touch this driver as new archs add their pmem api enabling.