Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
introduced a memory leak.
After this commit, the 'return' statement in pmd_free is executed in all
cases. Even for pmd that are not attached to the pgd.
So 'free_pages' can never be called anymore, leading to a memory leak.
Signed-off-by: Christophe JAILLET <[email protected]>
---
This patch is *untested* as I don't have the hardware to test it.
This is just a guess based on the indentation, the comment in the code
and the commit log.
---
arch/parisc/include/asm/pgalloc.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
index 3a08eae..f66d3738 100644
--- a/arch/parisc/include/asm/pgalloc.h
+++ b/arch/parisc/include/asm/pgalloc.h
@@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
{
- if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
+ if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
/*
* This is the permanent pmd attached to the pgd;
* cannot free it.
@@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
*/
mm_inc_nr_pmds(mm);
return;
+ }
free_pages((unsigned long)pmd, PMD_ORDER);
}
--
2.1.4
Christophe JAILLET wrote:
> Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
> introduced a memory leak.
>
> After this commit, the 'return' statement in pmd_free is executed in all
> cases. Even for pmd that are not attached to the pgd.
> So 'free_pages' can never be called anymore, leading to a memory leak.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Acked-by: Kirill A. Shutemov <[email protected]>
> ---
> This patch is *untested* as I don't have the hardware to test it.
>
> This is just a guess based on the indentation, the comment in the code
> and the commit log.
> ---
> arch/parisc/include/asm/pgalloc.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
> index 3a08eae..f66d3738 100644
> --- a/arch/parisc/include/asm/pgalloc.h
> +++ b/arch/parisc/include/asm/pgalloc.h
> @@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
>
> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> {
> - if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
> + if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
> /*
> * This is the permanent pmd attached to the pgd;
> * cannot free it.
> @@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> */
> mm_inc_nr_pmds(mm);
> return;
> + }
> free_pages((unsigned long)pmd, PMD_ORDER);
> }
>
> --
> 2.1.4
>
On Mon, 13 Jul 2015, Christophe JAILLET wrote:
> Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
> introduced a memory leak.
>
> After this commit, the 'return' statement in pmd_free is executed in all
> cases. Even for pmd that are not attached to the pgd.
> So 'free_pages' can never be called anymore, leading to a memory leak.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Acked-by: Mikulas Patocka <[email protected]>
also add this, so that it is backported to 4.0 and 4.2:
Cc: [email protected] # 4.0+
Fixes: 0e0da48dee8d
> ---
> This patch is *untested* as I don't have the hardware to test it.
>
> This is just a guess based on the indentation, the comment in the code
> and the commit log.
> ---
> arch/parisc/include/asm/pgalloc.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
> index 3a08eae..f66d3738 100644
> --- a/arch/parisc/include/asm/pgalloc.h
> +++ b/arch/parisc/include/asm/pgalloc.h
> @@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
>
> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> {
> - if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
> + if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
> /*
> * This is the permanent pmd attached to the pgd;
> * cannot free it.
> @@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> */
> mm_inc_nr_pmds(mm);
> return;
> + }
> free_pages((unsigned long)pmd, PMD_ORDER);
> }
>
> --
> 2.1.4
>
Hi Christophe,
On 13.07.2015 11:32, Christophe JAILLET wrote:
> Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
> introduced a memory leak.
>
> After this commit, the 'return' statement in pmd_free is executed in all
> cases. Even for pmd that are not attached to the pgd.
> So 'free_pages' can never be called anymore, leading to a memory leak.
That's really great!!! Thanks for spotting this!
I assume this fixes the leak which killed our debian buildds with OOM
after an uptime of 1-4 days and which only happened since kernel 4.0.
Meelis Roos reported the issue already in this thread:
http://marc.info/?l=linux-parisc&m=142999113232154&w=2
> Signed-off-by: Christophe JAILLET <[email protected]>
Acked-by: Helge Deller <[email protected]>
Will this patch be pushed via linux-mm or another tree, if
not I can take it via the parisc tree?
Helge
> ---
> This patch is *untested* as I don't have the hardware to test it.
>
> This is just a guess based on the indentation, the comment in the code
> and the commit log.
> ---
> arch/parisc/include/asm/pgalloc.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
> index 3a08eae..f66d3738 100644
> --- a/arch/parisc/include/asm/pgalloc.h
> +++ b/arch/parisc/include/asm/pgalloc.h
> @@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
>
> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> {
> - if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
> + if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
> /*
> * This is the permanent pmd attached to the pgd;
> * cannot free it.
> @@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> */
> mm_inc_nr_pmds(mm);
> return;
> + }
> free_pages((unsigned long)pmd, PMD_ORDER);
> }
>
>
I assume you found this with Coccinelle? Smatch can also find this bug
but you would need a cross compile environment set up.
regards,
dan carpenter
Sadly, it appears we are not out of the woods yet:
mx3210 login: Backtrace:
[<000000004021c904>] free_hot_cold_page+0x234/0x250
[<000000004021c974>] free_hot_cold_page_list+0x54/0x90
[<0000000040224f28>] release_pages+0x308/0x3f0
[<0000000040226ae8>] __pagevec_release+0x58/0x78
[<00000000402287f4>] invalidate_mapping_pages+0x244/0x258
[<000000004029c284>] inode_lru_isolate+0x1e4/0x2b0
[<0000000040240f34>] __list_lru_walk_one.isra.4+0xc4/0x1c0
[<00000000402411bc>] list_lru_walk_one+0x64/0x80
[<000000004029cc2c>] prune_icache_sb+0x54/0x88
[<000000004027c564>] super_cache_scan+0x2a4/0x308
[<0000000040228d6c>] shrink_slab+0x34c/0x408
[<000000004022d23c>] shrink_zone+0x2bc/0x2c8
[<000000004022e0d4>] kswapd+0x6a4/0xb70
[<00000000401a5e14>] kthread+0x144/0x178
[<0000000040145020>] end_fault_vector+0x20/0x28
[<0000000014281000>] 0x14281000
Kernel Fault: Code=15 regs=000000007c749020 (Addr=0000000000001841)
CPU: 2 PID: 286 Comm: kswapd2 Not tainted 4.0.8+ #1
task: 000000007f75f528 ti: 000000007c748000 task.ti: 000000007c748000
YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
PSW: 00001000000001101111111100001110 Not tainted
r00-03 000000ff0806ff0e 0000000000000000 0000000040766a40 000000007c748f00
r04-07 00000000406eac70 00000000421d1540 0000000000000040 0000000040766940
r08-11 ffffffffffffff80 0000000000000013 0000000000200200 0000000000100100
r12-15 0000000000000001 ffffffffffffffff 00000000407fd980 00000000427ea580
r16-19 0000000000000013 00000000427ea5b0 0000000000000002 00000000427ea5a0
r20-23 000000007c748ff0 0000000040766a40 0000000000000020 0000000040766980
r24-27 00000000421d1560 0000000000001839 0000000000000000 00000000406eac70
r28-31 0000000000000000 0000000000000000 000000007c749020 0000000000000040
sr00-03 0000000000b45800 0000000001060800 0000000000000000 0000000000b45800
sr04-07 0000000000000000 0000000000000000 0000000000000000 0000000000000000
IASQ: 0000000000000000 0000000000000000 IAOQ: 000000004021a11c 000000004021a120
IIR: 0f3812d0 ISR: 0000000000000000 IOR: 0000000000001841
CPU: 2 CR30: 000000007c748000 CR31: 572c033000297031
ORIG_R28: 000000007c749210
IAOQ[0]: free_pcppages_bulk+0x34c/0x470
IAOQ[1]: free_pcppages_bulk+0x350/0x470
RP(r2): 0x40766a40
Backtrace:
[<000000004021c904>] free_hot_cold_page+0x234/0x250
[<000000004021c974>] free_hot_cold_page_list+0x54/0x90
[<0000000040224f28>] release_pages+0x308/0x3f0
[<0000000040226ae8>] __pagevec_release+0x58/0x78
[<00000000402287f4>] invalidate_mapping_pages+0x244/0x258
[<000000004029c284>] inode_lru_isolate+0x1e4/0x2b0
[<0000000040240f34>] __list_lru_walk_one.isra.4+0xc4/0x1c0
[<00000000402411bc>] list_lru_walk_one+0x64/0x80
[<000000004029cc2c>] prune_icache_sb+0x54/0x88
[<000000004027c564>] super_cache_scan+0x2a4/0x308
[<0000000040228d6c>] shrink_slab+0x34c/0x408
[<000000004022d23c>] shrink_zone+0x2bc/0x2c8
[<000000004022e0d4>] kswapd+0x6a4/0xb70
[<00000000401a5e14>] kthread+0x144/0x178
[<0000000040145020>] end_fault_vector+0x20/0x28
[<0000000014281000>] 0x14281000
Kernel panic - not syncing: Kernel Fault
---[ end Kernel panic - not syncing: Kernel Fault
This is with:
Linux version 4.0.8+ (dave@mx3210) (gcc version 4.9.3 (GCC) ) #1 SMP Mon Jul 13 18:47:55 EDT 2015
Dave
On 2015-07-13, at 2:54 PM, Helge Deller wrote:
> COOL !!!
> I think this is our kernel 4.0 OOM bugfix !!
>
> Helge
>
>
> -------- Forwarded Message --------
> Subject: Re: [PATCH] parisc: mm: Fix a memory leak related to pmd not attached to the pgd
> Date: Mon, 13 Jul 2015 20:52:37 +0200
> From: Helge Deller <[email protected]>
> To: Christophe JAILLET <[email protected]>, [email protected], [email protected], [email protected]
> CC: [email protected], [email protected], [email protected], John David Anglin <[email protected]>, Meelis Roos <[email protected]>
>
> Hi Christophe,
>
> On 13.07.2015 11:32, Christophe JAILLET wrote:
>> Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
>> introduced a memory leak.
>>
>> After this commit, the 'return' statement in pmd_free is executed in all
>> cases. Even for pmd that are not attached to the pgd.
>> So 'free_pages' can never be called anymore, leading to a memory leak.
>
> That's really great!!! Thanks for spotting this!
>
> I assume this fixes the leak which killed our debian buildds with OOM
> after an uptime of 1-4 days and which only happened since kernel 4.0.
> Meelis Roos reported the issue already in this thread:
> http://marc.info/?l=linux-parisc&m=142999113232154&w=2
>
>> Signed-off-by: Christophe JAILLET <[email protected]>
>
> Acked-by: Helge Deller <[email protected]>
>
>
> Will this patch be pushed via linux-mm or another tree, if
> not I can take it via the parisc tree?
>
>
> Helge
>
>
>> ---
>> This patch is *untested* as I don't have the hardware to test it.
>>
>> This is just a guess based on the indentation, the comment in the code
>> and the commit log.
>> ---
>> arch/parisc/include/asm/pgalloc.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
>> index 3a08eae..f66d3738 100644
>> --- a/arch/parisc/include/asm/pgalloc.h
>> +++ b/arch/parisc/include/asm/pgalloc.h
>> @@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
>>
>> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
>> {
>> - if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
>> + if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
>> /*
>> * This is the permanent pmd attached to the pgd;
>> * cannot free it.
>> @@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
>> */
>> mm_inc_nr_pmds(mm);
>> return;
>> + }
>> free_pages((unsigned long)pmd, PMD_ORDER);
>> }
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
--
John David Anglin [email protected]
Faulting instruction is:
4021a11c: 0f 38 12 d0 std r24,8(r25)
Register r25 is misaligned by 1.
Dave
On 2015-07-14, at 7:52 AM, John David Anglin wrote:
> Sadly, it appears we are not out of the woods yet:
>
> mx3210 login: Backtrace:
> [<000000004021c904>] free_hot_cold_page+0x234/0x250
> [<000000004021c974>] free_hot_cold_page_list+0x54/0x90
> [<0000000040224f28>] release_pages+0x308/0x3f0
> [<0000000040226ae8>] __pagevec_release+0x58/0x78
> [<00000000402287f4>] invalidate_mapping_pages+0x244/0x258
> [<000000004029c284>] inode_lru_isolate+0x1e4/0x2b0
> [<0000000040240f34>] __list_lru_walk_one.isra.4+0xc4/0x1c0
> [<00000000402411bc>] list_lru_walk_one+0x64/0x80
> [<000000004029cc2c>] prune_icache_sb+0x54/0x88
> [<000000004027c564>] super_cache_scan+0x2a4/0x308
> [<0000000040228d6c>] shrink_slab+0x34c/0x408
> [<000000004022d23c>] shrink_zone+0x2bc/0x2c8
> [<000000004022e0d4>] kswapd+0x6a4/0xb70
> [<00000000401a5e14>] kthread+0x144/0x178
> [<0000000040145020>] end_fault_vector+0x20/0x28
> [<0000000014281000>] 0x14281000
>
>
> Kernel Fault: Code=15 regs=000000007c749020 (Addr=0000000000001841)
> CPU: 2 PID: 286 Comm: kswapd2 Not tainted 4.0.8+ #1
> task: 000000007f75f528 ti: 000000007c748000 task.ti: 000000007c748000
>
> YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> PSW: 00001000000001101111111100001110 Not tainted
> r00-03 000000ff0806ff0e 0000000000000000 0000000040766a40 000000007c748f00
> r04-07 00000000406eac70 00000000421d1540 0000000000000040 0000000040766940
> r08-11 ffffffffffffff80 0000000000000013 0000000000200200 0000000000100100
> r12-15 0000000000000001 ffffffffffffffff 00000000407fd980 00000000427ea580
> r16-19 0000000000000013 00000000427ea5b0 0000000000000002 00000000427ea5a0
> r20-23 000000007c748ff0 0000000040766a40 0000000000000020 0000000040766980
> r24-27 00000000421d1560 0000000000001839 0000000000000000 00000000406eac70
> r28-31 0000000000000000 0000000000000000 000000007c749020 0000000000000040
> sr00-03 0000000000b45800 0000000001060800 0000000000000000 0000000000b45800
> sr04-07 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>
> IASQ: 0000000000000000 0000000000000000 IAOQ: 000000004021a11c 000000004021a120
> IIR: 0f3812d0 ISR: 0000000000000000 IOR: 0000000000001841
> CPU: 2 CR30: 000000007c748000 CR31: 572c033000297031
> ORIG_R28: 000000007c749210
> IAOQ[0]: free_pcppages_bulk+0x34c/0x470
> IAOQ[1]: free_pcppages_bulk+0x350/0x470
> RP(r2): 0x40766a40
> Backtrace:
> [<000000004021c904>] free_hot_cold_page+0x234/0x250
> [<000000004021c974>] free_hot_cold_page_list+0x54/0x90
> [<0000000040224f28>] release_pages+0x308/0x3f0
> [<0000000040226ae8>] __pagevec_release+0x58/0x78
> [<00000000402287f4>] invalidate_mapping_pages+0x244/0x258
> [<000000004029c284>] inode_lru_isolate+0x1e4/0x2b0
> [<0000000040240f34>] __list_lru_walk_one.isra.4+0xc4/0x1c0
> [<00000000402411bc>] list_lru_walk_one+0x64/0x80
> [<000000004029cc2c>] prune_icache_sb+0x54/0x88
> [<000000004027c564>] super_cache_scan+0x2a4/0x308
> [<0000000040228d6c>] shrink_slab+0x34c/0x408
> [<000000004022d23c>] shrink_zone+0x2bc/0x2c8
> [<000000004022e0d4>] kswapd+0x6a4/0xb70
> [<00000000401a5e14>] kthread+0x144/0x178
> [<0000000040145020>] end_fault_vector+0x20/0x28
> [<0000000014281000>] 0x14281000
>
> Kernel panic - not syncing: Kernel Fault
> ---[ end Kernel panic - not syncing: Kernel Fault
>
> This is with:
> Linux version 4.0.8+ (dave@mx3210) (gcc version 4.9.3 (GCC) ) #1 SMP Mon Jul 13 18:47:55 EDT 2015
>
> Dave
>
> On 2015-07-13, at 2:54 PM, Helge Deller wrote:
>
>> COOL !!!
>> I think this is our kernel 4.0 OOM bugfix !!
>>
>> Helge
>>
>>
>> -------- Forwarded Message --------
>> Subject: Re: [PATCH] parisc: mm: Fix a memory leak related to pmd not attached to the pgd
>> Date: Mon, 13 Jul 2015 20:52:37 +0200
>> From: Helge Deller <[email protected]>
>> To: Christophe JAILLET <[email protected]>, [email protected], [email protected], [email protected]
>> CC: [email protected], [email protected], [email protected], John David Anglin <[email protected]>, Meelis Roos <[email protected]>
>>
>> Hi Christophe,
>>
>> On 13.07.2015 11:32, Christophe JAILLET wrote:
>>> Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
>>> introduced a memory leak.
>>>
>>> After this commit, the 'return' statement in pmd_free is executed in all
>>> cases. Even for pmd that are not attached to the pgd.
>>> So 'free_pages' can never be called anymore, leading to a memory leak.
>>
>> That's really great!!! Thanks for spotting this!
>>
>> I assume this fixes the leak which killed our debian buildds with OOM
>> after an uptime of 1-4 days and which only happened since kernel 4.0.
>> Meelis Roos reported the issue already in this thread:
>> http://marc.info/?l=linux-parisc&m=142999113232154&w=2
>>
>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>
>> Acked-by: Helge Deller <[email protected]>
>>
>>
>> Will this patch be pushed via linux-mm or another tree, if
>> not I can take it via the parisc tree?
>>
>>
>> Helge
>>
>>
>>> ---
>>> This patch is *untested* as I don't have the hardware to test it.
>>>
>>> This is just a guess based on the indentation, the comment in the code
>>> and the commit log.
>>> ---
>>> arch/parisc/include/asm/pgalloc.h | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/parisc/include/asm/pgalloc.h b/arch/parisc/include/asm/pgalloc.h
>>> index 3a08eae..f66d3738 100644
>>> --- a/arch/parisc/include/asm/pgalloc.h
>>> +++ b/arch/parisc/include/asm/pgalloc.h
>>> @@ -72,7 +72,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
>>>
>>> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
>>> {
>>> - if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
>>> + if (pmd_flag(*pmd) & PxD_FLAG_ATTACHED) {
>>> /*
>>> * This is the permanent pmd attached to the pgd;
>>> * cannot free it.
>>> @@ -81,6 +81,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
>>> */
>>> mm_inc_nr_pmds(mm);
>>> return;
>>> + }
>>> free_pages((unsigned long)pmd, PMD_ORDER);
>>> }
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>
> --
> John David Anglin [email protected]
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
John David Anglin [email protected]
> Hi Christophe,
>
> On 13.07.2015 11:32, Christophe JAILLET wrote:
> > Commit 0e0da48dee8d ("parisc: mm: don't count preallocated pmds")
> > introduced a memory leak.
> >
> > After this commit, the 'return' statement in pmd_free is executed in all
> > cases. Even for pmd that are not attached to the pgd.
> > So 'free_pages' can never be called anymore, leading to a memory leak.
>
> That's really great!!! Thanks for spotting this!
>
> I assume this fixes the leak which killed our debian buildds with OOM
> after an uptime of 1-4 days and which only happened since kernel 4.0.
> Meelis Roos reported the issue already in this thread:
> http://marc.info/?l=linux-parisc&m=142999113232154&w=2
Yes, the patch that is merged in 4.2-rc3 fixed my RP3410 with 1G RAM.
--
Meelis Roos ([email protected])