RFR: ShenandoahControlThread should not get GCLABs
Roman Kennke
rkennke at redhat.com
Fri Aug 31 14:27:40 UTC 2018
Very good. Patch seems ok.
It will prevent the StringDedupThread from getting a GCLAB. Would that
be an issue? We can infact exclude that from the make_parsable() stuff
now I think.
More generally, we need to ensure that make_parsable() and init-gclabs
(and even init-tlab) stuff agree about the set of threads that are
handled in both cases (maybe by a common assert in both paths). Anything
that is not covered by make_parsable() must never be allowed to get a
GCLAB or TLAB. That may be a useful follow-up.
Roman
> This seems to be the bug that we are chasing now in sh/jdk. ShenandoahControlThread accidentally got
> the GCLAB, but never retired it. When traversal-precleaning started, it walked some unmarked
> objects, evacuating them, which happened in the ShControlThread context. Which means we have
> allocated something in GCLAB we don't otherwise know about. And the whole thing crashes if we get to
> recycle the region holding that GCLAB before it retires.
>
> There are two possible fixes: let GCLAB retirement code know about ShControlThread, or make
> ShControlThread go via shared allocs all the time, like other non-Java threads do. I think the
> second option is better, because doing the GCLAB retirement from the ShControlThread might be prone
> to other errors (e.g. deadlocks). We intended that only Java threads and GC worker threads have
> GCLABs to simplify GCLAB tracking, but this is not true it current code, see below.
>
> (Separately, we shall fix traversal-precleaning to get to worker threads so it can use GCLABs there).
>
> Fix:
>
> diff -r ea2f39b37e25 src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Fri Aug 31 13:29:24 2018 +0200
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Fri Aug 31 15:49:05 2018 +0200
> @@ -564,12 +564,11 @@
> }
>
> class ShenandoahInitGCLABClosure : public ThreadClosure {
> public:
> void do_thread(Thread* thread) {
> - if (thread != NULL && (thread->is_Java_thread() || thread->is_Worker_thread() ||
> - thread->is_ConcurrentGC_thread())) {
> + if (thread != NULL && (thread->is_Java_thread() || thread->is_Worker_thread())) {
> ShenandoahThreadLocalData::initialize_gclab(thread);
> }
> }
> };
>
> diff -r ea2f39b37e25 src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp Fri Aug 31 13:29:24 2018 +0200
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp Fri Aug 31 15:49:05 2018 +0200
> @@ -120,10 +120,11 @@
> static bool is_force_satb_flush(Thread* thread) {
> return data(thread)->_force_satb_flush;
> }
>
> static void initialize_gclab(Thread* thread) {
> + assert (thread->is_Java_thread() || thread->is_Worker_thread(), "Only Java and GC worker
> threads are allowed to get GCLABs");
> data(thread)->_gclab = new PLAB(PLAB::min_size());
> data(thread)->_gclab_size = 0;
> }
>
> static PLAB* gclab(Thread* thread) {
>
> Testing: failing tests on my desktop and gotland, tier3_gc_shenandoah (running)
>
> Thanks,
> -Aleksey
>
More information about the shenandoah-dev
mailing list