RFR: 8358821: patch_verified_entry causes problems, use nmethod entry barriers instead [v4]

[Dean Long]

On Mon, 23 Jun 2025 12:47:07 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> I think making it less slippery in one place but still leaving other races gives a false sense of security and makes the code harder to understand.  Arming the barrier is not guaranteed to be visible until there is a safepoint.  Note that AArch64 and RISCV only call increment_patching_epoch() when the guard value is set to the disarmed value, so there is no invalidation of the CPU pipeline or instruction buffer (cross modification fence) when arming.
>
> Okay. I would have preferred to not enter the nmethod when we evaluate the guard bits under the lock that protects it and see that it's supposed to be not entrant. But I won't argue for it further if you prefer not to change that. Other than that, I think this looks good.

I think it's OK if there is a race to have a point of no return, and if one thread gets there first, it wins, and we don't need to check again.  It's tempting to want to do an extra check when we disarm under the lock, but then it would need a comment explaining why we do it, even though the make_not_entrant could come in right after and we would miss it.  And we have already done the work of healing the oops by this point.  Finally, I like the encapsulation that only nmethod_stub_entry_barrier needs to know about not_entrant, and nmethod_entry_barrier doesn't need to know.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25764#discussion_r2162359865