[PATCH] x86: revert "x86: Fix S4 regression"

[PATCH] x86: revert "x86: Fix S4 regression"

[Cong Wang]

From: Cong Wang <xiyou.wangcong@gmail.com>

This reverts the following commit:

	commit 8548c84da2f47e71bbbe300f55edb768492575f7
	Author: Takashi Iwai <tiwai@suse.de>
	Date:   Sun Oct 23 23:19:12 2011 +0200

	    x86: Fix S4 regression
	    
	    Commit 4b239f458 ("x86-64, mm: Put early page table high") causes a S4
	    regression since 2.6.39, namely the machine reboots occasionally at S4
	    resume.  It doesn't happen always, overall rate is about 1/20.  But,
	    like other bugs, once when this happens, it continues to happen.
	    
	    This patch fixes the problem by essentially reverting the memory
	    assignment in the older way.

According to the previous discussion:
http://marc.info/?l=linux-kernel&m=133161674120253&w=2
it seems that so far the best solution is just reverting it.

Takashi, could you help to test if the S4 regression is still
there after this patch?

Reported-by: CAI Qian <caiqian@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Yinghai Lu <yinghai.lu@oracle.com>
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

---
 arch/x86/mm/init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index bc4e9d8..7ab7975 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -74,8 +74,9 @@ static void __init find_early_table_space(struct map_range *mr, unsigned long en
 #ifdef CONFIG_X86_32
 	/* for fixmap */
 	tables += roundup(__end_of_fixed_addresses * sizeof(pte_t), PAGE_SIZE);
-#endif
+
 	good_end = max_pfn_mapped << PAGE_SHIFT;
+#endif
 
 	base = memblock_find_in_range(start, good_end, tables, PAGE_SIZE);
 	if (!base)
-- 
1.7.7.6

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Cong Wang]

On Tue, Jun 12, 2012 at 1:21 PM, Cong Wang <amwang@redhat.com> wrote:
>
> Takashi, could you help to test if the S4 regression is still
> there after this patch?

Hello,

Any comments?

I have tested this revert on my laptop, with running simple
"suspend and resume" test for several times, I didn't see any
problem.

Thanks!

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Takashi Iwai]

At Fri, 15 Jun 2012 19:11:12 +0800,
Cong Wang wrote:
> 
> On Tue, Jun 12, 2012 at 1:21 PM, Cong Wang <amwang@redhat.com> wrote:
> >
> > Takashi, could you help to test if the S4 regression is still
> > there after this patch?
> 
> Hello,
> 
> Any comments?
> 
> I have tested this revert on my laptop, with running simple
> "suspend and resume" test for several times, I didn't see any
> problem.

Sorry for the late response.

The problem happens only on certain laptops.  I've tested the recent
IvyBridge laptops and these were OK with the reverted patch.  But a
SandyBridge laptop, the one I tested in the last year, still hits the
S4 problem with the reverted kernel.

But, the recent kernels seem to have other S4 problems on this
machine, so it's not 100% clear whether it's triggered by that.
At least, it jumpts to the boot at S4 resume more frequently when the
patch is reverted.

So, I need to start from 2.6.32 again to see what regressions have
been introduced.  It'll take time, since I'd have to S4 20 cycles
for reproducing the bug.


Takashi

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Rafael J. Wysocki]

On Friday, June 15, 2012, Takashi Iwai wrote:
> At Fri, 15 Jun 2012 19:11:12 +0800,
> Cong Wang wrote:
> > 
> > On Tue, Jun 12, 2012 at 1:21 PM, Cong Wang <amwang@redhat.com> wrote:
> > >
> > > Takashi, could you help to test if the S4 regression is still
> > > there after this patch?
> > 
> > Hello,
> > 
> > Any comments?
> > 
> > I have tested this revert on my laptop, with running simple
> > "suspend and resume" test for several times, I didn't see any
> > problem.
> 
> Sorry for the late response.
> 
> The problem happens only on certain laptops.  I've tested the recent
> IvyBridge laptops and these were OK with the reverted patch.  But a
> SandyBridge laptop, the one I tested in the last year, still hits the
> S4 problem with the reverted kernel.
> 
> But, the recent kernels seem to have other S4 problems on this
> machine, so it's not 100% clear whether it's triggered by that.
> At least, it jumpts to the boot at S4 resume more frequently when the
> patch is reverted.
> 
> So, I need to start from 2.6.32 again to see what regressions have
> been introduced.  It'll take time, since I'd have to S4 20 cycles
> for reproducing the bug.

Thanks for doing this, please let me know if there's any way I can help.

Rafael

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Cong Wang]

On Fri, Jun 15, 2012 at 8:15 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Fri, 15 Jun 2012 19:11:12 +0800,
> Cong Wang wrote:
>>
>> On Tue, Jun 12, 2012 at 1:21 PM, Cong Wang <amwang@redhat.com> wrote:
>> >
>> > Takashi, could you help to test if the S4 regression is still
>> > there after this patch?
>>
>> Hello,
>>
>> Any comments?
>>
>> I have tested this revert on my laptop, with running simple
>> "suspend and resume" test for several times, I didn't see any
>> problem.
>
> Sorry for the late response.
>
> The problem happens only on certain laptops.  I've tested the recent
> IvyBridge laptops and these were OK with the reverted patch.  But a
> SandyBridge laptop, the one I tested in the last year, still hits the
> S4 problem with the reverted kernel.

