[PATCH] dax: Fix use of zero page

[PATCH] dax: Fix use of zero page

[Matthew Wilcox]

I plucked this patch from my XArray work.  It seems self-contained enough
that it could go into the DAX tree for merging this cycle.

>From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
From: Matthew Wilcox <mawilcox@microsoft.com>
Date: Thu, 29 Mar 2018 22:41:18 -0400
Subject: [PATCH] dax: Fix use of zero page

Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
works on MIPS and s390.

Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
 fs/dax.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 6a26626f20f3..f643e8fc34ee 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1006,17 +1006,9 @@ static vm_fault_t dax_load_hole(struct address_space *mapping, void *entry,
 	struct inode *inode = mapping->host;
 	unsigned long vaddr = vmf->address;
 	vm_fault_t ret = VM_FAULT_NOPAGE;
-	struct page *zero_page;
 	void *entry2;
-	pfn_t pfn;
-
-	zero_page = ZERO_PAGE(0);
-	if (unlikely(!zero_page)) {
-		ret = VM_FAULT_OOM;
-		goto out;
-	}
+	pfn_t pfn = pfn_to_pfn_t(my_zero_pfn(vaddr));
 
-	pfn = page_to_pfn_t(zero_page);
 	entry2 = dax_insert_mapping_entry(mapping, vmf, entry, pfn,
 			DAX_ZERO_PAGE, false);
 	if (IS_ERR(entry2)) {
-- 
2.17.0

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Ross Zwisler]

On Thu, May 17, 2018 at 11:37:11AM -0700, Matthew Wilcox wrote:
> 
> I plucked this patch from my XArray work.  It seems self-contained enough
> that it could go into the DAX tree for merging this cycle.
> 
> From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> From: Matthew Wilcox <mawilcox@microsoft.com>
> Date: Thu, 29 Mar 2018 22:41:18 -0400
> Subject: [PATCH] dax: Fix use of zero page
> 
> Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> works on MIPS and s390.
> 
> Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>

Yep, this looks fine.

Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Ross Zwisler]

On Thu, May 17, 2018 at 01:24:00PM -0600, Ross Zwisler wrote:
> On Thu, May 17, 2018 at 11:37:11AM -0700, Matthew Wilcox wrote:
> > 
> > I plucked this patch from my XArray work.  It seems self-contained enough
> > that it could go into the DAX tree for merging this cycle.
> > 
> > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> > From: Matthew Wilcox <mawilcox@microsoft.com>
> > Date: Thu, 29 Mar 2018 22:41:18 -0400
> > Subject: [PATCH] dax: Fix use of zero page
> > 
> > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> > works on MIPS and s390.
> > 
> > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> 
> Yep, this looks fine.
> 
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

Huh, actually, it looks like this relies on patch 01/63 of your full Xarray
series where you s/RADIX_DAX_ZERO_PAGE/DAX_ZERO_PAGE/g.

Ditto for the 2nd patch you sent today.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Matthew Wilcox]

On Thu, May 17, 2018 at 01:29:10PM -0600, Ross Zwisler wrote:
> On Thu, May 17, 2018 at 01:24:00PM -0600, Ross Zwisler wrote:
> > On Thu, May 17, 2018 at 11:37:11AM -0700, Matthew Wilcox wrote:
> > > 
> > > I plucked this patch from my XArray work.  It seems self-contained enough
> > > that it could go into the DAX tree for merging this cycle.
> > > 
> > > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > Date: Thu, 29 Mar 2018 22:41:18 -0400
> > > Subject: [PATCH] dax: Fix use of zero page
> > > 
> > > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> > > works on MIPS and s390.
> > > 
> > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> > 
> > Yep, this looks fine.
> > 
> > Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> 
> Huh, actually, it looks like this relies on patch 01/63 of your full Xarray
> series where you s/RADIX_DAX_ZERO_PAGE/DAX_ZERO_PAGE/g.
> 
> Ditto for the 2nd patch you sent today.

Argh, thanks.  I can respin them against linux-next if you like.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Ross Zwisler]

On Thu, May 17, 2018 at 12:57:39PM -0700, Matthew Wilcox wrote:
> On Thu, May 17, 2018 at 01:29:10PM -0600, Ross Zwisler wrote:
> > On Thu, May 17, 2018 at 01:24:00PM -0600, Ross Zwisler wrote:
> > > On Thu, May 17, 2018 at 11:37:11AM -0700, Matthew Wilcox wrote:
> > > > 
> > > > I plucked this patch from my XArray work.  It seems self-contained enough
> > > > that it could go into the DAX tree for merging this cycle.
> > > > 
> > > > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> > > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > > Date: Thu, 29 Mar 2018 22:41:18 -0400
> > > > Subject: [PATCH] dax: Fix use of zero page
> > > > 
> > > > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> > > > works on MIPS and s390.
> > > > 
> > > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> > > 
> > > Yep, this looks fine.
> > > 
> > > Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > 
> > Huh, actually, it looks like this relies on patch 01/63 of your full Xarray
> > series where you s/RADIX_DAX_ZERO_PAGE/DAX_ZERO_PAGE/g.
> > 
> > Ditto for the 2nd patch you sent today.
> 
> Argh, thanks.  I can respin them against linux-next if you like.

