RFR: 8310239: Add missing cross modifying fence in nmethod entry barriers [v2]

Erik Österlund eosterlund at openjdk.org
Tue Jun 20 08:26:08 UTC 2023


> In fact, there is a current race in the nmethod entry barriers, where what we are doing violates the AMD APM (cf. APM volume 2 section 7.6.1 https://www.amd.com/system/files/TechDocs/24593.pdf). 
> In particular, if the compare instruction of the nmethod entry barrier is not yet patched and we call a slow path on thread 1, then before taking the nmethod entry lock, another thread 2 could fix and disarm the nmethod. Then thread 1 will observe *data* suggesting the nmethod has been patched, but never re-executes the patched compare (which might indeed still be stale), hence not qualifying for asynchronous cross modifying code, and neither do we run a serializing cpuid instruction, qualifying for synchronous cross modifying code. In this scenario, we can indeed start executing the nmethod instructions, while observing inconsistent concurrent patching effects, where some instructions will be updated and some not.
> 
> The following patch ensures that x86 nmethod entry barriers execute cross modifying fence after calling into the VM, where another thread could have disarmed the nmethod. I also ensured the other platforms perform their fencing after the VM call, instead of before - including a cross_modify_fence in the shared code for OSR nmethod entries. While fencing before will flush out the instruction pipeline, and it shouldn't be populated with problematic instructions until after we start executing the nmethod again, it feels unnecessary to fence on the wrong side of the modifications it wishes to guard, and hence not strictly following the synchronous cross modifying fence recipe.
> 
> I'm currently running tier1-5 and running performance testing in aurora. In the interest of time, I'm opening this PR before getting the final result, and will report the results when they come in.

Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:

  Typo in comment

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14543/files
  - new: https://git.openjdk.org/jdk/pull/14543/files/ff69b45d..329625d9

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14543&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14543&range=00-01

  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/14543.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14543/head:pull/14543

PR: https://git.openjdk.org/jdk/pull/14543


More information about the hotspot-dev mailing list