Bug 213646 - [GTK][WPE] Fix the matching of an empty value in getCgroupControllerPath() when only cgroupsV2 hierarchy is found
Summary: [GTK][WPE] Fix the matching of an empty value in getCgroupControllerPath() wh...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Pablo Saavedra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-26 08:20 PDT by Pablo Saavedra
Modified: 2020-06-28 04:50 PDT (History)
5 users (show)

See Also:


Attachments
patch (5.85 KB, patch)
2020-06-26 08:33 PDT, Pablo Saavedra
no flags Details | Formatted Diff | Diff
patch (5.83 KB, patch)
2020-06-26 14:10 PDT, Pablo Saavedra
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 ...