RFR: 8169931: 8k class metaspace chunks misallocated from 4k chunk freelist

[Stefan Karlsson]

Thanks, Thomas.

StefanK

On 2016-11-23 08:42, Thomas Stüfe wrote:
> Hi Stefan,
>
> this looks fine!
>
> Thanks,
> Thomas
>
> On Tue, Nov 22, 2016 at 10:37 PM, Stefan Karlsson
> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> wrote:
>
>     Hi all,
>
>     Here are the update patch, with changes suggested by Coleen and Thomas:
>      http://cr.openjdk.java.net/~stefank/8169931/webrev.02.delta
>     <http://cr.openjdk.java.net/~stefank/8169931/webrev.02.delta>
>      http://cr.openjdk.java.net/~stefank/8169931/webrev.02
>     <http://cr.openjdk.java.net/~stefank/8169931/webrev.02>
>
>     Changes to the previous patch:
>     * Removed list_chunk_size and instead used free_chunks(index)->size()
>     * Removed the const qualifier from list_index, since free_chunks
>     isn't declared const. Fixing this would have been a too large change
>     for this bug fix.
>     * Moved ChunkManager_test_list_index into the unit test section of
>     metaspace.cpp
>     * Fixed a broken assert
>
>     Thanks,
>     StefanK
>
>
>     On 2016-11-22 15:54, Stefan Karlsson wrote:
>
>         Hi all,
>
>         Please, review this patch to fix a bug in
>         ChunkManager::list_index():
>          http://cr.openjdk.java.net/~stefank/8169931/webrev.01
>         <http://cr.openjdk.java.net/~stefank/8169931/webrev.01>
>
>         There's a great description of the bug in the bug report:
>          https://bugs.openjdk.java.net/browse/JDK-8169931
>         <https://bugs.openjdk.java.net/browse/JDK-8169931>
>
>         There are two conceptual parts of the metaspace. The _class_
>         metaspace, and the _non-class_ metaspace. They have different
>         chunk sizes, and while querying for the list index of a
>         humongous chunk in the class metaspace, the code accidentally
>         matched the size against the MediumChunk size of the non-class
>         metaspace.
>
>         I've changed the code to not query against the global ChunkSizes
>         enum, but rather the values stored inside the ChunkManager
>         instances. Therefore, the list_index() function was changed into
>         an instance method.
>
>         I've written a unit test that provoked the bug. It's a
>         simplified test with vm asserts instead of gtest asserts. The
>         reason is that the ChunkManager class is currently located in
>         metaspace.cpp, and is not accessible from the gtest unit tests.
>
>         Testing: jprt, Kitchensink, parallel class loading tests
>
>         Thanks,
>         StefanK
>
>
>
>