Request for review(M): 4930919: race condition in MDO creation at back branch locations

[Tom Rodriguez]

Looks good.

tom

On Jan 10, 2011, at 6:23 PM, Igor Veresov wrote:

> On 1/10/11 5:33 PM, Vladimir Kozlov wrote:
>> Looks good but you don't need 'next' label.
>> 
> 
> Right. Forgot it. Thanks!
> 
> igor
> 
>> Vladimir
>> 
>> Igor Veresov wrote:
>>> On 1/10/11 10:51 AM, Vladimir Kozlov wrote:
>>>> Igor Veresov wrote:
>>>>> The problem arises when a thread fails to allocate an MDO (by loosing
>>>>> the race) and so profile_method() returns 0. And then we reload the
>>>>> MDO pointer and see it's not null and has been just set by another
>>>>> thread which makes us decide that the data index value the
>>>>> profile_method() returned is valid.
>>>>> 
>>>>> The solution is to reuse set_method_data_for_bcp() set up mdp: make
>>>>> profile_method() just to create MDO and leave bcp->di translation for
>>>>> set_method_data_for_bcp().
>>>>> 
>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/4930919/webrev.00/
>>>> 
>>>> Looks good. One suggestion.
>>>> Swapping registers in addptr() will allow you to avoid double branch:
>>> 
>>> That's an excellent suggestion, thank you!
>>> 
>>> igor
>>>> 
>>>> testptr(rax, rax);
>>>> ! jcc(Assembler::zero, set_mdp); // set mdp to zero
>>>> 
>>>> // rbx,: method
>>>> // rsi: bcp
>>>> call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::bcp_to_di),
>>>> rbx, rsi);
>>>> // rax,: mdi
>>>> 
>>>> + // mdo is guaranteed to be non-zero here, we checked for it before the
>>>> call.
>>>> movptr(rbx, Address(rbx,
>>>> in_bytes(methodOopDesc::method_data_offset())));
>>>> testptr(rbx, rbx);
>>>> jcc(Assembler::zero, zero_continue);
>>>> addptr(rbx, in_bytes(methodDataOopDesc::data_offset()));
>>>> - addptr(rbx, rax);
>>>> + addptr(rax, rbx);
>>>> + bind(set_mdp);
>>>> - movptr(Address(rbp, frame::interpreter_frame_mdx_offset * wordSize),
>>>> rbx);
>>>> + movptr(Address(rbp, frame::interpreter_frame_mdx_offset * wordSize),
>>>> rax);
>>>> 
>>>> Vladimir
>>> 
>