839312 - GonkHal incorrectly setting lowmemorykiller parameters

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Michael Schwartz [:m4]]

$ adb shell cat /sys/module/lowmemorykiller/parameters/adj
 0,1,6,3
 $ adb shell cat /sys/module/lowmemorykiller/parameters/minfree
 256,1024,2048,1280

This results in poor performance and undesirable behaviour in low memory conditions.  This happens because:
# priorityClasses list (http://mxr.mozilla.org/mozilla-b2g18/source/hal/gonk/GonkHal.cpp#976) is indexing into settings from b2g.js (http://mxr.mozilla.org/mozilla-b2g18/source/b2g/app/b2g.js) and therefore populating oomScoreAdj and killUnderMB in the wrong order.  This error results in oomScoreAdj of "0,1,6,3,2" (after mapping to the adj domain) and killUnderMB is similarly messed.  Note, the order in b2g.js looks correct.  Simple fix would be to reorder the priorityClasses list to match b2g.js; a complex fix might be to add sorting
# The last entry of the adj and minfree lists are truncated assumingly here (http://mxr.mozilla.org/mozilla-b2g18/source/hal/gonk/GonkHal.cpp#1005) so that ultimately a cat of adj results in "0,1,6,3"
# The values in b2g.js, although ordered correctly are too low and result in poor performance under low memory conditions; I intend to investigate this further

Comment 1

[Alan Huang]

I also see a typo in b2g.js would cause bug.
http://mxr.mozilla.org/mozilla-b2g18/source/b2g/app/b2g.js#564
>> backgroundPerceivebleKillUnderMB should be backgroundPerceivableKillUnderMB

Comment 2

[Alan Huang]

Attachment: Fix without sorting, v1

This patch intends to give priorityClasses correct order and fix typo in b2g.js

Comment 3

[Justin Lebar (not reading bugmail)]

Comment on attachment 711712 [details] [diff] [review]
Fix without sorting, v1

Nice catch; thank you!

Comment 4

[Justin Lebar (not reading bugmail)]

> # The values in b2g.js, although ordered correctly are too low and result in poor 
> performance under low memory conditions; I intend to investigate this further

It would be helpful if you could file a separate bug on this, so we don't lose track of this issue after we fix the one Alan fixed with is patch here.

Comment 5

[Justin Lebar (not reading bugmail)]

I see why the typo causes problems, but I don't get why the order matters.  So long as |adj| and |minfree| match up, we're OK, right?

Comment 6

[Justin Lebar (not reading bugmail)]

> I see why the typo causes problems, but I don't get why the order matters.

I guess it could have prevented the kernel from interpolating between OOM classes properly?

Anyway, landed.  https://hg.mozilla.org/integration/mozilla-inbound/rev/dc11dee7fffb

Comment 7

[Justin Lebar (not reading bugmail)]

Comment on attachment 711712 [details] [diff] [review]
Fix without sorting, v1

This is a critical bug.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 793105
User impact if declined: We can kill the wrong process sometimes, perhaps contributing to bug 836199 and similar bugs.
Testing completed: Trivial patch.
Risk to taking this patch (and alternatives if risky): Trivial patch.
String or UUID changes made by this patch: None.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 711712 [details] [diff] [review]
Fix without sorting, v1

Approving for uplift to v1.0.1 since this is trivial, not blocking because at this point we will only be blocking specific partner requests.

Comment 9

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/76b7c62e9db4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dc11dee7fffb

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: Bugs fixed on b2g18 after 1/25 merge to v1.0 branch are fixed on v1.0.1 branch.