Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Andrew Morton]

On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:

> When doing some numa tests on powerpc, I triggered an oops bug. I find
> it is caused by using page->_last_cpupid.  It should be initialized as
> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
> And finally cause an oops bug in task_numa_group(), since the online cpu is
> less than possible cpu.

I grabbed this.  I added this to the changelog:

: PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
: support a large physical address region, up to 2^46 but small section size
: (2^24).  So when NR_CPUS grows up, it is easily to cause
: not-in-page-flags.

to hopefully address Peter's observation.

How should we proceed with this?  I'm getting the impression that numa
balancing on ppc is a dead duck in 3.14, so perhaps this and 

powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch

are 3.15-rc1 material?

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Aneesh Kumar K.V]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>
>> When doing some numa tests on powerpc, I triggered an oops bug. I find
>> it is caused by using page->_last_cpupid.  It should be initialized as
>> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
>> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
>> And finally cause an oops bug in task_numa_group(), since the online cpu is
>> less than possible cpu.
>
> I grabbed this.  I added this to the changelog:
>
> : PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
> : support a large physical address region, up to 2^46 but small section size
> : (2^24).  So when NR_CPUS grows up, it is easily to cause
> : not-in-page-flags.
>
> to hopefully address Peter's observation.
>
> How should we proceed with this?  I'm getting the impression that numa
> balancing on ppc is a dead duck in 3.14, so perhaps this and 
>
> powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
> mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
> mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
>

All these are already in 3.14  ?

> are 3.15-rc1 material?
>

We should push the first hunk to 3.14. I will wait for Liu to redo the
patch. BTW this should happen only when SPARSE_VMEMMAP is not
specified. Srikar had reported the issue here

http://mid.gmane.org/20140219180200.GA29257@linux.vnet.ibm.com

#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
#define SECTIONS_WIDTH		SECTIONS_SHIFT
#else
#define SECTIONS_WIDTH		0
#endif

-aneesh

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Andrew Morton]

On Wed, 26 Feb 2014 13:22:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> Andrew Morton <akpm@linux-foundation.org> writes:
> 
> > On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
> >
> >> When doing some numa tests on powerpc, I triggered an oops bug. I find
> >> it is caused by using page->_last_cpupid.  It should be initialized as
> >> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
> >> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
> >> And finally cause an oops bug in task_numa_group(), since the online cpu is
> >> less than possible cpu.
> >
> > I grabbed this.  I added this to the changelog:
> >
> > : PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
> > : support a large physical address region, up to 2^46 but small section size
> > : (2^24).  So when NR_CPUS grows up, it is easily to cause
> > : not-in-page-flags.
> >
> > to hopefully address Peter's observation.
> >
> > How should we proceed with this?  I'm getting the impression that numa
> > balancing on ppc is a dead duck in 3.14, so perhaps this and 
> >
> > powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
> > mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
> > mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
> >
> 
> All these are already in 3.14  ?

Yes.

> > are 3.15-rc1 material?
> >
> 
> We should push the first hunk to 3.14. I will wait for Liu to redo the
> patch. BTW this should happen only when SPARSE_VMEMMAP is not
> specified. Srikar had reported the issue here
> 
> http://mid.gmane.org/20140219180200.GA29257@linux.vnet.ibm.com
> 
> #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
> #define SECTIONS_WIDTH		SECTIONS_SHIFT
> #else
> #define SECTIONS_WIDTH		0
> #endif
> 

I'm lost.  What patch are you talking about?  The first hunk of what?

I assume we're talking about
mm-numa-bugfix-for-last_cpupid_not_in_page_flags.patch, which I had
queued for 3.14.  I'll put it on hold until there's some clarity here.

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[liu ping fan]

