New issue
Advanced search Search tips

Issue 638755 link

Starred by 1 user

Issue metadata

Status: Verified
Owner: ----
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

suspend_stress_test --memory_check is broken on 4G arm64 machines

Project Member Reported by diand...@chromium.org, Aug 17 2016

Issue description

If you run "suspend_stress_test --memory_check" on rk3399-keivn, you see:

Aborted (core dumped)
Suspend failed.

You can "fix" by hacking the suspend_stress_test script:

  if [ ${FLAGS_memory_check} -eq ${FLAGS_TRUE} ]; then
    suspend_cmd="memory_suspend_test --size=1073741824" 

===

Of course, suspend_stress_test is deprecated, right?  So maybe we don't care?
 

Comment 1 by derat@chromium.org, Aug 23 2016

Is the segfault in memory_suspend_test or in the suspend_stress_test shell script?
memory_suspend_test:

localhost ~ # memory_suspend_test
[0823/103715:FATAL:memory_suspend_test.cc(140)] Check failed: ptr. 
Aborted (core dumped)

Comment 3 by derat@chromium.org, Aug 23 2016

(Sorry about SEGV/ABRT confusion in #1!)

If it's actually deprecated, we should delete the code. I'm happy if we can do that, but are you sure it isn't still being used?

What's the underlying problem here? It looks like memory_suspend_test first reads /proc/meminfo and tries to allocate MemFree + Inactive - 192 MB, and if that fails and we're 32-bit, tries to allocate 2.9 GB. If the allocation(s) all fail, then it aborts. It sounds like allocating just 1 GB works here, but why doesn't the existing code work?
It's deprecated but still in wide use.  ;-P  Like all good things.

Yes, I think we just need to adjust the size it tries to allocate, or possibly make it fallback to a smaller size if the bigger one fails.

Oddly enough, though, I wonder if this is just stupidity.  On arm64 it seems like userspace should easily get close to 4GB of virtual address space.  On arm32 we had to split the virtual space between the kernel and userspace, but that doesn't seem like it should be required on arm64.  Odd...
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/60c0faf2fcd14c88a7872f15b5c458d6e43b7e89

commit 60c0faf2fcd14c88a7872f15b5c458d6e43b7e89
Author: Douglas Anderson <dianders@chromium.org>
Date: Wed Sep 07 18:36:37 2016

power: memory_suspend_test: Use 2.7 GB as fallback, not 2.9 GB

It appears that 2.9 GB isn't quite guaranteed to pass in 32-bit
userspace on all systems.  Let's be a little more conservative and go
down to 2.7 GB.  If there are really memory errors this will be almost
as likely to catch them anyway and then we don't have to try to fiddle
more.

Note: from testing in python, the max I could allocate on rk3399-kevin
was about 0xab175000 bytes.  Specifically:

  >>> import ctypes
  >>> libc = ctypes.CDLL("libc.so.6")
  >>> p = libc.malloc(0xab175000); p; libc.free(p)
  32776
  0
  >>> p = libc.malloc(0xab176000); p; libc.free(p)
  0
  0

From above, max:    2,870,431,744 bytes.
2800 * 1024 * 1024: 2,936,012,800 bytes.
2700 * 1024 * 1024: 2,831,155,200 bytes.

NOTE: it's possible (probable) that we can actually go higher than
0xab175000 bytes in C code (because of the missing python overhead), but
let's just be safe and stick with something that should definitely work.

BUG= chromium:638755 
TEST=suspend_stress_test --memory_check no longer crashes.

Change-Id: I600eceec1d5be203ef36e288f8a80c5a838ee49a
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/382151
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/60c0faf2fcd14c88a7872f15b5c458d6e43b7e89/power_manager/tools/memory_suspend_test.cc

Labels: M-55
Status: Fixed (was: Untriaged)
Status: Verified (was: Fixed)
Verified in ChromeOS: 8872.0.0 / 55.0.2882.0

Sign in to add a comment