Bug 156003 - bmalloc: support physical page sizes that don't match the virtual page size (take 2)
Summary: bmalloc: support physical page sizes that don't match the virtual page size (...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-29 19:50 PDT by Geoffrey Garen
Modified: 2016-03-29 23:42 PDT (History)
6 users (show)

See Also:

Attachments
Patch (11.07 KB, patch)
2016-03-29 19:58 PDT, Geoffrey Garen 		kling: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (784.79 KB, application/zip)
2016-03-29 20:45 PDT, Build Bot 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2016-03-29 19:50:06 PDT 
bmalloc: support physical page sizes that don't match the virtual page size (take 2)
Comment 1 Geoffrey Garen 2016-03-29 19:58:32 PDT 
Created attachment 275168 [details]
Patch
Comment 2 Andreas Kling 2016-03-29 20:38:00 PDT 
Comment on attachment 275168 [details]
Patch

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

r=me

> Source/bmalloc/bmalloc/VMAllocate.h:55
> +    static size_t cached;
> +    if (!cached)
> +        cached = sysconf(_SC_PAGESIZE);

Is this meaningfully different from
static size_t cached = sysconf(_SC_PAGESIZE);
?

> Source/bmalloc/bmalloc/VMAllocate.h:64
> +    static size_t cached;
> +    if (!cached)
> +        cached = log2(vmPageSize());
> +    return cached;

Same question.

> Source/bmalloc/bmalloc/VMAllocate.h:96
> +    static size_t cached;
> +    if (!cached)
> +        cached = sysconf(_SC_PAGESIZE);
> +    return cached;

Here too.
Comment 3 Build Bot 2016-03-29 20:45:12 PDT 
Comment on attachment 275168 [details]
Patch

Attachment 275168 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1066936

New failing tests:
inspector/debugger/breakpoint-condition-with-exception.html
inspector/debugger/breakpoint-eval-with-exception.html
Comment 4 Build Bot 2016-03-29 20:45:15 PDT 
Created attachment 275174 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 5 Geoffrey Garen 2016-03-29 21:15:41 PDT 
> > +    static size_t cached;
> > +    if (!cached)
> > +        cached = sysconf(_SC_PAGESIZE);
> 
> Is this meaningfully different from
> static size_t cached = sysconf(_SC_PAGESIZE);

static will use a separate variable to indicate whether the value has been initialized. So, this is one load fewer. Meh.
Comment 6 Geoffrey Garen 2016-03-29 21:17:03 PDT 
(In reply to comment #5)
> > > +    static size_t cached;
> > > +    if (!cached)
> > > +        cached = sysconf(_SC_PAGESIZE);
> > 
> > Is this meaningfully different from
> > static size_t cached = sysconf(_SC_PAGESIZE);
> 
> static will use a separate variable to indicate whether the value has been
> initialized. So, this is one load fewer. Meh.

Better answer: It is not thread-safe to use a separate control variable to indicate that cached has been set. The control variable can appear to be set before cached appears to be set.
Comment 7 Geoffrey Garen 2016-03-29 21:39:55 PDT 
mac-debug-ewe failure appears to be a so-called "flaky" test.

Windows build failure is a pre-existing issue: WebCore/CoreGraphicsSPI.h.

mac-wk2-ews failure -- I'm not sure yet.
Comment 8 Geoffrey Garen 2016-03-29 23:42:32 PDT 
Committed r198829: <http://trac.webkit.org/changeset/198829>