Bug 213646

Summary: [GTK][WPE] Fix the matching of an empty value in getCgroupControllerPath() when only cgroupsV2 hierarchy is found
Product: WebKit Reporter: Pablo Saavedra <psaavedra>
Component: WebKitGTKAssignee: Pablo Saavedra <psaavedra>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, clopez, mcatanzaro, pnormand
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch none

Description Pablo Saavedra 2020-06-26 08:20:59 PDT
"[" (fscanf) Matches ONLY a nonempty sequence of characters from the specified set. This is good enough for cgroups version 1 but when you are in a pure cgroups version 2 environment probably you need to be able to parse the "0::/some_path" chain. This chain should be parseable by the scanf line but it is not due to the reason exposed early:

    int scanResult = fscanf(cgroupControllerFile, "%*u:%" STRINGIFY(CGROUP_NAME_BUFFER_SIZE) "[^:]:%" STRINGIFY(PATH_MAX) "[^\n]", name, path);


From cgroups man page:

    The colon-separated fields are, from left to right:

              1. For cgroups version 1 hierarchies, this field contains a
                 unique hierarchy ID number that can be matched to a hierar‐
                 chy ID in /proc/cgroups.  For the cgroups version 2 hierar‐
                 chy, this field contains the value 0.

              2. For cgroups version 1 hierarchies, this field contains a
                 comma-separated list of the controllers bound to the hier‐
                 archy.  For the cgroups version 2 hierarchy, this field is
                 empty.

              3. This field contains the pathname of the control group in
                 the hierarchy to which the process belongs.  This pathname
                 is relative to the mount point of the hierarchy.
Comment 1 Pablo Saavedra 2020-06-26 08:33:29 PDT
Created attachment 402865 [details]
patch
Comment 2 Adrian Perez 2020-06-26 13:00:56 PDT
Comment on attachment 402865 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402865&action=review

> Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:220
> +            // cgroupv2

You can remove this comment because the comparisong with
the CGROUP_V2_HIERARCHY constant in the line right above
is self-explanatory—which is why we like using constants
with meaningful names, of course :)
Comment 3 Pablo Saavedra 2020-06-26 14:10:22 PDT
Created attachment 402900 [details]
patch
Comment 4 Pablo Saavedra 2020-06-26 14:11:10 PDT
(In reply to Adrian Perez from comment #2)
> Comment on attachment 402865 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402865&action=review
> 
> > Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:220
> > +            // cgroupv2
> 
> You can remove this comment because the comparisong with
> the CGROUP_V2_HIERARCHY constant in the line right above
> is self-explanatory—which is why we like using constants
> with meaningful names, of course :)

done.
Comment 5 EWS 2020-06-26 14:53:40 PDT
Committed r263589: <https://trac.webkit.org/changeset/263589>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402900 [details].
Comment 6 Philippe Normand 2020-06-28 04:50:56 PDT
Debug build fixed in https://trac.webkit.org/changeset/263632/webkit ...