RFR: 8331911: Reconsider locking for recently disarmed nmethods [v3]

[Erik Österlund]

On Wed, 29 May 2024 17:14:31 GMT, Neethu Prasad <duke at openjdk.org> wrote:

> > It seems fine to me that the GC backends are responsible for checking if the nmethod is disarmed outside the lock. However, we have some callers that now check it redundantly. I think those callers should stop doing that now. Otherwise, this looks good to me.
> 
> 
> 
> Thanks for the feedback! Looking around the code, I think there are a few places where we can do more changes.
> 
> 
> 
> First, remove check here:
> 
> https://github.com/openjdk/jdk/blob/bc7d9e3d0bc663bbbeb068889082da4a9f0fa8de/src/hotspot/share/code/nmethod.cpp#L852-L855
> 
> 
> 
> This would force us to add the check in super-class implementation here:
> 
> https://github.com/openjdk/jdk/blob/bc7d9e3d0bc663bbbeb068889082da4a9f0fa8de/src/hotspot/share/gc/shared/barrierSetNMethod.cpp#L83
> 
> 
> 
> Second, we can remove the check here:
> 
> https://github.com/openjdk/jdk/blob/bc7d9e3d0bc663bbbeb068889082da4a9f0fa8de/src/hotspot/share/gc/shared/barrierSetNMethod.cpp#L175-L181
> 
> 
> 
> But it does not seem straightforward, because we currently skip cross-modification fence based on is_armed(...) check. Unfortunately, we cannot easily know if nmethod_entry_barrier acted or not, we only know if method is safe or not.  Can we / should we do these refactoring separately?

I see your point. However, this PR is refactoring the code to iron out who is responsible for checking is_armed, so I would prefer if we got that right in this PR. We say it should be the backend code doing that, so the callers shouldn't. I agree with all the changes you just listed and if you make them I would be happy.

Regarding the cross modifying fence, I strongly prefer to not try and be clever. Just run the cross modifying fence unconditionally after calling the backend code. We get there because the barrier was armed anyway.

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

PR Comment: https://git.openjdk.org/jdk/pull/19285#issuecomment-2140077263