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

[Stefan Karlsson]

Thanks, Erik.

StefanK

On 2016-11-23 08:09, Erik Helin wrote:
> On 11/22/2016 10:37 PM, Stefan Karlsson 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
>
> Hey StefanK, thanks for taking care of this! The patch looks good to me,
> Reviewed.
>
> Thanks,
> Erik
>
>> 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
>>>
>>> There's a great description of the bug in the bug report:
>>>  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
>>
>>