Yep, that would be awesome, thanks.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Dan Williams]

On Thu, May 17, 2018 at 11:37 AM, Matthew Wilcox <willy@infradead.org> wrote:
>
> I plucked this patch from my XArray work.  It seems self-contained enough
> that it could go into the DAX tree for merging this cycle.
>
> From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> From: Matthew Wilcox <mawilcox@microsoft.com>
> Date: Thu, 29 Mar 2018 22:41:18 -0400
> Subject: [PATCH] dax: Fix use of zero page
>
> Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> works on MIPS and s390.
>
> Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>

I'm being thick and / or lazy, what's the user visible effect of this fix?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Matthew Wilcox]

On Thu, May 17, 2018 at 12:32:07PM -0700, Dan Williams wrote:
> On Thu, May 17, 2018 at 11:37 AM, Matthew Wilcox <willy@infradead.org> wrote:
> >
> > I plucked this patch from my XArray work.  It seems self-contained enough
> > that it could go into the DAX tree for merging this cycle.
> >
> > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> > From: Matthew Wilcox <mawilcox@microsoft.com>
> > Date: Thu, 29 Mar 2018 22:41:18 -0400
> > Subject: [PATCH] dax: Fix use of zero page
> >
> > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> > works on MIPS and s390.
> >
> > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> 
> I'm being thick and / or lazy, what's the user visible effect of this fix?

For s390 it appears to be a performance issue:

Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date:   Mon Oct 25 16:10:07 2010 +0200

    [S390] zero page cache synonyms
    
    If the zero page is mapped to virtual user space addresses that differ
    only in bit 2^12 or 2^13 we get L1 cache synonyms which can affect
    performance. Follow the mips model and use multiple zero pages to avoid
    the synonyms.

MIPS' use of multiple ZERO_PAGEs predates git history.  Given the
history of MIPS' caches behaving in incredibly weird ways, I'd assume
that getting this wrong results in miniature black holes forming and/or
the CPU calculating the largest prime number.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Dan Williams]

On Thu, May 17, 2018 at 12:56 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Thu, May 17, 2018 at 12:32:07PM -0700, Dan Williams wrote:
>> On Thu, May 17, 2018 at 11:37 AM, Matthew Wilcox <willy@infradead.org> wrote:
>> >
>> > I plucked this patch from my XArray work.  It seems self-contained enough
>> > that it could go into the DAX tree for merging this cycle.
>> >
>> > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
>> > From: Matthew Wilcox <mawilcox@microsoft.com>
>> > Date: Thu, 29 Mar 2018 22:41:18 -0400
>> > Subject: [PATCH] dax: Fix use of zero page
>> >
>> > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
>> > works on MIPS and s390.
>> >
>> > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
>>
>> I'm being thick and / or lazy, what's the user visible effect of this fix?
>
> For s390 it appears to be a performance issue:
>
> Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date:   Mon Oct 25 16:10:07 2010 +0200
>
>     [S390] zero page cache synonyms
>
>     If the zero page is mapped to virtual user space addresses that differ
>     only in bit 2^12 or 2^13 we get L1 cache synonyms which can affect
>     performance. Follow the mips model and use multiple zero pages to avoid
>     the synonyms.
>
> MIPS' use of multiple ZERO_PAGEs predates git history.  Given the
> history of MIPS' caches behaving in incredibly weird ways, I'd assume
> that getting this wrong results in miniature black holes forming and/or
> the CPU calculating the largest prime number.

Unless I am missing something I think this sounds like 4.18-rc1
material with a cc: stable. Last I heard no one is really using
dccssblk + dax, and MIPS has no way to describe pmem outside of
memmap= which is only a development tool.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Ross Zwisler]

