[Webkit-unassigned] [Bug 155255] MemoryPressureHandler doesn't work if cgroups aren't present in Linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=155255

--- Comment #38 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #37)
> Comment on attachment 283624 [details]
> Try to fix builds
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=283624&action=review

Thanks for the review.

> > Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:196
> > +bool MemoryPressureHandler::tryEnsureEventFD()
> 
> tryEnsure seems a bit weird name to me.

It tries to ensure an eventFD. The try is because it can fail, and the ensure is because it doesn't return the new FD, but updates the member, and does nothing if we already have an fd.

> > Source/WebKit2/NetworkProcess/NetworkProcess.cpp:196
> > +void NetworkProcess::initializeNetworkProcess(NetworkProcessCreationParameters&& parameters)
> 
> Seems unrelated

It's not actually. We can't do Attachment::releaseFileDescriptor with a const reference. This change is consistent with all other initializeFooProcess methods.

> > Source/WebKit2/PluginProcess/PluginProcess.cpp:142
> > +    memoryPressureHandler.install();
> 
> I understand you move this here because you need to set the handle before
> the install, I hope this does not add any race condition I've seen before
> when moving stuff like this.

No, I don't think so, this is what all other processes do, so this is also consistent.

> > Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:47
> > +static const size_t notSet = static_cast<size_t>(-1);
> 
> Instead of notSet, consider making  lowWatermarkPages() and
> calculateMemoryAvailable() return an Optional<size_t>.

Ok, I'll check it again.

> > Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:189
> > +        // See https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773
> 
> I'd find more useful to have this link just above the function definition.
> 
> > Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:195
> > +    return ((memoryTotal - memoryAvailable) * 100) / memoryTotal;
> 
> Do we have any guarantee that memoryTotal is not 0?
> 
> Do we have any guarantee that memoryTotal > memoryAvailable ?

I'll check this

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160718/bab97cdd/attachment.html>