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