On Mon, Jun 29, 2020 at 1:29 PM Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com> wrote: > > Architectures like ppc64 provide persistent memory specific barriers > that will ensure that all stores for which the modifications are > written to persistent storage by preceding dcbfps and dcbstps > instructions have updated persistent storage before any data > access or data transfer caused by subsequent instructions is initiated. > This is in addition to the ordering done by wmb() > > Update nvdimm core such that architecture can use barriers other than > wmb to ensure all previous writes are architecturally visible for > the platform buffer flush. > > Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com> > --- > drivers/md/dm-writecache.c | 2 +- > drivers/nvdimm/region_devs.c | 8 ++++---- > include/linux/libnvdimm.h | 4 ++++ > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c > index 74f3c506f084..8c6b6dce64e2 100644 > --- a/drivers/md/dm-writecache.c > +++ b/drivers/md/dm-writecache.c > @@ -536,7 +536,7 @@ static void ssd_commit_superblock(struct dm_writecache > *wc) > static void writecache_commit_flushed(struct dm_writecache *wc, bool > wait_for_ios) > { > if (WC_MODE_PMEM(wc)) > - wmb(); > + arch_pmem_flush_barrier(); > else > ssd_commit_flushed(wc, wait_for_ios); > } > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c > index 4502f9c4708d..b308ad09b63d 100644 > --- a/drivers/nvdimm/region_devs.c > +++ b/drivers/nvdimm/region_devs.c > @@ -1206,13 +1206,13 @@ int generic_nvdimm_flush(struct nd_region *nd_region) > idx = this_cpu_add_return(flush_idx, hash_32(current->pid + idx, 8)); > > /* > - * The first wmb() is needed to 'sfence' all previous writes > - * such that they are architecturally visible for the platform > - * buffer flush. Note that we've already arranged for pmem > + * The first arch_pmem_flush_barrier() is needed to 'sfence' all > + * previous writes such that they are architecturally visible for > + * the platform buffer flush. Note that we've already arranged for > pmem > * writes to avoid the cache via memcpy_flushcache(). The final > * wmb() ensures ordering for the NVDIMM flush write. > */ > - wmb(); > + arch_pmem_flush_barrier(); > for (i = 0; i < nd_region->ndr_mappings; i++) > if (ndrd_get_flush_wpq(ndrd, i, 0)) > writeq(1, ndrd_get_flush_wpq(ndrd, i, idx)); > diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h > index 18da4059be09..66f6c65bd789 100644 > --- a/include/linux/libnvdimm.h > +++ b/include/linux/libnvdimm.h > @@ -286,4 +286,8 @@ static inline void arch_invalidate_pmem(void *addr, > size_t size) > } > #endif > > +#ifndef arch_pmem_flush_barrier > +#define arch_pmem_flush_barrier() wmb() > +#endif
I think it is out of place to define this in libnvdimm.h and it is odd to give it such a long name. The other pmem api helpers like arch_wb_cache_pmem() and arch_invalidate_pmem() are function calls for libnvdimm driver operations, this barrier is just an instruction and is closer to wmb() than the pmem api routine. Since it is a store fence for pmem, so let's just call it pmem_wmb() and define the generic version in include/linux/compiler.h. It should probably also be documented alongside dma_wmb() in Documentation/memory-barriers.txt about why code would use it over wmb(), and why a symmetric pmem_rmb() is not needed.