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

[Jon Masamitsu]

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/f9853e5f/attachment.htm>