Closed Bug 973824 Opened 10 years ago Closed 10 years ago

Memory pressure watcher causes CPU spin

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

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

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed

People

(Reporter: cyu, Assigned: tzimmermann)

References

Details

(Keywords: qablocker, regression, smoketest)

Attachments

(1 file)

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
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?
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
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?
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
Closed: 10 years ago
Resolution: --- → FIXED
blocking-b2g: 1.4? → 1.4+
Target Milestone: --- → 1.4 S1 (14feb)
Looks like this is still affecting b2g-inbound.
You need to log in before you can comment on or make changes to this bug.