From a6b7d6f88815930102ce9f9e5a76211348abb1f6 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 1 Aug 2019 21:48:23 +0300 Subject: [PATCH] [sgen] Fix check-nursery-pinned debug option (mono/mono#15865) * [sgen] Remove dead unpin_queue argument * [sgen] Fix check-nursery-pinned debug option And rename it to better reflect what is is doing now (which is to check that we don't leave any vtables tagged at the end of a collection). It used to also check that nursery objects are pinned, back when we were doing a nursery collection as part of the finishing pause of a concurrent collection. Commit migrated from https://github.com/mono/mono/commit/87ba12e13f98a24eb140e933876a25b45fd806b8 --- src/mono/mono/sgen/sgen-debug.c | 15 +++++---------- src/mono/mono/sgen/sgen-gc.c | 30 ++++++++++++++--------------- src/mono/mono/sgen/sgen-gc.h | 4 ++-- src/mono/mono/sgen/sgen-nursery-allocator.c | 7 ++----- 4 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/mono/mono/sgen/sgen-debug.c b/src/mono/mono/sgen/sgen-debug.c index 9dbe89d..39a5ba9 100644 --- a/src/mono/mono/sgen/sgen-debug.c +++ b/src/mono/mono/sgen/sgen-debug.c @@ -597,23 +597,18 @@ sgen_check_heap_marked (gboolean nursery_must_be_pinned) } static void -check_nursery_objects_pinned_callback (char *obj, size_t size, void *data /* ScanCopyContext *ctx */) +check_nursery_objects_untag_callback (char *obj, size_t size, void *data) { - gboolean pinned = (gboolean) (size_t) data; - g_assert (!SGEN_OBJECT_IS_FORWARDED (obj)); - if (pinned) - g_assert (SGEN_OBJECT_IS_PINNED (obj)); - else - g_assert (!SGEN_OBJECT_IS_PINNED (obj)); + g_assert (!SGEN_OBJECT_IS_PINNED (obj)); } void -sgen_check_nursery_objects_pinned (gboolean pinned) +sgen_check_nursery_objects_untag (void) { sgen_clear_nursery_fragments (); sgen_scan_area_with_callback (sgen_nursery_section->data, sgen_nursery_section->end_data, - (IterateObjectCallbackFunc)check_nursery_objects_pinned_callback, (void*) (size_t) pinned /* (void*)&ctx */, FALSE, TRUE); + (IterateObjectCallbackFunc)check_nursery_objects_untag_callback, NULL, FALSE, TRUE); } static void @@ -1249,7 +1244,7 @@ sgen_check_mod_union_consistency (void) } void -sgen_check_nursery_objects_pinned (gboolean pinned) +sgen_check_nursery_objects_untag (void) { } diff --git a/src/mono/mono/sgen/sgen-gc.c b/src/mono/mono/sgen/sgen-gc.c index b0df131..1572800 100644 --- a/src/mono/mono/sgen/sgen-gc.c +++ b/src/mono/mono/sgen/sgen-gc.c @@ -223,8 +223,8 @@ static gboolean remset_consistency_checks = FALSE; static gboolean mod_union_consistency_check = FALSE; /* If set, check whether mark bits are consistent after major collections */ static gboolean check_mark_bits_after_major_collection = FALSE; -/* If set, check that all nursery objects are pinned/not pinned, depending on context */ -static gboolean check_nursery_objects_pinned = FALSE; +/* If set, check that all vtables of nursery objects are untagged */ +static gboolean check_nursery_objects_untag = FALSE; /* If set, do a few checks when the concurrent collector is used */ static gboolean do_concurrent_checks = FALSE; /* If set, do a plausibility check on the scan_starts before and after @@ -1677,7 +1677,7 @@ enqueue_scan_from_roots_jobs (SgenGrayQueue *gc_thread_gray_queue, char *heap_st * Return whether any objects were late-pinned due to being out of memory. */ static gboolean -collect_nursery (const char *reason, gboolean is_overflow, SgenGrayQueue *unpin_queue) +collect_nursery (const char *reason, gboolean is_overflow) { gboolean needs_major, is_parallel = FALSE; mword fragment_total; @@ -1844,7 +1844,7 @@ collect_nursery (const char *reason, gboolean is_overflow, SgenGrayQueue *unpin_ * next allocations. */ sgen_client_binary_protocol_reclaim_start (GENERATION_NURSERY); - fragment_total = sgen_build_nursery_fragments (sgen_nursery_section, unpin_queue); + fragment_total = sgen_build_nursery_fragments (sgen_nursery_section); if (!fragment_total) sgen_degraded_mode = 1; @@ -1899,8 +1899,8 @@ collect_nursery (const char *reason, gboolean is_overflow, SgenGrayQueue *unpin_ sgen_binary_protocol_collection_end (mono_atomic_load_i32 (&mono_gc_stats.minor_gc_count) - 1, GENERATION_NURSERY, 0, 0); - if (check_nursery_objects_pinned && !sgen_minor_collector.is_split) - sgen_check_nursery_objects_pinned (unpin_queue != NULL); + if (check_nursery_objects_untag) + sgen_check_nursery_objects_untag (); return needs_major; } @@ -2020,8 +2020,6 @@ major_copy_or_mark_from_roots (SgenGrayQueue *gc_thread_gray_queue, size_t *old_ } pin_objects_in_nursery (mode == COPY_OR_MARK_FROM_ROOTS_START_CONCURRENT, ctx); - if (check_nursery_objects_pinned && !sgen_minor_collector.is_split) - sgen_check_nursery_objects_pinned (mode != COPY_OR_MARK_FROM_ROOTS_START_CONCURRENT); sgen_major_collector.pin_objects (gc_thread_gray_queue); if (old_next_pin_slot) @@ -2257,13 +2255,15 @@ major_finish_collection (SgenGrayQueue *gc_thread_gray_queue, const char *reason * pinned objects as we go, memzero() the empty fragments so they are ready for the * next allocations. */ - fragment_total = sgen_build_nursery_fragments (sgen_nursery_section, NULL); + fragment_total = sgen_build_nursery_fragments (sgen_nursery_section); if (!fragment_total) sgen_degraded_mode = 1; SGEN_LOG (4, "Free space in nursery after major %ld", (long)fragment_total); if (do_concurrent_checks && sgen_concurrent_collection_in_progress) sgen_debug_check_nursery_is_clean (); + if (check_nursery_objects_untag) + sgen_check_nursery_objects_untag (); /* prepare the pin queue for the next collection */ sgen_finish_pinning (); @@ -2541,7 +2541,7 @@ sgen_perform_collection_inner (size_t requested_size, int generation_to_collect, if (sgen_concurrent_collection_in_progress) major_update_concurrent_collection (); - if (collect_nursery (reason, FALSE, NULL) && !sgen_concurrent_collection_in_progress) { + if (collect_nursery (reason, FALSE) && !sgen_concurrent_collection_in_progress) { overflow_generation_to_collect = GENERATION_OLD; overflow_reason = "Minor overflow"; } @@ -2553,7 +2553,7 @@ sgen_perform_collection_inner (size_t requested_size, int generation_to_collect, } else { SGEN_ASSERT (0, generation_to_collect == GENERATION_OLD, "We should have handled nursery collections above"); if (sgen_major_collector.is_concurrent && !forced_serial) { - collect_nursery ("Concurrent start", FALSE, NULL); + collect_nursery ("Concurrent start", FALSE); major_start_concurrent_collection (reason); oldest_generation_collected = GENERATION_NURSERY; } else if (major_do_collection (reason, FALSE, forced_serial)) { @@ -2571,7 +2571,7 @@ sgen_perform_collection_inner (size_t requested_size, int generation_to_collect, */ if (overflow_generation_to_collect == GENERATION_NURSERY) - collect_nursery (overflow_reason, TRUE, NULL); + collect_nursery (overflow_reason, TRUE); else major_do_collection (overflow_reason, TRUE, forced_serial); @@ -3647,8 +3647,8 @@ sgen_gc_init (void) mod_union_consistency_check = TRUE; } else if (!strcmp (opt, "check-mark-bits")) { check_mark_bits_after_major_collection = TRUE; - } else if (!strcmp (opt, "check-nursery-pinned")) { - check_nursery_objects_pinned = TRUE; + } else if (!strcmp (opt, "check-nursery-untag")) { + check_nursery_objects_untag = TRUE; } else if (!strcmp (opt, "clear-at-gc")) { sgen_nursery_clear_policy = CLEAR_AT_GC; } else if (!strcmp (opt, "clear-nursery-at-gc")) { @@ -3706,7 +3706,7 @@ sgen_gc_init (void) fprintf (stderr, " max-valloc-size=N (where N is an integer, possibly with a k, m or a g suffix)\n"); fprintf (stderr, " check-remset-consistency\n"); fprintf (stderr, " check-mark-bits\n"); - fprintf (stderr, " check-nursery-pinned\n"); + fprintf (stderr, " check-nursery-untag\n"); fprintf (stderr, " verify-before-collections\n"); fprintf (stderr, " verify-nursery-at-minor-gc\n"); fprintf (stderr, " dump-nursery-at-minor-gc\n"); diff --git a/src/mono/mono/sgen/sgen-gc.h b/src/mono/mono/sgen/sgen-gc.h index af86ae6..f9fbb92 100644 --- a/src/mono/mono/sgen/sgen-gc.h +++ b/src/mono/mono/sgen/sgen-gc.h @@ -932,7 +932,7 @@ void sgen_clear_nursery_fragments (void); void sgen_nursery_allocator_prepare_for_pinning (void); void sgen_nursery_allocator_set_nursery_bounds (char *nursery_start, size_t min_size, size_t max_size); void sgen_resize_nursery (gboolean need_shrink); -mword sgen_build_nursery_fragments (GCMemSection *nursery_section, SgenGrayQueue *unpin_queue); +mword sgen_build_nursery_fragments (GCMemSection *nursery_section); void sgen_init_nursery_allocator (void); void sgen_nursery_allocator_init_heavy_stats (void); void sgen_init_allocator (void); @@ -1093,7 +1093,7 @@ void sgen_check_whole_heap_stw (void) MONO_PERMIT (need (sgen_gc_locked, sgen_stop_world)); void sgen_check_objref (char *obj); void sgen_check_heap_marked (gboolean nursery_must_be_pinned); -void sgen_check_nursery_objects_pinned (gboolean pinned); +void sgen_check_nursery_objects_untag (void); void sgen_check_for_xdomain_refs (void); GCObject* sgen_find_object_for_ptr (char *ptr); diff --git a/src/mono/mono/sgen/sgen-nursery-allocator.c b/src/mono/mono/sgen/sgen-nursery-allocator.c index 0a8ee12..da817e4 100644 --- a/src/mono/mono/sgen/sgen-nursery-allocator.c +++ b/src/mono/mono/sgen/sgen-nursery-allocator.c @@ -711,7 +711,7 @@ add_nursery_frag_checks (SgenFragmentAllocator *allocator, char *frag_start, cha } mword -sgen_build_nursery_fragments (GCMemSection *nursery_section, SgenGrayQueue *unpin_queue) +sgen_build_nursery_fragments (GCMemSection *nursery_section) { char *frag_start, *frag_end; size_t frag_size; @@ -747,10 +747,7 @@ sgen_build_nursery_fragments (GCMemSection *nursery_section, SgenGrayQueue *unpi addr1 = frags_ranges->fragment_start; if (addr0 < addr1) { - if (unpin_queue) - GRAY_OBJECT_ENQUEUE_SERIAL (unpin_queue, (GCObject*)addr0, sgen_obj_get_descriptor_safe ((GCObject*)addr0)); - else - SGEN_UNPIN_OBJECT (addr0); + SGEN_UNPIN_OBJECT (addr0); size = SGEN_ALIGN_UP (sgen_safe_object_get_size ((GCObject*)addr0)); CANARIFY_SIZE (size); sgen_set_nursery_scan_start (addr0); -- 2.7.4