RFR: 8229020: Failure on CPUs allowing loads reordering: assert(_tasks[t] == 1) failed: What else?
Thomas Schatzl
thomas.schatzl at oracle.com
Fri Aug 2 15:39:54 UTC 2019
Hi Jie,
On 02.08.19 02:44, Jie Fu wrote:
> Hi all,
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8229020
> Webrev: http://cr.openjdk.java.net/~jiefu/8229020/webrev.00/
>
> *Background*
> The failure was first observed on our Loongson CPUs which allow loads
> reordering with the following test
> ---------------------------------------------------------
> make test
> TEST="compiler/codecache/stress/UnexpectedDeoptimizationTest.java"
> CONF=fastdebug
> ---------------------------------------------------------
>
> *Analysis*
> The failure was caused by the loads reordering on CPUs with weak memory
> consistency.
> Just imagine the following case:
> - If the load of _tasks[t] in line 436 is floating up before the load
> of _tasks[t] in line 432, and
> - the load in line 436 may read 0 and the load in line 432 may read 1,
> - then the if-condition in line 433 is false, so Atomic::cmpxchg
> won't be executed,
> - then the assert in line 436 fails.
> ---------------------------------------------------------
> 429
> 430 bool SubTasksDone::try_claim_task(uint t) {
> 431 assert(t < _n_tasks, "bad task id.");
> 432 uint old = _tasks[t];
> 433 if (old == 0) {
> 434 old = Atomic::cmpxchg(1u, &_tasks[t], 0u);
> 435 }
> 436 assert(_tasks[t] == 1, "What else?");
> 437 bool res = old == 0;
> 438 #ifdef ASSERT
> 439 if (res) {
> 440 assert(_claimed < _n_tasks, "Too many tasks claimed; missing
> clear?");
> 441 Atomic::inc(&_claimed);
> 442 }
> 443 #endif
> 444 return res;
> 445 }
> 446
> ---------------------------------------------------------
>
> *Fix*
> It would be better to insert a memory fence before line 436 to prevent
> the load from floating up.
>
> Could you please review it?
remove the assert. It's old paranoid code adding no further
information imho.
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list