RFR: 8145725: Remove the WorkAroundNPTLTimedWaitHang workaround

[David Holmes]

Hi Dmitry,

Great cleanup suggestions. I also did some missing assert -> 
assert_status conversions for the pthread_* functions. (And reverted a 
change that shouldn't have been made - assert_status is for when the 
"status" is the error code.)

New webrev: http://cr.openjdk.java.net/~dholmes/8145725/webrev.v2/

Thanks,
David
-----

On 8/02/2016 11:24 PM, Dmitry Dmitriev wrote:
> Hello David,
>
> Looks good to me. Just few additional clean-ups which you can consider
> for implementing.
> 1) hotspot/src/os/bsd/vm/os_bsd.cpp module
>
> I think that you can remove duplicated logic for mutex unlock from
> Parker::unpark function.
>
> I.e. transform it from:
> 4376 void Parker::unpark() {
> ...
> 4381   if (s < 1) {
> 4382     status = pthread_mutex_unlock(_mutex);
> 4383     assert_status(status == 0, status, "invariant");
> 4384     status = pthread_cond_signal(_cond);
> 4385     assert_status(status == 0, status, "invariant");
> 4386   } else {
> 4387     pthread_mutex_unlock(_mutex);
> 4388     assert_status(status == 0, status, "invariant");
> 4389   }
> 4390 }
>
> To something like that:
> void Parker::unpark() {
> ...
>    status = pthread_mutex_unlock(_mutex);
>    assert_status(status == 0, status, "invariant");
>    if (s < 1) {
>      status = pthread_cond_signal(_cond);
>      assert_status(status == 0, status, "invariant");
>    }
>   }
>
> 2) hotspot/src/os/linux/vm/os_linux.cpp
>
> Similar refactoring for Parker::unpark for removing duplicated logic for
> mutex unlock.
>
> From:
> 5720 void Parker::unpark() {
> ...
> 5725   if (s < 1) {
> 5726     // thread might be parked
> 5727     if (_cur_index != -1) {
> 5728       // thread is definitely parked
> 5729       // must capture correct index before unlocking
> 5730       int index = _cur_index;
> 5731       status = pthread_mutex_unlock(_mutex);
> 5732       assert(status == 0, "invariant");
> 5733       status = pthread_cond_signal(&_cond[index]);
> 5734       assert(status == 0, "invariant");
> 5735     } else {
> 5736       pthread_mutex_unlock(_mutex);
> 5737       assert(status == 0, "invariant");
> 5738     }
> 5739   } else {
> 5740     pthread_mutex_unlock(_mutex);
> 5741     assert(status == 0, "invariant");
> 5742   }
> 5743 }
>
> To:
>
> void Parker::unpark() {
> ...
>    if ((s < 1) && (_cur_index != -1)) {
>        // thread is definitely parked
>        // must capture correct index before unlocking
>        int index = _cur_index;
>        status = pthread_mutex_unlock(_mutex);
>        assert(status == 0, "invariant");
>        status = pthread_cond_signal(&_cond[index]);
>        assert(status == 0, "invariant");
>     } else {
>       pthread_mutex_unlock(_mutex);
>       assert(status == 0, "invariant");
>     }
> }
>
>
> Thanks,
> Dmitry
>
> On 08.02.2016 10:16, David Holmes wrote:
>> webrev: http://cr.openjdk.java.net/~dholmes/8145725/webrev/
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8145725
>>
>> In JDK-8130728 we disabled the workaround by default and have not seen
>> any issues (other than JDK-8029453). So now the workaround can be
>> removed completely.
>>
>> Thanks,
>> David
>