x86: Fix S4 regression

PATCH Yinghai Lu 3 participants 4 messages Oct 24, 2011 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): s5hwrbvv227.wl%[email protected]

Yinghai Lu

author

2011-10-24 00:29 UTC #1 lore raw

Quoted 8 lines · Takashi Iwai

The 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.

Quoted 8 lines

Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
I resend this as a "fix" patch now before it's forgotten and rotten. It's just papering again over the mystery, but IMO better than the hard-reset behavior as of now. Unfortunately, bisection is pretty much difficult because the bug itself is fairly unstable...

Did you try to check several commit that Rafael pointed out:

Quoted 9 lines · Rafael J. Wysocki

Quoted 5 lines · Takashi Iwai
If my previous test -- 2.6.37+Yinghai's patches didn't show the problem -- is correct, it means that some change in 2.6.38 reacted badly with Yinghai's patches, not about 2.6.39. I'll check tomorrow again whether this observation is really correct.
Yes, that would be good to know, thanks for doing this!

Quoted 7 lines

If that turns out to be the case, there are the following commits looking like worth checking:
d344e38 x86, nx: Mark the ACPI resume trampoline code as +x 884b821 ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory() (v2) d551d81 ACPI / PM: Call suspend_nvs_free() earlier during resume 2d6d9fd ACPI: Introduce acpi_os_ioremap()

Thanks

Yinghai Lu

2011-10-24 04:10 UTC #2 lore raw

Quoted 21 lines · Yinghai Lu

Quoted 8 lines · Takashi Iwai
The 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.
Quoted 8 lines
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
I resend this as a "fix" patch now before it's forgotten and rotten. It's just papering again over the mystery, but IMO better than the hard-reset behavior as of now. Unfortunately, bisection is pretty much difficult because the bug itself is fairly unstable...

Quoted 21 lines

Did you try to check several commit that Rafael pointed out:
Quoted 16 lines · Rafael J. Wysocki
Quoted 5 lines · Takashi Iwai
If my previous test -- 2.6.37+Yinghai's patches didn't show the problem -- is correct, it means that some change in 2.6.38 reacted badly with Yinghai's patches, not about 2.6.39. I'll check tomorrow again whether this observation is really correct.
Yes, that would be good to know, thanks for doing this!
If that turns out to be the case, there are the following commits looking like worth checking:
d344e38 x86, nx: Mark the ACPI resume trampoline code as +x 884b821 ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory() (v2) d551d81 ACPI / PM: Call suspend_nvs_free() earlier during resume 2d6d9fd ACPI: Introduce acpi_os_ioremap()

Also, can you check if reverting following patch could help?

| commit e5f15b45ddf3afa2bbbb10c7ea34fb32b6de0a0e
| Author: Yinghai Lu <[email protected]>
| Date:   Fri Feb 18 11:30:30 2011 +0000
|
|    x86: Cleanup highmap after brk is concluded
|
|    Now cleanup_highmap actually is in two steps: one is early in head64.c
|    and only clears above _end; a second one is in init_memory_mapping() and
|    tries to clean from _brk_end to _end.
|    It should check if those boundaries are PMD_SIZE aligned but currently
|    does not.
|    Also init_memory_mapping() is called several times for numa or memory
|    hotplug, so we really should not handle initial kernel mappings there.
|
|   This patch moves cleanup_highmap() down after _brk_end is settled so
|    we can do everything in one step.
|    Also we honor max_pfn_mapped in the implementation of cleanup_highmap.

Thanks

Yinghai Lu

Takashi Iwai

2011-10-24 09:03 UTC #3 lore raw

At Sun, 23 Oct 2011 21:10:11 -0700,

Quoted 39 lines · Yinghai Lu

Quoted 37 lines · Yinghai Lu
Quoted 8 lines · Takashi Iwai
The 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.
Quoted 8 lines
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
I resend this as a "fix" patch now before it's forgotten and rotten. It's just papering again over the mystery, but IMO better than the hard-reset behavior as of now. Unfortunately, bisection is pretty much difficult because the bug itself is fairly unstable...
Did you try to check several commit that Rafael pointed out:
Quoted 12 lines · Rafael J. Wysocki
Quoted 5 lines · Takashi Iwai
If my previous test -- 2.6.37+Yinghai's patches didn't show the problem -- is correct, it means that some change in 2.6.38 reacted badly with Yinghai's patches, not about 2.6.39. I'll check tomorrow again whether this observation is really correct.
Yes, that would be good to know, thanks for doing this!
If that turns out to be the case, there are the following commits looking like worth checking:

Quoted 7 lines

Quoted 5 lines
Quoted 4 lines
d344e38 x86, nx: Mark the ACPI resume trampoline code as +x 884b821 ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory() (v2) d551d81 ACPI / PM: Call suspend_nvs_free() earlier during resume 2d6d9fd ACPI: Introduce acpi_os_ioremap()
Also, can you check if reverting following patch could help?

OK, I'll try it later but in the next week, as I'll be in conferences for the whole this week...

thanks,

Takashi

Quoted 22 lines

| commit e5f15b45ddf3afa2bbbb10c7ea34fb32b6de0a0e
| Author: Yinghai Lu <[email protected]>
| Date:   Fri Feb 18 11:30:30 2011 +0000
|
|    x86: Cleanup highmap after brk is concluded
|
|    Now cleanup_highmap actually is in two steps: one is early in head64.c
|    and only clears above _end; a second one is in init_memory_mapping() and
|    tries to clean from _brk_end to _end.
|    It should check if those boundaries are PMD_SIZE aligned but currently
|    does not.
|    Also init_memory_mapping() is called several times for numa or memory
|    hotplug, so we really should not handle initial kernel mappings there.
|
|   This patch moves cleanup_highmap() down after _brk_end is settled so
|    we can do everything in one step.
|    Also we honor max_pfn_mapped in the implementation of cleanup_highmap.

Thanks

Yinghai Lu

Takashi Iwai

2011-10-24 09:02 UTC #4 lore raw

In reply to Yinghai Lu

At Sun, 23 Oct 2011 17:29:24 -0700,

Quoted 22 lines · Yinghai Lu

Quoted 8 lines · Takashi Iwai
The 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.
Quoted 8 lines
Cc <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
---
I resend this as a "fix" patch now before it's forgotten and rotten. It's just papering again over the mystery, but IMO better than the hard-reset behavior as of now. Unfortunately, bisection is pretty much difficult because the bug itself is fairly unstable...

Quoted 20 lines

Did you try to check several commit that Rafael pointed out:
Quoted 16 lines · Rafael J. Wysocki
Quoted 5 lines · Takashi Iwai
If my previous test -- 2.6.37+Yinghai's patches didn't show the problem -- is correct, it means that some change in 2.6.38 reacted badly with Yinghai's patches, not about 2.6.39. I'll check tomorrow again whether this observation is really correct.
Yes, that would be good to know, thanks for doing this!
If that turns out to be the case, there are the following commits looking like worth checking:
d344e38 x86, nx: Mark the ACPI resume trampoline code as +x 884b821 ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory() (v2) d551d81 ACPI / PM: Call suspend_nvs_free() earlier during resume 2d6d9fd ACPI: Introduce acpi_os_ioremap()

Yes, but these are harmless, as far as I've tested.

Takashi