On Thu, May 17, 2018 at 01:03:48PM -0700, Dan Williams wrote:
> On Thu, May 17, 2018 at 12:56 PM, Matthew Wilcox <willy@infradead.org> wrote:
> > On Thu, May 17, 2018 at 12:32:07PM -0700, Dan Williams wrote:
> >> On Thu, May 17, 2018 at 11:37 AM, Matthew Wilcox <willy@infradead.org> wrote:
> >> >
> >> > I plucked this patch from my XArray work.  It seems self-contained enough
> >> > that it could go into the DAX tree for merging this cycle.
> >> >
> >> > From 8cb56f4ba36af38814ca7b8ba030a66384e59a21 Mon Sep 17 00:00:00 2001
> >> > From: Matthew Wilcox <mawilcox@microsoft.com>
> >> > Date: Thu, 29 Mar 2018 22:41:18 -0400
> >> > Subject: [PATCH] dax: Fix use of zero page
> >> >
> >> > Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> >> > works on MIPS and s390.
> >> >
> >> > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> >>
> >> I'm being thick and / or lazy, what's the user visible effect of this fix?
> >
> > For s390 it appears to be a performance issue:
> >
> > Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
> > Date:   Mon Oct 25 16:10:07 2010 +0200
> >
> >     [S390] zero page cache synonyms
> >
> >     If the zero page is mapped to virtual user space addresses that differ
> >     only in bit 2^12 or 2^13 we get L1 cache synonyms which can affect
> >     performance. Follow the mips model and use multiple zero pages to avoid
> >     the synonyms.
> >
> > MIPS' use of multiple ZERO_PAGEs predates git history.  Given the
> > history of MIPS' caches behaving in incredibly weird ways, I'd assume
> > that getting this wrong results in miniature black holes forming and/or
> > the CPU calculating the largest prime number.
> 
> Unless I am missing something I think this sounds like 4.18-rc1
> material with a cc: stable. Last I heard no one is really using
> dccssblk + dax, and MIPS has no way to describe pmem outside of
> memmap= which is only a development tool.

Yea, I agree that this is v4.18 material.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH] dax: Fix use of zero page

[Matthew Wilcox]

This patch got caught in the crossfire and didn't make it into 4.18
as planned.  Can it make 4.19 please?

---- >8 ----

Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
works on MIPS and s390.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 fs/dax.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 467601a134bc..c7917b46a75b 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1093,21 +1093,12 @@ static vm_fault_t dax_load_hole(struct address_space *mapping, void *entry,
 {
 	struct inode *inode = mapping->host;
 	unsigned long vaddr = vmf->address;
-	vm_fault_t ret = VM_FAULT_NOPAGE;
-	struct page *zero_page;
-	pfn_t pfn;
-
-	zero_page = ZERO_PAGE(0);
-	if (unlikely(!zero_page)) {
-		ret = VM_FAULT_OOM;
-		goto out;
-	}
+	pfn_t pfn = pfn_to_pfn_t(my_zero_pfn(vaddr));
+	vm_fault_t ret;
 
-	pfn = page_to_pfn_t(zero_page);
 	dax_insert_mapping_entry(mapping, vmf, entry, pfn, RADIX_DAX_ZERO_PAGE,
 			false);
 	ret = vmf_insert_mixed(vmf->vma, vaddr, pfn);
-out:
 	trace_dax_load_hole(inode, vmf, ret);
 	return ret;
 }
-- 
2.17.1

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] dax: Fix use of zero page

[Dave Jiang]

On 08/30/2018 09:20 AM, Matthew Wilcox wrote:
> 
> This patch got caught in the crossfire and didn't make it into 4.18
> as planned.  Can it make 4.19 please?
> 
> ---- >8 ----
> 
> Use my_zero_pfn instead of ZERO_PAGE, and pass the vaddr to it so it
> works on MIPS and s390.
> 
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

Applied

> ---
>  fs/dax.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 467601a134bc..c7917b46a75b 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1093,21 +1093,12 @@ static vm_fault_t dax_load_hole(struct address_space *mapping, void *entry,
>  {
>  	struct inode *inode = mapping->host;
>  	unsigned long vaddr = vmf->address;
> -	vm_fault_t ret = VM_FAULT_NOPAGE;
> -	struct page *zero_page;
> -	pfn_t pfn;
> -
> -	zero_page = ZERO_PAGE(0);
> -	if (unlikely(!zero_page)) {
> -		ret = VM_FAULT_OOM;
> -		goto out;
> -	}
> +	pfn_t pfn = pfn_to_pfn_t(my_zero_pfn(vaddr));
> +	vm_fault_t ret;
>  
> -	pfn = page_to_pfn_t(zero_page);
>  	dax_insert_mapping_entry(mapping, vmf, entry, pfn, RADIX_DAX_ZERO_PAGE,
>  			false);
>  	ret = vmf_insert_mixed(vmf->vma, vaddr, pfn);
> -out:
>  	trace_dax_load_hole(inode, vmf, ret);
>  	return ret;
>  }
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm