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

[Dean Long]

On Mon, 16 Jun 2025 00:54:48 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   remove is_sigill_not_entrant
>
> src/hotspot/share/gc/z/zBarrierSetNMethod.cpp line 114:
> 
>> 112:   // Preserve the sticky bit
>> 113:   if (is_not_entrant(nm)) {
>> 114:     value |= not_entrant;
> 
> Is it possible to have a race where another thread sets an nmethod to not entrant and the thread calling this making the nmethod entry barrier not entrant?
> 
> If this was called to disarm a method and then enter it, it seems a bit sneaky in that case that we pass the nmethod entry barrier even though we under the lock see that it is not entrant. Probably okay but still feels like it might be more robust if the thread setting an nmethod to not entrant is always the one that arms the nmethod entry barrier.

If I understand your concern correctly, there is no race.  The only caller of BarrierSetNMethod::make_not_entrant() is nmethod::make_not_entrant(), and it is done inside a NMethodState_lock critical section.  After a call to nmethod::make_not_entrant(), the nmethod entry barrier is armed and stays that way.
And by design, a disarm only disarms at the inner nmethod_entry_barrier level, not the outer nmethod_stub_entry_barrier level.

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

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