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

[Stefan Karlsson]

Hi Thomas,

On 2016-11-22 18:09, Thomas Stüfe wrote:
> Hi Stefan,
>
> this change looks good!

Thanks!

>
> Small nitpick, there exists already a function returning a pointer to 
> the free list by chunk index (ChunkManager::free_chunks(index)). You 
> could have implemented ChunkManager::list_chunk_size() using this 
> function (return free_chunks(index)->size()) and add your assert to 
> ChunkManager::free_chunks(index) instead. Or, alternativly, just use 
> free_chunks(index)->size() directly instead of adding list_chunk_size().

Sure. I'll send out a new patch including your suggestion.

Thanks,
StefanK
>
> Kind Regards, Thomas
>
>
> On Tue, Nov 22, 2016 at 3:54 PM, Stefan Karlsson 
> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> 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/%7Estefank/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
>
>