From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "ross.zwisler@linux.intel.com" <ross.zwisler@linux.intel.com>,
"kirill.shutemov@linux.intel.com"
<kirill.shutemov@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"xfs@oss.sgi.com" <xfs@oss.sgi.com>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
"willy@linux.intel.com" <willy@linux.intel.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"david@fromorbit.com" <david@fromorbit.com>,
"jack@suse.cz" <jack@suse.cz>
Subject: Re: [PATCH 1/7] Revert "mm: take i_mmap_lock in unmap_mapping_range() for DAX"
Date: Thu, 1 Oct 2015 16:45:04 -0600 [thread overview]
Message-ID: <20151001224504.GA7634@linux.intel.com> (raw)
In-Reply-To: <1443737659.4886.3.camel@intel.com>
On Thu, Oct 01, 2015 at 10:14:22PM +0000, Williams, Dan J wrote:
> Subject: pmem, dax: clean up clear_pmem()
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> Both, __dax_pmd_fault, and clear_pmem() were taking special steps to
> clear memory a page at a time to take advantage of non-temporal
> clear_page() implementations. However, x86_64 does not use
> non-temporal instructions for clear_page(), and arch_clear_pmem() was
> always incurring the cost of __arch_wb_cache_pmem().
>
> Clean up the assumption that doing clear_pmem() a page at a time is more
> performant.
>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> arch/x86/include/asm/pmem.h | 7 +------
> fs/dax.c | 4 +---
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
> index d8ce3ec816ab..1544fabcd7f9 100644
> --- a/arch/x86/include/asm/pmem.h
> +++ b/arch/x86/include/asm/pmem.h
> @@ -132,12 +132,7 @@ static inline void arch_clear_pmem(void __pmem *addr, size_t size)
> {
> void *vaddr = (void __force *)addr;
>
> - /* TODO: implement the zeroing via non-temporal writes */
> - if (size == PAGE_SIZE && ((unsigned long)vaddr & ~PAGE_MASK) == 0)
> - clear_page(vaddr);
> - else
> - memset(vaddr, 0, size);
> -
> + memset(vaddr, 0, size);
> __arch_wb_cache_pmem(vaddr, size);
> }
>
> diff --git a/fs/dax.c b/fs/dax.c
> index b36d6d2e7f87..3faff9227135 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -625,9 +625,7 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
> goto fallback;
>
> if (buffer_unwritten(&bh) || buffer_new(&bh)) {
> - int i;
> - for (i = 0; i < PTRS_PER_PMD; i++)
> - clear_page(kaddr + i * PAGE_SIZE);
> + clear_pmem(kaddr, HPAGE_SIZE);
> count_vm_event(PGMAJFAULT);
> mem_cgroup_count_vm_event(vma->vm_mm, PGMAJFAULT);
> result |= VM_FAULT_MAJOR;
>
This clear_pmem() needs a wmb_pmem() after it. I'll make a quick series with
the clean revert and this guy at the end and try and get them in v4.3 - sound
good?
WARNING: multiple messages have this Message-ID (diff)
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "jack@suse.cz" <jack@suse.cz>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"xfs@oss.sgi.com" <xfs@oss.sgi.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"willy@linux.intel.com" <willy@linux.intel.com>,
"ross.zwisler@linux.intel.com" <ross.zwisler@linux.intel.com>,
"kirill.shutemov@linux.intel.com"
<kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH 1/7] Revert "mm: take i_mmap_lock in unmap_mapping_range() for DAX"
Date: Thu, 1 Oct 2015 16:45:04 -0600 [thread overview]
Message-ID: <20151001224504.GA7634@linux.intel.com> (raw)
In-Reply-To: <1443737659.4886.3.camel@intel.com>
On Thu, Oct 01, 2015 at 10:14:22PM +0000, Williams, Dan J wrote:
> Subject: pmem, dax: clean up clear_pmem()
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> Both, __dax_pmd_fault, and clear_pmem() were taking special steps to
> clear memory a page at a time to take advantage of non-temporal
> clear_page() implementations. However, x86_64 does not use
> non-temporal instructions for clear_page(), and arch_clear_pmem() was
> always incurring the cost of __arch_wb_cache_pmem().
>
> Clean up the assumption that doing clear_pmem() a page at a time is more
> performant.
>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> arch/x86/include/asm/pmem.h | 7 +------
> fs/dax.c | 4 +---
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
> index d8ce3ec816ab..1544fabcd7f9 100644
> --- a/arch/x86/include/asm/pmem.h
> +++ b/arch/x86/include/asm/pmem.h
> @@ -132,12 +132,7 @@ static inline void arch_clear_pmem(void __pmem *addr, size_t size)
> {
> void *vaddr = (void __force *)addr;
>
> - /* TODO: implement the zeroing via non-temporal writes */
> - if (size == PAGE_SIZE && ((unsigned long)vaddr & ~PAGE_MASK) == 0)
> - clear_page(vaddr);
> - else
> - memset(vaddr, 0, size);
> -
> + memset(vaddr, 0, size);
> __arch_wb_cache_pmem(vaddr, size);
> }
>
> diff --git a/fs/dax.c b/fs/dax.c
> index b36d6d2e7f87..3faff9227135 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -625,9 +625,7 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
> goto fallback;
>
> if (buffer_unwritten(&bh) || buffer_new(&bh)) {
> - int i;
> - for (i = 0; i < PTRS_PER_PMD; i++)
> - clear_page(kaddr + i * PAGE_SIZE);
> + clear_pmem(kaddr, HPAGE_SIZE);
> count_vm_event(PGMAJFAULT);
> mem_cgroup_count_vm_event(vma->vm_mm, PGMAJFAULT);
> result |= VM_FAULT_MAJOR;
>
This clear_pmem() needs a wmb_pmem() after it. I'll make a quick series with
the clean revert and this guy at the end and try and get them in v4.3 - sound
good?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
WARNING: multiple messages have this Message-ID (diff)
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "ross.zwisler@linux.intel.com" <ross.zwisler@linux.intel.com>,
"kirill.shutemov@linux.intel.com"
<kirill.shutemov@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"xfs@oss.sgi.com" <xfs@oss.sgi.com>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>,
"willy@linux.intel.com" <willy@linux.intel.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"david@fromorbit.com" <david@fromorbit.com>,
"jack@suse.cz" <jack@suse.cz>
Subject: Re: [PATCH 1/7] Revert "mm: take i_mmap_lock in unmap_mapping_range() for DAX"
Date: Thu, 1 Oct 2015 16:45:04 -0600 [thread overview]
Message-ID: <20151001224504.GA7634@linux.intel.com> (raw)
In-Reply-To: <1443737659.4886.3.camel@intel.com>
On Thu, Oct 01, 2015 at 10:14:22PM +0000, Williams, Dan J wrote:
> Subject: pmem, dax: clean up clear_pmem()
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> Both, __dax_pmd_fault, and clear_pmem() were taking special steps to
> clear memory a page at a time to take advantage of non-temporal
> clear_page() implementations. However, x86_64 does not use
> non-temporal instructions for clear_page(), and arch_clear_pmem() was
> always incurring the cost of __arch_wb_cache_pmem().
>
> Clean up the assumption that doing clear_pmem() a page at a time is more
> performant.
>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> arch/x86/include/asm/pmem.h | 7 +------
> fs/dax.c | 4 +---
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h
> index d8ce3ec816ab..1544fabcd7f9 100644
> --- a/arch/x86/include/asm/pmem.h
> +++ b/arch/x86/include/asm/pmem.h
> @@ -132,12 +132,7 @@ static inline void arch_clear_pmem(void __pmem *addr, size_t size)
> {
> void *vaddr = (void __force *)addr;
>
> - /* TODO: implement the zeroing via non-temporal writes */
> - if (size == PAGE_SIZE && ((unsigned long)vaddr & ~PAGE_MASK) == 0)
> - clear_page(vaddr);
> - else
> - memset(vaddr, 0, size);
> -
> + memset(vaddr, 0, size);
> __arch_wb_cache_pmem(vaddr, size);
> }
>
> diff --git a/fs/dax.c b/fs/dax.c
> index b36d6d2e7f87..3faff9227135 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -625,9 +625,7 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
> goto fallback;
>
> if (buffer_unwritten(&bh) || buffer_new(&bh)) {
> - int i;
> - for (i = 0; i < PTRS_PER_PMD; i++)
> - clear_page(kaddr + i * PAGE_SIZE);
> + clear_pmem(kaddr, HPAGE_SIZE);
> count_vm_event(PGMAJFAULT);
> mem_cgroup_count_vm_event(vma->vm_mm, PGMAJFAULT);
> result |= VM_FAULT_MAJOR;
>
This clear_pmem() needs a wmb_pmem() after it. I'll make a quick series with
the clean revert and this guy at the end and try and get them in v4.3 - sound
good?
next prev parent reply other threads:[~2015-10-01 22:45 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-01 7:46 [PATCH 0/7] xfs, dax: fix the page fault/allocation mess Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 1/7] Revert "mm: take i_mmap_lock in unmap_mapping_range() for DAX" Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 8:35 ` kbuild test robot
2015-10-01 8:35 ` kbuild test robot
2015-10-01 8:35 ` kbuild test robot
2015-10-01 20:27 ` Ross Zwisler
2015-10-01 20:27 ` Ross Zwisler
2015-10-01 20:27 ` Ross Zwisler
2015-10-01 22:14 ` Williams, Dan J
2015-10-01 22:14 ` Williams, Dan J
2015-10-01 22:14 ` Williams, Dan J
2015-10-01 22:45 ` Ross Zwisler [this message]
2015-10-01 22:45 ` Ross Zwisler
2015-10-01 22:45 ` Ross Zwisler
2015-10-01 22:32 ` Dave Chinner
2015-10-01 22:32 ` Dave Chinner
2015-10-01 22:32 ` Dave Chinner
2015-10-01 22:47 ` Ross Zwisler
2015-10-01 22:47 ` Ross Zwisler
2015-10-01 22:47 ` Ross Zwisler
2015-10-01 7:46 ` [PATCH 2/7] Revert "dax: fix race between simultaneous faults" Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 3/7] xfs: fix inode size update overflow in xfs_map_direct() Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 4/7] xfs: introduce BMAPI_ZERO for allocating zeroed extents Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 5/7] xfs: Don't use unwritten extents for DAX Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 6/7] xfs: DAX does not use IO completion callbacks Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` [PATCH 7/7] xfs: add ->pfn_mkwrite support for DAX Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 7:46 ` Dave Chinner
2015-10-01 20:31 ` [PATCH 0/7] xfs, dax: fix the page fault/allocation mess Ross Zwisler
2015-10-01 20:31 ` Ross Zwisler
2015-10-01 20:31 ` Ross Zwisler
2015-10-01 22:54 ` Dave Chinner
2015-10-01 22:54 ` Dave Chinner
2015-10-01 22:54 ` Dave Chinner
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=20151001224504.GA7634@linux.intel.com \
--to=ross.zwisler@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=david@fromorbit.com \
--cc=jack@suse.cz \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=willy@linux.intel.com \
--cc=xfs@oss.sgi.com \
/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: link
Be 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.