Memory pressure watcher causes CPU spin

RESOLVED FIXED in Firefox OS v1.4

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cyu, Assigned: tzimmermann)

Tracking

(Blocks: 1 bug, {qablocker, regression, smoketest})

unspecified
1.4 S1 (14feb)
ARM
Gonk (Firefox OS)
qablocker, regression, smoketest
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.4+, b2g-v1.4 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
We found that on today's build, GP has CPU spin on the IO threads of several processes. The CPU spin disappears when we disable GonkMemoryPressureWatcher. Adding log to MemoryPressureWatcher::CheckForMemoryPressure() and then we saw logs of log messages appear in adb logcat.

Reverting the following change makes the problem go away:
parent:      169119:6f957a000186
user:        Thomas Zimmermann <tdz@users.sourceforge.net>
date:        Mon Feb 17 12:29:26 2014 +0100
summary:     Bug 970895: Use I/O loop for polling memory-pressure events, r=dhylands

I guess GP's memory pressure device doesn't support epoll. We need GP to confirm.
Flags: needinfo?(gp)
This is probably a regression from bug 970895.
I'll have a look.
Assignee: nobody → tzimmermann
Status: NEW → ASSIGNED
(Reporter)

Comment 3

4 years ago
MemoryPressureWatcher was originally using poll(), but running on the IPC thread makes it use epoll(). I think this is the main difference.

This bug appears on GP only (for now, other devices not checked yet). unagi isn't found to have this CPU spin.
There is tons of stuff running on the I/O thread. And non of this causes any CPU spinning. Maybe the lowmem interface is different on GP. Do you use Peak or Keon?
(Reporter)

Comment 5

4 years ago
It's peak.
(In reply to Cervantes Yu from comment #3)
> This bug appears on GP only (for now, other devices not checked yet). unagi
> isn't found to have this CPU spin.

unagis don't have the appropriate sysfs interface so we don't check for memory pressure there, I'll test on my hamachi and see if the problem reproduces there too.
I haven't see this problem on the Hamachi when testing bug 970895.
I think I've found the bug. Need to check this though.
Blocks: 970895
I think the problem is how we poll the sysfs file. According to the commit message for poll-able sysfs files [1] the correct way is to poll for POLLPRI events. The original code did this, but the I/O thread uses POLLIN, which is always true.

I guess we need to revert bug 970895. :(


[1] https://lkml.org/lkml/2006/4/14/126
Created attachment 8377537 [details] [diff] [review]
[01] Bug 973824: Revert bug 970895
Attachment #8377537 - Flags: review?(dhylands)
Now that I know what to look for, it is reproducible on the Hamachi.
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #9)
> I think the problem is how we poll the sysfs file. According to the commit
> message for poll-able sysfs files [1] the correct way is to poll for POLLPRI
> events. The original code did this, but the I/O thread uses POLLIN, which is
> always true.

Awww, yes. The node is always readable so POLLIN will always return immediately.

> I guess we need to revert bug 970895. :(

Is there no way to poll for POLLPRI in the I/O thread?
Duplicate of this bug: 973940
Flagging for qablocker, as this regression is blocking bug 973940, which is a automation and smoketest blocker.
blocking-b2g: --- → 1.4?
Keywords: qablocker, smoketest
Editing subject, as this is not just Geeksphone related.   Testers in bug 973940 have reported this regression has occured on the Buri devices.

Build info:
Gaia      ae90f9b322509ee09fbd3963bd23e142845613ab
Gecko     https://hg.mozilla.org/mozilla-central/rev/318c0d6e24c5
BuildID   20140218040203
Version   30.0a1
ro.build.version.incremental=eng.zxliu.20130911.142924
ro.build.date=Wed Sep 11 14:29:37 CST 2013
Keywords: regression
Summary: [Geeksphone] Memory pressure watcher causes CPU spin → Memory pressure watcher causes CPU spin
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #10)
> Created attachment 8377537 [details] [diff] [review]
> [01] Bug 973824: Revert bug 970895

No need for reviews to land backouts
Going to ping a sheriff for a backout on m-c asap.
Clearing needinfo on geeksphone as the issue isn't geeksphone-specific.
Flags: needinfo?(gp)
Comment on attachment 8377537 [details] [diff] [review]
[01] Bug 973824: Revert bug 970895

Ed is in the process of backing this patch out on m-c, so clearing the review.
Attachment #8377537 - Flags: review?(dhylands)
And...backed out.

https://hg.mozilla.org/mozilla-central/rev/eb675c2b6dee
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
blocking-b2g: 1.4? → 1.4+
status-b2g-v1.4: --- → fixed
Target Milestone: --- → 1.4 S1 (14feb)
Looks like this is still affecting b2g-inbound.
Blocks: 973689
You need to log in before you can comment on or make changes to this bug.