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

[Igor Veresov]

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
>>