From 37ec22b569fceb88b0f6ffa6ff2acc027dae904c Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Fri, 13 Dec 2024 17:12:02 -0800 Subject: Document and fix thread flags memory ordering Expand mutator_gc_coord.md with a discussion of memory ordering for state and flags accesses. Change ReadFlag() to take a non-defaulted memory order argument. Fix all calls to supply it. The prior memory_order_relaxed default does not make sense, since it is incorrect in a few places. And even when it is correct, the arguments behind that are extremely subtle and vary by call site. We want this to be very explicit and documented. Document ThreadList::RunCheckpoint memory ordering requirements. Document the fact that empty checkpoinst should also satisfy those requirements, but currently do not fully do so. Temporarily strengthen memory ordering to test kEmptyCheckpointRequest to partially work around the above issue. Strengthen some of the loads to test kRunningFlipFunction. We did not always perform an acquire load before concluding that the flip function had completed. Weaken one load in EnsureFlipFunctionStarted for consistency. In a corner case, reload a flag with acquire ordering in the same function. This is the result of a fairly complete pass over the flag setting and reading logic. The empty checkpoint issue still needs to be more completley addressed, most probably by switching empty checkpoints to use the general checkpoint logic. It currently uses an "optimization" that is very difficult, and probably expensive, to fully correct. It is not clear whether we have observed bugs caused by this in the field. Bug: 382722942 Bug: 380159893 Test: Build and boot aosp Test: Checked .md file with lowdown Change-Id: I793eac7ddf80fada36bb4f16ddfaaf317829caa3 --- compiler/jni/jni_compiler_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'compiler/jni/jni_compiler_test.cc') diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc index 35ee1edafd..1a408bb724 100644 --- a/compiler/jni/jni_compiler_test.cc +++ b/compiler/jni/jni_compiler_test.cc @@ -912,14 +912,14 @@ void JniCompilerTest::CompileAndRun_fooJJ_synchronizedImpl() { gLogVerbosity.systrace_lock_logging = true; InitEntryPoints(&self->tlsPtr_.jni_entrypoints, &self->tlsPtr_.quick_entrypoints, - self->ReadFlag(ThreadFlag::kMonitorJniEntryExit)); + self->ReadFlag(ThreadFlag::kMonitorJniEntryExit, std::memory_order_relaxed)); result = env_->CallNonvirtualLongMethod(jobj_, jklass_, jmethod_, a, b); EXPECT_EQ(a | b, result); EXPECT_EQ(5, gJava_MyClassNatives_fooJJ_synchronized_calls[gCurrentJni]); gLogVerbosity.systrace_lock_logging = false; InitEntryPoints(&self->tlsPtr_.jni_entrypoints, &self->tlsPtr_.quick_entrypoints, - self->ReadFlag(ThreadFlag::kMonitorJniEntryExit)); + self->ReadFlag(ThreadFlag::kMonitorJniEntryExit, std::memory_order_relaxed)); gJava_MyClassNatives_fooJJ_synchronized_calls[gCurrentJni] = 0; } -- cgit v1.2.3-59-g8ed1b