Yeah, but the crashkernel=512M regression seems to happen
on more machines. :)

>
> But, the recent kernels seem to have other S4 problems on this
> machine, so it's not 100% clear whether it's triggered by that.
> At least, it jumpts to the boot at S4 resume more frequently when the
> patch is reverted.
>
> So, I need to start from 2.6.32 again to see what regressions have
> been introduced.  It'll take time, since I'd have to S4 20 cycles
> for reproducing the bug.
>

Ok, thanks for your update!

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Takao Indoh]

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

Hi Cong,

When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
kernel's panic in find_early_table_space().

init_memory_mapping: [mem 0x00000000-0x36ffafff]
Kernel panic - not syncing: Cannot find space for the kernel page tables
Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
Call Trace:
 [<ffffffff8158549b>] panic+0xb8/0x1c8
 [<ffffffff8158565d>] ? printk+0x48/0x4a
 [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
 [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
 [<ffffffff8158565d>] ? printk+0x48/0x4a
 [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
 [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
 [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4

In find_early_table_space(), a kernel tries to find free area below 512M
for pgtable using memblock_find_in_range, but it fails because kdump
kernel does not have enough free space below 512M due to the memmap
restriction. This is the memmap option specified against kdump kernel
when crashkernel=128M.

memmap=560K@64K memmap=130492K@770608K

Only 560KB area is available and it is not sufficient for pgtable (it
seems that about 1.8MB area is needed for pgtable). This problem is
fixed by your revert patch. I hope this patch gets merged.

Thanks,
Takao Indoh

(2012/06/12 14:21), Cong Wang wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> This reverts the following commit:
> 
> 	commit 8548c84da2f47e71bbbe300f55edb768492575f7
> 	Author: Takashi Iwai <tiwai@suse.de>
> 	Date:   Sun Oct 23 23:19:12 2011 +0200
> 
> 	    x86: Fix S4 regression
> 	
> 	    Commit 4b239f458 ("x86-64, mm: Put early page table high") causes a S4
> 	    regression since 2.6.39, namely the machine reboots occasionally at S4
> 	    resume.  It doesn't happen always, overall rate is about 1/20.  But,
> 	    like other bugs, once when this happens, it continues to happen.
> 	
> 	    This patch fixes the problem by essentially reverting the memory
> 	    assignment in the older way.
> 
> According to the previous discussion:
> http://marc.info/?l=linux-kernel&m=133161674120253&w=2
> it seems that so far the best solution is just reverting it.
> 
> Takashi, could you help to test if the S4 regression is still
> there after this patch?
> 
> Reported-by: CAI Qian <caiqian@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Yinghai Lu <yinghai.lu@oracle.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> 
> ---
>   arch/x86/mm/init.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index bc4e9d8..7ab7975 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -74,8 +74,9 @@ static void __init find_early_table_space(struct map_range *mr, unsigned long en
>   #ifdef CONFIG_X86_32
>   	/* for fixmap */
>   	tables += roundup(__end_of_fixed_addresses * sizeof(pte_t), PAGE_SIZE);
> -#endif
> +
>   	good_end = max_pfn_mapped << PAGE_SHIFT;
> +#endif
>   
>   	base = memblock_find_in_range(start, good_end, tables, PAGE_SIZE);
>   	if (!base)
> 

[-- Attachment #2: log.txt --]
[-- Type: text/plain, Size: 4911 bytes --]

[    0.000000] Linux version 3.5.0-rc6 (root@localhost) (gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC) ) #17 SMP Thu Jul 12 13:49:46 JST 2012
[    0.000000] Command line: ro root=UUID=1893a13e-19af-439b-9d39-0a42260f3eaa rd_NO_LUKS rd_NO_MD KEYBOARDTYPE=pc KEYTABLE=jp106 LANG=ja_JP.UTF-8 rd_NO_LVM rd_NO_DM loglevel=7 earlyprintk=serial,ttyS0,19200n8 irqpoll nr_cpus=1 reset_devices cgroup_disable=memory mce=off  memmap=exactmap memmap=560K@64K memmap=130492K@770608K elfcorehdr=901100K memmap=64K$0K memmap=16K$624K memmap=112K$912K memmap=32832K$3103360K memmap=40K#3136192K memmap=4K#3136232K memmap=9492K$3136236K memmap=262144K$3670016K memmap=1024K$4173824K memmap=4K$4175872K memmap=17408K$4176896K
[    0.000000] e820: BIOS-provided physical RAM map:
[    0.000000] BIOS-e820: [mem 0x0000000000000100-0x000000000009bfff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009c000-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000e4000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x00000000bd69ffff] usable
[    0.000000] BIOS-e820: [mem 0x00000000bd6a0000-0x00000000bf6affff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000bf6b0000-0x00000000bf6b9fff] ACPI data
[    0.000000] BIOS-e820: [mem 0x00000000bf6ba000-0x00000000bf6bafff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000bf6bb000-0x00000000bfffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000e0000000-0x00000000efffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec00000-0x00000000fecfffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ffa00000-0x00000000ffffffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000043fffffff] usable
[    0.000000] bootconsole [earlyser0] enabled
[    0.000000] e820: last_pfn = 0x440000 max_arch_pfn = 0x400000000
[    0.000000] NX (Execute Disable) protection: active
[    0.000000] e820: user-defined physical RAM map:
[    0.000000] user: [mem 0x0000000000000000-0x000000000000ffff] reserved
[    0.000000] user: [mem 0x0000000000010000-0x000000000009bfff] usable
[    0.000000] user: [mem 0x000000000009c000-0x000000000009ffff] reserved
[    0.000000] user: [mem 0x00000000000e4000-0x00000000000fffff] reserved
[    0.000000] user: [mem 0x000000002f08c000-0x0000000036ffafff] usable
[    0.000000] user: [mem 0x00000000bd6a0000-0x00000000bf6affff] reserved
[    0.000000] user: [mem 0x00000000bf6b0000-0x00000000bf6bafff] ACPI data
[    0.000000] user: [mem 0x00000000bf6bb000-0x00000000bfffffff] reserved
[    0.000000] user: [mem 0x00000000e0000000-0x00000000efffffff] reserved
[    0.000000] user: [mem 0x00000000fec00000-0x00000000fecfffff] reserved
[    0.000000] user: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
[    0.000000] user: [mem 0x00000000fef00000-0x00000000ffffffff] reserved
[    0.000000] DMI present.
[    0.000000] No AGP bridge found
[    0.000000] e820: last_pfn = 0x36ffb max_arch_pfn = 0x400000000
[    0.000000] x86 PAT enabled: cpu 0, old 0x7010600070106, new 0x7010600070106
[    0.000000] found SMP MP-table at [mem 0x000f7ef0-0x000f7eff] mapped at [ffff8800000f7ef0]
[    0.000000] Using GB pages for direct mapping
[    0.000000] init_memory_mapping: [mem 0x00000000-0x36ffafff]
[    0.000000] Kernel panic - not syncing: Cannot find space for the kernel page tables
[    0.000000] Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
[    0.000000] Call Trace:
[    0.000000]  [<ffffffff8158549b>] panic+0xb8/0x1c8
[    0.000000]  [<ffffffff8158565d>] ? printk+0x48/0x4a
[    0.000000]  [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
[    0.000000]  [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
[    0.000000]  [<ffffffff8158565d>] ? printk+0x48/0x4a
[    0.000000]  [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
[    0.000000]  [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
[    0.000000]  [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
PANIC: early exception 0d rip 10:ffffffff8158556d error 77b cr2 0
[    0.000000] Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
[    0.000000] Call Trace:
[    0.000000]  [<ffffffff8158556d>] ? panic+0x18a/0x1c8
[    0.000000]  [<ffffffff818a3184>] early_idt_handler+0x64/0x94
[    0.000000]  [<ffffffff8158556d>] ? panic+0x18a/0x1c8
[    0.000000]  [<ffffffff815854d0>] ? panic+0xed/0x1c8
[    0.000000]  [<ffffffff8158565d>] ? printk+0x48/0x4a
[    0.000000]  [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
[    0.000000]  [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
[    0.000000]  [<ffffffff8158565d>] ? printk+0x48/0x4a
[    0.000000]  [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
[    0.000000]  [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
[    0.000000]  [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
[    0.000000] RIP 0x0

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Dave Young]

On 07/17/2012 11:15 AM, Takao Indoh wrote:

> Hi Cong,
> 
> When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
> kernel's panic in find_early_table_space().
> 
> init_memory_mapping: [mem 0x00000000-0x36ffafff]
> Kernel panic - not syncing: Cannot find space for the kernel page tables
> Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
> Call Trace:
>  [<ffffffff8158549b>] panic+0xb8/0x1c8
>  [<ffffffff8158565d>] ? printk+0x48/0x4a
>  [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
>  [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
>  [<ffffffff8158565d>] ? printk+0x48/0x4a
>  [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
>  [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
>  [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
> 
> In find_early_table_space(), a kernel tries to find free area below 512M
> for pgtable using memblock_find_in_range, but it fails because kdump
> kernel does not have enough free space below 512M due to the memmap
> restriction. This is the memmap option specified against kdump kernel
> when crashkernel=128M.
> 
> memmap=560K@64K memmap=130492K@770608K
> 
> Only 560KB area is available and it is not sufficient for pgtable (it
> seems that about 1.8MB area is needed for pgtable). This problem is
> fixed by your revert patch. I hope this patch gets merged.


I can reproduce this issue as well, probably related to some x86 mm init
commits, this alloc failure does not happen with reverting below commits:

bd2753b2dda7bb43c7468826de75f49c6a7e8965
722bc6b16771ed80871e1fd81c86d3627dda2ac8



> 
> Thanks,
> Takao Indoh
> 
> (2012/06/12 14:21), Cong Wang wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>>
>> This reverts the following commit:
>>
>> 	commit 8548c84da2f47e71bbbe300f55edb768492575f7
>> 	Author: Takashi Iwai <tiwai@suse.de>
>> 	Date:   Sun Oct 23 23:19:12 2011 +0200
>>
>> 	    x86: Fix S4 regression
>> 	
>> 	    Commit 4b239f458 ("x86-64, mm: Put early page table high") causes a S4
>> 	    regression since 2.6.39, namely the machine reboots occasionally at S4
>> 	    resume.  It doesn't happen always, overall rate is about 1/20.  But,
>> 	    like other bugs, once when this happens, it continues to happen.
>> 	
>> 	    This patch fixes the problem by essentially reverting the memory
>> 	    assignment in the older way.
>>
>> According to the previous discussion:
>> http://marc.info/?l=linux-kernel&m=133161674120253&w=2
>> it seems that so far the best solution is just reverting it.
>>
>> Takashi, could you help to test if the S4 regression is still
>> there after this patch?
>>
>> Reported-by: CAI Qian <caiqian@redhat.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>> Cc: Yinghai Lu <yinghai.lu@oracle.com>
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>
>> ---
>>   arch/x86/mm/init.c |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>> index bc4e9d8..7ab7975 100644
>> --- a/arch/x86/mm/init.c
>> +++ b/arch/x86/mm/init.c
>> @@ -74,8 +74,9 @@ static void __init find_early_table_space(struct map_range *mr, unsigned long en
>>   #ifdef CONFIG_X86_32
>>   	/* for fixmap */
>>   	tables += roundup(__end_of_fixed_addresses * sizeof(pte_t), PAGE_SIZE);
>> -#endif
>> +
>>   	good_end = max_pfn_mapped << PAGE_SHIFT;
>> +#endif
>>   
>>   	base = memblock_find_in_range(start, good_end, tables, PAGE_SIZE);
>>   	if (!base)
>>



-- 
Thanks
Dave

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Takao Indoh]

(2012/07/23 19:00), Dave Young wrote:
> On 07/17/2012 11:15 AM, Takao Indoh wrote:
> 
>> Hi Cong,
>>
>> When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
>> kernel's panic in find_early_table_space().
>>
>> init_memory_mapping: [mem 0x00000000-0x36ffafff]
>> Kernel panic - not syncing: Cannot find space for the kernel page tables
>> Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
>> Call Trace:
>>   [<ffffffff8158549b>] panic+0xb8/0x1c8
>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
>>   [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
>>   [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
>>   [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
>>   [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
>>   [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
>>
>> In find_early_table_space(), a kernel tries to find free area below 512M
>> for pgtable using memblock_find_in_range, but it fails because kdump
>> kernel does not have enough free space below 512M due to the memmap
>> restriction. This is the memmap option specified against kdump kernel
>> when crashkernel=128M.
>>
>> memmap=560K@64K memmap=130492K@770608K
>>
>> Only 560KB area is available and it is not sufficient for pgtable (it
>> seems that about 1.8MB area is needed for pgtable). This problem is
>> fixed by your revert patch. I hope this patch gets merged.
> 
> 
> I can reproduce this issue as well, probably related to some x86 mm init
> commits, this alloc failure does not happen with reverting below commits:
> 
> bd2753b2dda7bb43c7468826de75f49c6a7e8965
> 722bc6b16771ed80871e1fd81c86d3627dda2ac8
Yeah, my result of bisect is as follows and at first I thought the
commit 722bc6 caused this regression.

722bc6b16771ed80871e1fd81c86d3627dda2ac8 is the first bad commit
commit 722bc6b16771ed80871e1fd81c86d3627dda2ac8
Author: WANG Cong <xiyou.wangcong@gmail.com>
Date:   Mon Mar 5 15:05:13 2012 -0800

IIUC, this commit just fixes a bug of counting pgtable entries. As the
result, another problem came up to the surface. In the case of my
machine(16GB memory), before applying 722bc6, find_early_table_space()
requests about 12KB free area and it can be got from 560K@64K area
luckily. I think the size find_early_table_space() requests was a bug.
After the bug is fixed by the commit 722bc6, find_early_table_space()
requires 1.8MB area and it fails as I wrote.

Thanks,
Takao Indoh

> 
>>
>> Thanks,
>> Takao Indoh
>>
>> (2012/06/12 14:21), Cong Wang wrote:
>>> From: Cong Wang <xiyou.wangcong@gmail.com>
>>>
>>> This reverts the following commit:
>>>
>>> 	commit 8548c84da2f47e71bbbe300f55edb768492575f7
>>> 	Author: Takashi Iwai <tiwai@suse.de>
>>> 	Date:   Sun Oct 23 23:19:12 2011 +0200
>>>
>>> 	    x86: Fix S4 regression
>>> 	
>>> 	    Commit 4b239f458 ("x86-64, mm: Put early page table high") causes a S4
>>> 	    regression since 2.6.39, namely the machine reboots occasionally at S4
>>> 	    resume.  It doesn't happen always, overall rate is about 1/20.  But,
>>> 	    like other bugs, once when this happens, it continues to happen.
>>> 	
>>> 	    This patch fixes the problem by essentially reverting the memory
>>> 	    assignment in the older way.
>>>
>>> According to the previous discussion:
>>> http://marc.info/?l=linux-kernel&m=133161674120253&w=2
>>> it seems that so far the best solution is just reverting it.
>>>
>>> Takashi, could you help to test if the S4 regression is still
>>> there after this patch?
>>>
>>> Reported-by: CAI Qian <caiqian@redhat.com>
>>> Cc: Dave Young <dyoung@redhat.com>
>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>>> Cc: Yinghai Lu <yinghai.lu@oracle.com>
>>> Cc: Takashi Iwai <tiwai@suse.de>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>>
>>> ---
>>>    arch/x86/mm/init.c |    3 ++-
>>>    1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>>> index bc4e9d8..7ab7975 100644
>>> --- a/arch/x86/mm/init.c
>>> +++ b/arch/x86/mm/init.c
>>> @@ -74,8 +74,9 @@ static void __init find_early_table_space(struct map_range *mr, unsigned long en
>>>    #ifdef CONFIG_X86_32
>>>    	/* for fixmap */
>>>    	tables += roundup(__end_of_fixed_addresses * sizeof(pte_t), PAGE_SIZE);
>>> -#endif
>>> +
>>>    	good_end = max_pfn_mapped << PAGE_SHIFT;
>>> +#endif
>>>    
>>>    	base = memblock_find_in_range(start, good_end, tables, PAGE_SIZE);
>>>    	if (!base)
>>>
> 
> 
> 

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Dave Young]

On 07/23/2012 07:22 PM, Takao Indoh wrote:

> (2012/07/23 19:00), Dave Young wrote:
>> On 07/17/2012 11:15 AM, Takao Indoh wrote:
>>
>>> Hi Cong,
>>>
>>> When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
>>> kernel's panic in find_early_table_space().
>>>
>>> init_memory_mapping: [mem 0x00000000-0x36ffafff]
>>> Kernel panic - not syncing: Cannot find space for the kernel page tables
>>> Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
>>> Call Trace:
>>>   [<ffffffff8158549b>] panic+0xb8/0x1c8
>>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
>>>   [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
>>>   [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
>>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
>>>   [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
>>>   [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
>>>   [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
>>>
>>> In find_early_table_space(), a kernel tries to find free area below 512M
>>> for pgtable using memblock_find_in_range, but it fails because kdump
>>> kernel does not have enough free space below 512M due to the memmap
>>> restriction. This is the memmap option specified against kdump kernel
>>> when crashkernel=128M.
>>>
>>> memmap=560K@64K memmap=130492K@770608K
>>>
>>> Only 560KB area is available and it is not sufficient for pgtable (it
>>> seems that about 1.8MB area is needed for pgtable). This problem is
>>> fixed by your revert patch. I hope this patch gets merged.
>>
>>
>> I can reproduce this issue as well, probably related to some x86 mm init
>> commits, this alloc failure does not happen with reverting below commits:
>>
>> bd2753b2dda7bb43c7468826de75f49c6a7e8965
>> 722bc6b16771ed80871e1fd81c86d3627dda2ac8
> Yeah, my result of bisect is as follows and at first I thought the
> commit 722bc6 caused this regression.
> 
> 722bc6b16771ed80871e1fd81c86d3627dda2ac8 is the first bad commit
> commit 722bc6b16771ed80871e1fd81c86d3627dda2ac8
> Author: WANG Cong <xiyou.wangcong@gmail.com>
> Date:   Mon Mar 5 15:05:13 2012 -0800
> 
> IIUC, this commit just fixes a bug of counting pgtable entries. As the
> result, another problem came up to the surface. In the case of my
> machine(16GB memory), before applying 722bc6, find_early_table_space()
> requests about 12KB free area and it can be got from 560K@64K area
> luckily. I think the size find_early_table_space() requests was a bug.
> After the bug is fixed by the commit 722bc6, find_early_table_space()
> requires 1.8MB area and it fails as I wrote.


Sounds reasonable, if find_early_table_space is ok then I also vote to
revert the s4 regression fix.

> 
> Thanks,
> Takao Indoh
> 
>>
>>>
>>> Thanks,
>>> Takao Indoh
>>>
>>> (2012/06/12 14:21), Cong Wang wrote:
>>>> From: Cong Wang <xiyou.wangcong@gmail.com>
>>>>
>>>> This reverts the following commit:
>>>>
>>>> 	commit 8548c84da2f47e71bbbe300f55edb768492575f7
>>>> 	Author: Takashi Iwai <tiwai@suse.de>
>>>> 	Date:   Sun Oct 23 23:19:12 2011 +0200
>>>>
>>>> 	    x86: Fix S4 regression
>>>> 	
>>>> 	    Commit 4b239f458 ("x86-64, mm: Put early page table high") causes a S4
>>>> 	    regression since 2.6.39, namely the machine reboots occasionally at S4
>>>> 	    resume.  It doesn't happen always, overall rate is about 1/20.  But,
>>>> 	    like other bugs, once when this happens, it continues to happen.
>>>> 	
>>>> 	    This patch fixes the problem by essentially reverting the memory
>>>> 	    assignment in the older way.
>>>>
>>>> According to the previous discussion:
>>>> http://marc.info/?l=linux-kernel&m=133161674120253&w=2
>>>> it seems that so far the best solution is just reverting it.
>>>>
>>>> Takashi, could you help to test if the S4 regression is still
>>>> there after this patch?
>>>>
>>>> Reported-by: CAI Qian <caiqian@redhat.com>
>>>> Cc: Dave Young <dyoung@redhat.com>
>>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>>>> Cc: Yinghai Lu <yinghai.lu@oracle.com>
>>>> Cc: Takashi Iwai <tiwai@suse.de>
>>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>>>
>>>> ---
>>>>    arch/x86/mm/init.c |    3 ++-
>>>>    1 files changed, 2 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>>>> index bc4e9d8..7ab7975 100644
>>>> --- a/arch/x86/mm/init.c
>>>> +++ b/arch/x86/mm/init.c
>>>> @@ -74,8 +74,9 @@ static void __init find_early_table_space(struct map_range *mr, unsigned long en
>>>>    #ifdef CONFIG_X86_32
>>>>    	/* for fixmap */
>>>>    	tables += roundup(__end_of_fixed_addresses * sizeof(pte_t), PAGE_SIZE);
>>>> -#endif
>>>> +
>>>>    	good_end = max_pfn_mapped << PAGE_SHIFT;
>>>> +#endif
>>>>    
>>>>    	base = memblock_find_in_range(start, good_end, tables, PAGE_SIZE);
>>>>    	if (!base)
>>>>
>>
>>
>>
> 



-- 
Thanks
Dave

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Cong Wang]

On Mon, 2012-07-23 at 20:22 +0900, Takao Indoh wrote:
> (2012/07/23 19:00), Dave Young wrote:
> > On 07/17/2012 11:15 AM, Takao Indoh wrote:
> > 
> >> Hi Cong,
> >>
> >> When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
> >> kernel's panic in find_early_table_space().
> >>
> >> init_memory_mapping: [mem 0x00000000-0x36ffafff]
> >> Kernel panic - not syncing: Cannot find space for the kernel page tables
> >> Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
> >> Call Trace:
> >>   [<ffffffff8158549b>] panic+0xb8/0x1c8
> >>   [<ffffffff8158565d>] ? printk+0x48/0x4a
> >>   [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
> >>   [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
> >>   [<ffffffff8158565d>] ? printk+0x48/0x4a
> >>   [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
> >>   [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
> >>   [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
> >>
> >> In find_early_table_space(), a kernel tries to find free area below 512M
> >> for pgtable using memblock_find_in_range, but it fails because kdump
> >> kernel does not have enough free space below 512M due to the memmap
> >> restriction. This is the memmap option specified against kdump kernel
> >> when crashkernel=128M.
> >>
> >> memmap=560K@64K memmap=130492K@770608K
> >>
> >> Only 560KB area is available and it is not sufficient for pgtable (it
> >> seems that about 1.8MB area is needed for pgtable). This problem is
> >> fixed by your revert patch. I hope this patch gets merged.
> > 
> > 
> > I can reproduce this issue as well, probably related to some x86 mm init
> > commits, this alloc failure does not happen with reverting below commits:
> > 
> > bd2753b2dda7bb43c7468826de75f49c6a7e8965
> > 722bc6b16771ed80871e1fd81c86d3627dda2ac8
> Yeah, my result of bisect is as follows and at first I thought the
> commit 722bc6 caused this regression.
> 
> 722bc6b16771ed80871e1fd81c86d3627dda2ac8 is the first bad commit
> commit 722bc6b16771ed80871e1fd81c86d3627dda2ac8
> Author: WANG Cong <xiyou.wangcong@gmail.com>
> Date:   Mon Mar 5 15:05:13 2012 -0800
> 
> IIUC, this commit just fixes a bug of counting pgtable entries. As the
> result, another problem came up to the surface. In the case of my
> machine(16GB memory), before applying 722bc6, find_early_table_space()
> requests about 12KB free area and it can be got from 560K@64K area
> luckily. I think the size find_early_table_space() requests was a bug.
> After the bug is fixed by the commit 722bc6, find_early_table_space()
> requires 1.8MB area and it fails as I wrote.

Thanks for tracking this, Takao!

I bet you are using x86_64 not x86 PAE? If so, could you try this patch
https://patchwork.kernel.org/patch/1195751/
? I already reviewed it.

Sorry for the trouble!

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Takao Indoh]

(2012/07/25 0:55), Cong Wang wrote:
> On Mon, 2012-07-23 at 20:22 +0900, Takao Indoh wrote:
>> (2012/07/23 19:00), Dave Young wrote:
>>> On 07/17/2012 11:15 AM, Takao Indoh wrote:
>>>
>>>> Hi Cong,
>>>>
>>>> When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
>>>> kernel's panic in find_early_table_space().
>>>>
>>>> init_memory_mapping: [mem 0x00000000-0x36ffafff]
>>>> Kernel panic - not syncing: Cannot find space for the kernel page tables
>>>> Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
>>>> Call Trace:
>>>>    [<ffffffff8158549b>] panic+0xb8/0x1c8
>>>>    [<ffffffff8158565d>] ? printk+0x48/0x4a
>>>>    [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
>>>>    [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
>>>>    [<ffffffff8158565d>] ? printk+0x48/0x4a
>>>>    [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
>>>>    [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
>>>>    [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
>>>>
>>>> In find_early_table_space(), a kernel tries to find free area below 512M
>>>> for pgtable using memblock_find_in_range, but it fails because kdump
>>>> kernel does not have enough free space below 512M due to the memmap
>>>> restriction. This is the memmap option specified against kdump kernel
>>>> when crashkernel=128M.
>>>>
>>>> memmap=560K@64K memmap=130492K@770608K
>>>>
>>>> Only 560KB area is available and it is not sufficient for pgtable (it
>>>> seems that about 1.8MB area is needed for pgtable). This problem is
>>>> fixed by your revert patch. I hope this patch gets merged.
>>>
>>>
>>> I can reproduce this issue as well, probably related to some x86 mm init
>>> commits, this alloc failure does not happen with reverting below commits:
>>>
>>> bd2753b2dda7bb43c7468826de75f49c6a7e8965
>>> 722bc6b16771ed80871e1fd81c86d3627dda2ac8
>> Yeah, my result of bisect is as follows and at first I thought the
>> commit 722bc6 caused this regression.
>>
>> 722bc6b16771ed80871e1fd81c86d3627dda2ac8 is the first bad commit
>> commit 722bc6b16771ed80871e1fd81c86d3627dda2ac8
>> Author: WANG Cong <xiyou.wangcong@gmail.com>
>> Date:   Mon Mar 5 15:05:13 2012 -0800
>>
>> IIUC, this commit just fixes a bug of counting pgtable entries. As the
>> result, another problem came up to the surface. In the case of my
>> machine(16GB memory), before applying 722bc6, find_early_table_space()
>> requests about 12KB free area and it can be got from 560K@64K area
>> luckily. I think the size find_early_table_space() requests was a bug.
>> After the bug is fixed by the commit 722bc6, find_early_table_space()
>> requires 1.8MB area and it fails as I wrote.
>
> Thanks for tracking this, Takao!
>
> I bet you are using x86_64 not x86 PAE? If so, could you try this patch
> https://patchwork.kernel.org/patch/1195751/
> ? I already reviewed it.

Great, I applied it and now kdump works. Thanks!

Thanks,
Takao Indoh

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Vivek Goyal]

On Wed, Jul 25, 2012 at 09:19:08AM +0900, Takao Indoh wrote:
> (2012/07/25 0:55), Cong Wang wrote:
> >On Mon, 2012-07-23 at 20:22 +0900, Takao Indoh wrote:
> >>(2012/07/23 19:00), Dave Young wrote:
> >>>On 07/17/2012 11:15 AM, Takao Indoh wrote:
> >>>
> >>>>Hi Cong,
> >>>>
> >>>>When I tested kdump with 3.5.0-rc6 kernel, I found a problem of kdump
> >>>>kernel's panic in find_early_table_space().
> >>>>
> >>>>init_memory_mapping: [mem 0x00000000-0x36ffafff]
> >>>>Kernel panic - not syncing: Cannot find space for the kernel page tables
> >>>>Pid: 0, comm: swapper Not tainted 3.5.0-rc6 #17
> >>>>Call Trace:
> >>>>   [<ffffffff8158549b>] panic+0xb8/0x1c8
> >>>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
> >>>>   [<ffffffff8157304c>] init_memory_mapping+0x46c/0x530
> >>>>   [<ffffffff818a73c7>] setup_arch+0x669/0xb0e
> >>>>   [<ffffffff8158565d>] ? printk+0x48/0x4a
> >>>>   [<ffffffff818a3a1f>] start_kernel+0x9b/0x34a
> >>>>   [<ffffffff818a332d>] x86_64_start_reservations+0x131/0x136
> >>>>   [<ffffffff818a341f>] x86_64_start_kernel+0xed/0xf4
> >>>>
> >>>>In find_early_table_space(), a kernel tries to find free area below 512M
> >>>>for pgtable using memblock_find_in_range, but it fails because kdump
> >>>>kernel does not have enough free space below 512M due to the memmap
> >>>>restriction. This is the memmap option specified against kdump kernel
> >>>>when crashkernel=128M.
> >>>>
> >>>>memmap=560K@64K memmap=130492K@770608K
> >>>>
> >>>>Only 560KB area is available and it is not sufficient for pgtable (it
> >>>>seems that about 1.8MB area is needed for pgtable). This problem is
> >>>>fixed by your revert patch. I hope this patch gets merged.
> >>>
> >>>
> >>>I can reproduce this issue as well, probably related to some x86 mm init
> >>>commits, this alloc failure does not happen with reverting below commits:
> >>>
> >>>bd2753b2dda7bb43c7468826de75f49c6a7e8965
> >>>722bc6b16771ed80871e1fd81c86d3627dda2ac8
> >>Yeah, my result of bisect is as follows and at first I thought the
> >>commit 722bc6 caused this regression.
> >>
> >>722bc6b16771ed80871e1fd81c86d3627dda2ac8 is the first bad commit
> >>commit 722bc6b16771ed80871e1fd81c86d3627dda2ac8
> >>Author: WANG Cong <xiyou.wangcong@gmail.com>
> >>Date:   Mon Mar 5 15:05:13 2012 -0800
> >>
> >>IIUC, this commit just fixes a bug of counting pgtable entries. As the
> >>result, another problem came up to the surface. In the case of my
> >>machine(16GB memory), before applying 722bc6, find_early_table_space()
> >>requests about 12KB free area and it can be got from 560K@64K area
> >>luckily. I think the size find_early_table_space() requests was a bug.
> >>After the bug is fixed by the commit 722bc6, find_early_table_space()
> >>requires 1.8MB area and it fails as I wrote.
> >
> >Thanks for tracking this, Takao!
> >
> >I bet you are using x86_64 not x86 PAE? If so, could you try this patch
> >https://patchwork.kernel.org/patch/1195751/
> >? I already reviewed it.
> 
> Great, I applied it and now kdump works. Thanks!

Did something happen on this patch. We definitely want to gain back the
capability to be able to reserve 512MB of kdump memory.

Thanks
Vivek

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Yinghai Lu]

On Mon, Aug 6, 2012 at 1:42 PM, Vivek Goyal <vgoyal@redhat.com> wrote:

>
> Did something happen on this patch. We definitely want to gain back the
> capability to be able to reserve 512MB of kdump memory.

Maybe Ingo and peter could push that to Linus.

Assume we have Acked-by from you, me and others.

Thanks

Yinghai

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Jerry Snitselaar]

On Wed Jul 25 12, Takao Indoh wrote:
> >Thanks for tracking this, Takao!
> >
> >I bet you are using x86_64 not x86 PAE? If so, could you try this patch
> >https://patchwork.kernel.org/patch/1195751/
> >? I already reviewed it.
> 
> Great, I applied it and now kdump works. Thanks!
> 
> Thanks,
> Takao Indoh
> 

This patch from Jacob Shin solves the problem, and seems like it might
be a better solution.

[PATCH 2/5] x86: find_early_table_space based on memory ranges that
are being mapped

https://lkml.org/lkml/2012/8/9/540

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Jerry Snitselaar]

On Sat Aug 11 12, Jerry Snitselaar wrote:
> On Wed Jul 25 12, Takao Indoh wrote:
> > >Thanks for tracking this, Takao!
> > >
> > >I bet you are using x86_64 not x86 PAE? If so, could you try this patch
> > >https://patchwork.kernel.org/patch/1195751/
> > >? I already reviewed it.
> > 
> > Great, I applied it and now kdump works. Thanks!
> > 
> > Thanks,
> > Takao Indoh
> > 
> 
> This patch from Jacob Shin solves the problem, and seems like it might
> be a better solution.
> 
> [PATCH 2/5] x86: find_early_table_space based on memory ranges that
> are being mapped
> 
> https://lkml.org/lkml/2012/8/9/540
> 
Actually, apply that series of 5 patches.

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[H. Peter Anvin]

On 08/11/2012 11:26 AM, Jerry Snitselaar wrote:
>>
>> This patch from Jacob Shin solves the problem, and seems like it might
>> be a better solution.
>>
>> [PATCH 2/5] x86: find_early_table_space based on memory ranges that
>> are being mapped
>>
>> https://lkml.org/lkml/2012/8/9/540
>>
> Actually, apply that series of 5 patches.
>

I was hoping Tejun would comment on it, but I think I'll pull it into -tip.

However, the real question is what we should do for -stable; applying 
the full patch series seems a big aggressive for that.  On the other 
hand, if it really is The Right Thing then perhaps we should do so anyway.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[H. Peter Anvin]

On 08/11/2012 11:34 AM, H. Peter Anvin wrote:
> On 08/11/2012 11:26 AM, Jerry Snitselaar wrote:
>>>
>>> This patch from Jacob Shin solves the problem, and seems like it might
>>> be a better solution.
>>>
>>> [PATCH 2/5] x86: find_early_table_space based on memory ranges that
>>> are being mapped
>>>
>>> https://lkml.org/lkml/2012/8/9/540
>>>
>> Actually, apply that series of 5 patches.
>>
>
> I was hoping Tejun would comment on it, but I think I'll pull it into -tip.
>
> However, the real question is what we should do for -stable; applying
> the full patch series seems a big aggressive for that.  On the other
> hand, if it really is The Right Thing then perhaps we should do so anyway.
>

Ah, right... still waiting for a rev of the patch to address Yinghai's 
legitimate request for minor code restructuring.  Other than that, the 
patchset is really The Right Thing.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: revert "x86: Fix S4 regression"

[Tejun Heo]

On Sat, Aug 11, 2012 at 11:34:22AM -0700, H. Peter Anvin wrote:
> On 08/11/2012 11:26 AM, Jerry Snitselaar wrote:
> >>
> >>This patch from Jacob Shin solves the problem, and seems like it might
> >>be a better solution.
> >>
> >>[PATCH 2/5] x86: find_early_table_space based on memory ranges that
> >>are being mapped
> >>
> >>https://lkml.org/lkml/2012/8/9/540
> >>
> >Actually, apply that series of 5 patches.
> >
> 
> I was hoping Tejun would comment on it, but I think I'll pull it into -tip.

Wasn't cc'd.  Will take a look.

Thanks.

-- 
tejun