RFR: 2178143: VM crashes if the number of bound CPUs changed during runtime

[Jon Masamitsu]

On 3/20/2013 4:35 PM, Yumin Qi wrote:
> Hi, Dan
>
> On 3/20/2013 3:46 PM, Daniel D. Daugherty wrote:
>> On 3/20/13 3:27 PM, Yumin Qi wrote:
>>> Hi, can I have your code review of a small change?
>>>
>>> 2178143:  VM crashes if the number of bound CPUs changed during 
>>> runtime.
>>>
>>> Situation: Customer first configure only one CPU online and turn 
>>> others offline to run java application, after java program started, 
>>> bring more CPUs back online. Since VM started on a single CPU, 
>>> os::is_MP() will return false, but after more CPUs available, OS 
>>> will schedule the app run on multiple CPUs, this caused SEGV in 
>>> various places where data consistency was broken. The solution is 
>>> supply a flag to assume it is running on MP, so lock is forced to be 
>>> called.
>>>
>>> http://cr.openjdk.java.net/~minqi/2178143/
>>
>> The comments in the bug report indicate the possibility of using
>> the new NumberOfProcessors value as a hint for number of GC threads
>> with some sort of upper cap (32). I don't see that change here so
>> I'm guessing that may come via some other changeset...
>>
> The first version included change for ParallelGCThreads but removed 
> after talked to Jon, he suggest not to set ParallelGCThreads = 
> NumberOfProcessors since it will affect a lot of places where default 
> ParallelGCThreads checked.  In fact now the only usage of this flag is 
> make is_MP() return true.  We could use a boolean flag, but I keep the 
> current name in case in future changes rely on the number for GC 
> threads (this also the answer to Harold's question).

Setting ParallelGCThreads to NumberOfProcessors in argument.cpp will 
have ripple
effects that I cannot foresee.  There are lots of places where GC code 
looks a ParallelGCThreads
and adjusts it or makes some decision base on it.   Some other path 
needs to  be trod to
affect the number of GC workers.  I suggested 
available_processor_count() as a
safer path.

Jon

>> src/share/vm/runtime/arguments.cpp
>>
>> 3253   // NumberOfProcessors is a flag used to assume underlying 
>> architecture is MP
>>
>>     Instead of "assume" how about "hint that the" or "indicate that 
>> the".
>>     Might have to break the comment on two lines.
> Good, will change.
>>
>> 3257       "NumberOfProcessors should be greater that 1", NULL);
>>
>>     typo: 'that 1' -> 'than 1'
>>
> Thanks.
>> src/share/vm/runtime/globals.hpp
>>
>> 461           "Assume number of 
>> processors")                                    \
>>
>>     How about:
>>
>>               "Hint about number of processors (must be greater than 1)"
>>
>>     The above text is used by one of the special options for printing
>>     info about the command line options.
>>
> Will change.
>
>
> Thanks
> Yumin
>