RFR: 8310239: Add missing cross modifying fence in nmethod entry barriers [v3]
Erik Österlund
eosterlund at openjdk.org
Fri Oct 20 16:03: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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
- Move cross modify fence to the runtime code
- Merge branch 'master' into 8310239_cross_modify_nmethod_entry
- Typo in comment
- 8310239: Add missing cross modifying fence in nmethod entry barriers
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/14543/files
- new: https://git.openjdk.org/jdk/pull/14543/files/329625d9..034c94be
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14543&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14543&range=01-02
Stats: 327788 lines in 6892 files changed: 147438 ins; 113457 del; 66893 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