On Fri, Feb 28, 2014 at 7:41 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 26 Feb 2014 13:22:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Andrew Morton <akpm@linux-foundation.org> writes:
>>
>> > On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>> >
>> >> When doing some numa tests on powerpc, I triggered an oops bug. I find
>> >> it is caused by using page->_last_cpupid.  It should be initialized as
>> >> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
>> >> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
>> >> And finally cause an oops bug in task_numa_group(), since the online cpu is
>> >> less than possible cpu.
>> >
>> > I grabbed this.  I added this to the changelog:
>> >
>> > : PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
>> > : support a large physical address region, up to 2^46 but small section size
>> > : (2^24).  So when NR_CPUS grows up, it is easily to cause
>> > : not-in-page-flags.
>> >
>> > to hopefully address Peter's observation.
>> >
>> > How should we proceed with this?  I'm getting the impression that numa
>> > balancing on ppc is a dead duck in 3.14, so perhaps this and
>> >
>> > powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
>> > mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
>> > mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
>> >
>>
>> All these are already in 3.14  ?
>
> Yes.
>
>> > are 3.15-rc1 material?
>> >
>>
>> We should push the first hunk to 3.14. I will wait for Liu to redo the
>> patch. BTW this should happen only when SPARSE_VMEMMAP is not
>> specified. Srikar had reported the issue here
>>
>> http://mid.gmane.org/20140219180200.GA29257@linux.vnet.ibm.com
>>
>> #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
>> #define SECTIONS_WIDTH                SECTIONS_SHIFT
>> #else
>> #define SECTIONS_WIDTH                0
>> #endif
>>
>
> I'm lost.  What patch are you talking about?  The first hunk of what?
>
I think Aneesh was talking about the chunk of patch, which modified
the file "page-flags-layout.h".
I tried to collapse and simplify the logic, but it will incur that
LAST_CPUPID_WIDTH depends on CONFIG_NUMA_BALANCING.
It is an error since we need LAST_CPUPID_WIDTH even without
CONFIG_NUMA_BALANCING. (Sorry, I compiled and run kernel, but not find
this).

Thanks and best regards,
Fan

> I assume we're talking about
> mm-numa-bugfix-for-last_cpupid_not_in_page_flags.patch, which I had
> queued for 3.14.  I'll put it on hold until there's some clarity here.
>

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Aneesh Kumar K.V]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed, 26 Feb 2014 13:22:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Andrew Morton <akpm@linux-foundation.org> writes:
>> 
>> > On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>> >
>> >> When doing some numa tests on powerpc, I triggered an oops bug. I find
>> >> it is caused by using page->_last_cpupid.  It should be initialized as
>> >> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
>> >> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
>> >> And finally cause an oops bug in task_numa_group(), since the online cpu is
>> >> less than possible cpu.
>> >
>> > I grabbed this.  I added this to the changelog:
>> >
>> > : PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
>> > : support a large physical address region, up to 2^46 but small section size
>> > : (2^24).  So when NR_CPUS grows up, it is easily to cause
>> > : not-in-page-flags.
>> >
>> > to hopefully address Peter's observation.
>> >
>> > How should we proceed with this?  I'm getting the impression that numa
>> > balancing on ppc is a dead duck in 3.14, so perhaps this and 
>> >
>> > powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
>> > mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
>> > mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
>> >
>> 
>> All these are already in 3.14  ?
>
> Yes.
>
>> > are 3.15-rc1 material?
>> >
>> 
>> We should push the first hunk to 3.14. I will wait for Liu to redo the
>> patch. BTW this should happen only when SPARSE_VMEMMAP is not
>> specified. Srikar had reported the issue here
>> 
>> http://mid.gmane.org/20140219180200.GA29257@linux.vnet.ibm.com
>> 
>> #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
>> #define SECTIONS_WIDTH		SECTIONS_SHIFT
>> #else
>> #define SECTIONS_WIDTH		0
>> #endif
>> 
>
> I'm lost.  What patch are you talking about?  The first hunk of what?

The patch in this thread.

>
> I assume we're talking about
> mm-numa-bugfix-for-last_cpupid_not_in_page_flags.patch, which I had
> queued for 3.14.  I'll put it on hold until there's some clarity here.

