RFR(s): 8152176: Big ParGCCardsPerStrideChunk values can cause overflow for CMS GC

[sangheon]

Thanks, Jon.

Sangheon


On 03/29/2016 02:13 PM, Jon Masamitsu wrote:
>
>
> On 3/29/2016 12:07 PM, sangheon wrote:
>> Hi Jon,
>>
>> Thank you for reviewing this.
>>
>> On 03/29/2016 10:49 AM, Jon Masamitsu wrote:
>>> Sangheon,
>>>
>>> I agree that we don't want to expose 
>>> CardTableModRefBS::_last_valid_index
>>> but I'd consider making CardTableModRefBS::cards_required() public.
>>> That would remove the need to duplicate the calculation and it seems
>>> a reasonable question to ask the card table implementation how many
>>> cards it is going to require.  I'll let other disagree if they like.
>> Good idea and changed to 'public'.
>>
>>>
>>> Add in the message a little more about why the value is too large.
>>>
>>>
>>> 396 CommandLineError::print(verbose,
>>> 397 "ParGCCardsPerStrideChunk (" INTX_FORMAT ") "is too large for 
>>> the heap size and " "must be "
>>> 398 "less than or equal to card table size (" SIZE_FORMAT ")\n",
>>> 399 value, card_table_size);
>> Fixed.
>>
>>> I think a little more information here would help future maintainers so
>>> instead of
>>> 403 // If n_strides which is used with ParGCCardsPerStrideChunk is 
>>> really large, we would face an overflow.
>>>
>> I updated the comment.
>>
>>> This statement can overflow?
>>>
>>> 404 uintx n_strides = ParallelGCThreads * ParGCStridesPerThread;
>> No.
>> Because above statement is checked not to overflow from 
>> ParGCStridesPerThreadConstraintFunc() which is called before this 
>> constraint function. (added this explanation as well)
>
> Ok.
>
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~sangheki/8152176/webrev.01/
>> http://cr.openjdk.java.net/~sangheki/8152176/webrev.01_to_00/
>
> Looks good.
>
> Jon
>
>>
>> Thanks,
>> Sangheon
>>
>>
>>>
>>> Jon
>>>
>>> On 3/28/2016 3:50 PM, sangheon wrote:
>>>> Hi all,
>>>>
>>>> Could I have some reviews for this change?
>>>>
>>>> As very large value of ParGCCardsPerStrideChunk flag would result 
>>>> in an overflow, we need a constraint function after memory 
>>>> initialization. And the function will check the flag to be less 
>>>> than of equal to the size of card table and not to make an overflow 
>>>> with other stride factors(ParallelGCThreads and 
>>>> ParGCStridesPerThread).
>>>>
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8152176
>>>> Webrev: http://cr.openjdk.java.net/~sangheki/8152176/webrev.00
>>>> Testing: JPRT, all runtime/commandline JTREG tests on all platforms
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160329/c56dcda7/attachment.htm>