Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752241AbdFNKzq (ORCPT ); Wed, 14 Jun 2017 06:55:46 -0400 Received: from mx2.suse.de ([195.135.220.15]:53883 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750756AbdFNKzp (ORCPT ); Wed, 14 Jun 2017 06:55:45 -0400 Date: Wed, 14 Jun 2017 12:55:42 +0200 From: Jan Kara To: Dan Williams Cc: linux-nvdimm@lists.01.org, Jan Kara , dm-devel@redhat.com, Matthew Wilcox , x86@kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, Jeff Moyer , Ingo Molnar , viro@zeniv.linux.org.uk, "H. Peter Anvin" , linux-fsdevel@vger.kernel.org, Thomas Gleixner , Ross Zwisler Subject: Re: [PATCH v3 07/14] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush Message-ID: <20170614105542.GE21506@quack2.suse.cz> References: <149703982465.20620.14881139332926778446.stgit@dwillia2-desk3.amr.corp.intel.com> <149703986417.20620.16200765147913100718.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <149703986417.20620.16200765147913100718.stgit@dwillia2-desk3.amr.corp.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3518 Lines: 114 On Fri 09-06-17 13:24:24, Dan Williams wrote: > The clear_pmem() helper simply combines a memset() plus a cache flush. > Now that the flush routine is optionally provided by the dax device > driver we can avoid unnecessary cache management on dax devices fronting > volatile memory. > > With clear_pmem() gone we can follow on with a patch to make pmem cache > management completely defined within the pmem driver. > > Cc: > Cc: Jan Kara > Cc: Jeff Moyer > Cc: Ingo Molnar > Cc: Christoph Hellwig > Cc: "H. Peter Anvin" > Cc: Thomas Gleixner > Cc: Matthew Wilcox > Cc: Ross Zwisler > Signed-off-by: Dan Williams Looks good to me. You can add: Reviewed-by: Jan Kara Honza > --- > arch/x86/include/asm/pmem.h | 13 ------------- > fs/dax.c | 3 ++- > include/linux/pmem.h | 21 --------------------- > 3 files changed, 2 insertions(+), 35 deletions(-) > > diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h > index 60e8edbe0205..f4c119d253f3 100644 > --- a/arch/x86/include/asm/pmem.h > +++ b/arch/x86/include/asm/pmem.h > @@ -65,19 +65,6 @@ static inline void arch_wb_cache_pmem(void *addr, size_t size) > clwb(p); > } > > -/** > - * arch_clear_pmem - zero a PMEM memory range > - * @addr: virtual start address > - * @size: number of bytes to zero > - * > - * Write zeros into the memory range starting at 'addr' for 'size' bytes. > - */ > -static inline void arch_clear_pmem(void *addr, size_t size) > -{ > - memset(addr, 0, size); > - arch_wb_cache_pmem(addr, size); > -} > - > static inline void arch_invalidate_pmem(void *addr, size_t size) > { > clflush_cache_range(addr, size); > diff --git a/fs/dax.c b/fs/dax.c > index 0933fc460ada..554b8e7d921c 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -975,7 +975,8 @@ int __dax_zero_page_range(struct block_device *bdev, > dax_read_unlock(id); > return rc; > } > - clear_pmem(kaddr + offset, size); > + memset(kaddr + offset, 0, size); > + dax_flush(dax_dev, pgoff, kaddr + offset, size); > dax_read_unlock(id); > } > return 0; > diff --git a/include/linux/pmem.h b/include/linux/pmem.h > index 9d542a5600e4..772bd02a5b52 100644 > --- a/include/linux/pmem.h > +++ b/include/linux/pmem.h > @@ -31,11 +31,6 @@ static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n) > BUG(); > } > > -static inline void arch_clear_pmem(void *addr, size_t size) > -{ > - BUG(); > -} > - > static inline void arch_wb_cache_pmem(void *addr, size_t size) > { > BUG(); > @@ -73,22 +68,6 @@ static inline void memcpy_to_pmem(void *dst, const void *src, size_t n) > } > > /** > - * clear_pmem - zero a PMEM memory range > - * @addr: virtual start address > - * @size: number of bytes to zero > - * > - * Write zeros into the memory range starting at 'addr' for 'size' bytes. > - * See blkdev_issue_flush() note for memcpy_to_pmem(). > - */ > -static inline void clear_pmem(void *addr, size_t size) > -{ > - if (arch_has_pmem_api()) > - arch_clear_pmem(addr, size); > - else > - memset(addr, 0, size); > -} > - > -/** > * invalidate_pmem - flush a pmem range from the cache hierarchy > * @addr: virtual start address > * @size: bytes to invalidate (internally aligned to cache line size) > -- Jan Kara SUSE Labs, CR