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

[Vladimir Kozlov]

Looks good but you don't need 'next' label.

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
>