[dm-devel] [PATCH] dax: remove the pmem_dax_ops->flush abstraction

[Mikulas Patocka]

On Thu, 31 Aug 2017, Dan Williams wrote:

> On Thu, Aug 31, 2017 at 8:04 PM, Mikulas Patocka <mpatocka at redhat.com> wrote:
> >
> >
> > On Thu, 31 Aug 2017, Dan Williams wrote:
> >
> >> On Thu, Aug 31, 2017 at 6:47 PM, Mikulas Patocka <mpatocka at redhat.com> wrote:
> >> > The patch abebfbe2f731 ("dm: add ->flush() dax operation support") is
> >> > buggy. A dm device may be composed of multiple underlying devices and all
> >> > of them need to be flushed. The patch just routes the flush request to the
> >> > first device and ignores the other devices.
> >> >
> >> > It could be fixed by adding more complex logic to the device mapper. But
> >> > there is only one implementation of the method pmem_dax_ops->flush - that
> >> > is pmem_dax_flush() - and it calls arch_wb_cache_pmem(). Consequently, we
> >> > don't need the pmem_dax_ops->flush abstraction at all, we can call
> >> > arch_wb_cache_pmem() directly from dax_flush() because dax_dev->ops->flush
> >> > couldn't ever reach anything different from arch_wb_cache_pmem().
> >>
> >> Unfortunately, this is not true, see usage of DAXDEV_WRITE_CACHE. This
> >> is for platforms that arrange for cpu caches to be flushed on
> >> power-fail, like standalone storage appliances, where it would be a
> >
> > BTW. if the system is capable of flushing caches on power failure, it is
> > system-wide feature, it is not per-device feature. The CPU caches may
> > store data for any persistent memory device.
> >
> > You either have enough remaining power to flush the CPU caches or not.
> 
> Whether you need to flush caches or not can be a per-address range
> attribute and can change over time.

There is only one way to flush the cache (the clwb instruction), you don't 
need a method that abstracts over this instruction. A simple flag or 
static key that skips clwb() is sufficient.

Linux kernel API is changeable, so you don't need to overdesign the API 
for future. If it ever happens that we have a system with two types of 
persistent memory that need different instructions to flush them, we can 
design a proper abstraction. But there is no reason to design the 
abstraction in advance for a situation that will perhaps never happen.

That dm_dax_ops->flush abstraction slows down cache flushing almost twice 
(when used on device mapper) and it is just unacceptable. Unfortunatelly, 
there is no other architecture-independent function to flush cache.

Mikulas