<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GLib] Use a GSource instead of a thread to poll memory pressure eventFD in linux implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=159346#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GLib] Use a GSource instead of a thread to poll memory pressure eventFD in linux implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=159346">bug 159346</a>
from <span class="vcard"><a class="email" href="mailto:tonikitoo@webkit.org" title="Antonio Gomes <tonikitoo@webkit.org>"> <span class="fn">Antonio Gomes</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=283522&action=diff" name="attach_283522" title="Updated patch">attachment 283522</a> <a href="attachment.cgi?id=283522&action=edit" title="Updated patch">[details]</a></span>
Updated patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=283522&action=review">https://bugs.webkit.org/attachment.cgi?id=283522&action=review</a>
<span class="quote">> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:108
> + EventFDSource* eventFDSource = reinterpret_cast<EventFDSource*>(m_source.get());</span >
that reinterpret_cast was a bit tricky to understand, but Zan cleared it up (line 101 allocates m_source with "sizeof(EventFDSource)".
<span class="quote">> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:145
> + LOG(MemoryPressure, "Invalidate eventfd.");</span >
s/Invalidate/Invalid/g
<span class="quote">> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:230
> + m_eventFDPoller = std::make_unique<EventFDPoller>(m_eventFD.value(), [this] {</span >
So my understand is that m_eventFDPoller is an alternative memory pressure source that works independent from the existing 'cgroup' solution, right?
However, it gets initialized after the cgroup initialization stuff (lines 209 and 215). So, if cgroup fails to initialize - which happens in most cases - then ::install bails out earlier (lines 218 and 225) and line 230 is not executed.
Should line 230 be our first option, and cgroup the second?
<span class="quote">> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:234
> + // FIXME: Current memcg does not provide any way for users to know how serious the memory pressure is.
> + // So we assume all notifications from memcg are critical for now. If memcg had better inferfaces
> + // to get a detailed memory pressure level in the future, we should update here accordingly.
> + bool critical = true;</span >
I believe a similar comment could be in MemoryPressureHandler::EventFDPoller::readAndNotify(), before we call m_nofityHandler(); Maybe a reduced version of it?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>