RFR(s): 8029162: G1: Shared SATB queue never enabled

[Per Liden]

On 2013-11-29 10:25, Mikael Gerdin wrote:
> On Friday 29 November 2013 10.18.00 Thomas Schatzl wrote:
>> Hi Per,
>>
>> On Fri, 2013-11-29 at 09:47 +0100, Per Liden wrote:
>>> Hi Mikael,
>>>
>>> On 2013-11-29 08:59, Mikael Gerdin wrote:
>>>> Per,
>>>>
>>>> On Thursday 28 November 2013 17.49.53 Per Liden wrote:
>>>>> Summary: When activating/deactivating the G1 pre-barrier all SATB
>>>>> queues
>>>>> are looped through and have their active state set accordingly. There
>>>>> is
>>>>> one queue per Java thread and a shared queue for all non-Java threads.
>>>>> The loop done to set the active state of all queues forgets to set the
>>>>> active state on the shared queue. I suspect this was missed because the
>>>>> logic in SATBMarkQueueSet::set_active_all_threads() is mixed with quite
>>>>> a bit of verification code. This patch also moves this verification
>>>>> code
>>>>> out into a separate function.
>>>>>
>>>>> Testing done: jprt, kitchensink (10 hours), gcbasher (10 hours)
>>>>>
>>>>> http://cr.openjdk.java.net/~pliden/8029162/webrev.0/
>>>> The change looks good to me.
>>> Thanks for reviewing!
>>>
>>>> Since this change enables the bug in JDK-8029255 I assume that these
>>>> changes must be applied together?
>>> Not necessarily applied together, but yes, JDK-892255 needs to be
>>> applied first. That patch is not dependent on this fix.
>> Please strongly consider to push them in the same changeset (you can set
>> two CRs in the header). This avoids somebody ever checking out/using a
>> bugged version.
> I agree.

Please note that there's no double directed dependency between the 
patches. As long as JDK-892255 is pushed first there's no risk of 
checking out a buggy version.

cheers,
/Per