We don't need the complete patch, it is just the first hunk that we need
to fix the crash ie. we only need

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a7b4e31..ddc66df4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -727,7 +727,7 @@ static inline int page_cpupid_last(struct page *page)
 }
 static inline void page_cpupid_reset_last(struct page *page)
 {
-	page->_last_cpupid = -1;
+	page->_last_cpupid = -1 & LAST_CPUPID_MASK;
 }
 #else
 static inline int page_cpupid_last(struct page *page)

Also the issue will only happen when SPARSE_VMEMMAP is not enabled. I
will send a proper patch with updated changelog. I was hoping Liu will
get to that quickly


-aneesh

[PATCH V2] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Aneesh Kumar K.V]

From: Liu Ping Fan <qemulist@gmail.com>

When doing some numa tests on powerpc, I triggered an oops bug. I find
it is caused by using page->_last_cpupid.  It should be initialized as
"-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
And finally cause an oops bug in task_numa_group(), since the online cpu is
less than possible cpu. This happen with CONFIG_SPARSE_VMEMMAP disabled

Call trace:
[   55.978091] SMP NR_CPUS=64 NUMA PowerNV
[   55.978118] Modules linked in:
[   55.978145] CPU: 24 PID: 804 Comm: systemd-udevd Not tainted3.13.0-rc1+ #32
[   55.978183] task: c000001e2746aa80 ti: c000001e32c50000 task.ti:c000001e32c50000
[   55.978219] NIP: c0000000000f5ad0 LR: c0000000000f5ac8 CTR:c000000000913cf0
[   55.978256] REGS: c000001e32c53510 TRAP: 0300   Not tainted(3.13.0-rc1+)
[   55.978286] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR:28024424  XER: 20000000
[   55.978380] CFAR: c000000000009324 DAR: 7265717569726857 DSISR:40000000 SOFTE: 1
GPR00: c0000000000f5ac8 c000001e32c53790 c000000001f343380000000000000021
GPR04: 0000000000000000 0000000000000031 c000000001f743380000ffffffffffff
GPR08: 0000000000000001 7265717569726573 00000000000000000000000000000000
GPR12: 0000000028024422 c00000000ffdd800 00000000296b2e640000000000000020
GPR16: 0000000000000002 0000000000000003 c000001e2f8e4658c000001e25c1c1d8
GPR20: c000001e2f8e4000 c000000001f7a858 00000000000006580000000040000392
GPR24: 00000000000000a8 c000001e33c1a400 00000000000001d8c000001e25c1c000
GPR28: c000001e33c37ff0 0007837840000392 000000000000003fc000001e32c53790
[   55.978903] NIP [c0000000000f5ad0] .task_numa_fault+0x1470/0x2370
[   55.978934] LR [c0000000000f5ac8] .task_numa_fault+0x1468/0x2370
[   55.978964] Call Trace:
[   55.978978] [c000001e32c53790] [c0000000000f5ac8].task_numa_fault+0x1468/0x2370 (unreliable)
[   55.979036] [c000001e32c539e0] [c00000000020a820].do_numa_page+0x480/0x4a0
[   55.979072] [c000001e32c53b10] [c00000000020bfec].handle_mm_fault+0x4ec/0xc90
[   55.979123] [c000001e32c53c00] [c000000000e88c98].do_page_fault+0x3a8/0x890
[   55.979161] [c000001e32c53e30] [c000000000009568]handle_page_fault+0x10/0x30
[   55.979197] Instruction dump:
[   55.979216] 3c82fefb 3884b138 48d9cff1 60000000 48000574 3c62fefb3863af78 3c82fefb
[   55.979277] 3884b138 48d9cfd5 60000000 e93f0100 <812902e4> 7d2907b45529063e 7d2a07b4
[   55.979354] ---[ end trace 15f2510da5ae07cf ]---

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 include/linux/mm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index f28f46eade6a..1a0ea24ff972 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -766,7 +766,7 @@ static inline int page_cpupid_last(struct page *page)
 }
 static inline void page_cpupid_reset_last(struct page *page)
 {
-	page->_last_cpupid = -1;
+	page->_last_cpupid = -1 & LAST_CPUPID_MASK;
 }
 #else
 static inline int page_cpupid_last(struct page *page)
-- 
1.8.3.2

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[liu ping fan]

On Fri, Feb 28, 2014 at 12:47 PM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> Andrew Morton <akpm@linux-foundation.org> writes:
>
>> On Wed, 26 Feb 2014 13:22:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>
>>> Andrew Morton <akpm@linux-foundation.org> writes:
>>>
>>> > On Wed,  5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>>> >
>>> >> When doing some numa tests on powerpc, I triggered an oops bug. I find
>>> >> it is caused by using page->_last_cpupid.  It should be initialized as
>>> >> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
>>> >> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
>>> >> And finally cause an oops bug in task_numa_group(), since the online cpu is
>>> >> less than possible cpu.
>>> >
>>> > I grabbed this.  I added this to the changelog:
>>> >
>>> > : PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
>>> > : support a large physical address region, up to 2^46 but small section size
>>> > : (2^24).  So when NR_CPUS grows up, it is easily to cause
>>> > : not-in-page-flags.
>>> >
>>> > to hopefully address Peter's observation.
>>> >
>>> > How should we proceed with this?  I'm getting the impression that numa
>>> > balancing on ppc is a dead duck in 3.14, so perhaps this and
>>> >
>>> > powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
>>> > mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
>>> > mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
>>> >
>>>
>>> All these are already in 3.14  ?
>>
>> Yes.
>>
>>> > are 3.15-rc1 material?
>>> >
>>>
>>> We should push the first hunk to 3.14. I will wait for Liu to redo the
>>> patch. BTW this should happen only when SPARSE_VMEMMAP is not
>>> specified. Srikar had reported the issue here
>>>
>>> http://mid.gmane.org/20140219180200.GA29257@linux.vnet.ibm.com
>>>
>>> #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
>>> #define SECTIONS_WIDTH               SECTIONS_SHIFT
>>> #else
>>> #define SECTIONS_WIDTH               0
>>> #endif
>>>
>>
>> I'm lost.  What patch are you talking about?  The first hunk of what?
>
> The patch in this thread.
>
>>
>> I assume we're talking about
>> mm-numa-bugfix-for-last_cpupid_not_in_page_flags.patch, which I had
>> queued for 3.14.  I'll put it on hold until there's some clarity here.
>
> We don't need the complete patch, it is just the first hunk that we need
> to fix the crash ie. we only need
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a7b4e31..ddc66df4 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -727,7 +727,7 @@ static inline int page_cpupid_last(struct page *page)
>  }
>  static inline void page_cpupid_reset_last(struct page *page)
>  {
> -       page->_last_cpupid = -1;
> +       page->_last_cpupid = -1 & LAST_CPUPID_MASK;
>  }
>  #else
>  static inline int page_cpupid_last(struct page *page)
>
> Also the issue will only happen when SPARSE_VMEMMAP is not enabled. I
> will send a proper patch with updated changelog. I was hoping Liu will
> get to that quickly
>
Thanks for sending V2.  Since the ppc machine env is changed by
others, I am blocking on setting up the env for re-test this patch.
And not send out it quickly.

Best regards,
Fan
>
> -aneesh
>

Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS

[Aneesh Kumar K.V]

liu ping fan <qemulist@gmail.com> writes:

> On Fri, Feb 28, 2014 at 12:47 PM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> Andrew Morton <akpm@linux-foundation.org> writes:
>>
> Thanks for sending V2.  Since the ppc machine env is changed by
> others, I am blocking on setting up the env for re-test this patch.
> And not send out it quickly.

I sent an updated v3 also taking care of xchg

http://article.gmane.org/gmane.linux.kernel/1657613

-aneesh