Skip to content

Commit

Permalink
[mono][interp] Fix concurrency issues with publishing transfomed imet…
Browse files Browse the repository at this point in the history
…hod (#87555)

* [mono][utils] Move all membar code to single file

* [mono][utils] Redefine write and read memory barriers

Before this change they were doing a full memory barrier, regardless of architecture. We have a beginning of more precise implementation via the *_FENCE defines. Implement mono_memory_write_barrier and mono_memory_read_barrier reusing these defines instead.

The only consequence of this change is that, on x86 and amd64, `mono_memory_write_barrier` and `mono_memory_read_barrier` become a compiler barrier instead of a full mfence.

* [mono][interp] Fix concurrency issues with publishing transfomed imethod

When publishing a transformed InterpMethod*, we first set all relevant fields (like `code`, `alloca_size` etc), we execute a write barrier and finally we set the `transformed` flag. On relaxed memory arches we need to have a read barrier on the consumer, since there is no data dependency between `transformed` and the other fields of `InterpMethod`.

On arm this change does a full barrier (we could get away with just a load acquire but we haven't yet added support for emitting this in the runtime). Still, this doesn't seem to introduce a heavy perf penalty (on my arm64 M1) but we can revisit if necessary. On x86/amd64 this is a compiler barrier so it should have no impact. WASM is single threaded for now.
  • Loading branch information
BrzVlad authored Jun 16, 2023
1 parent 4ae1668 commit 2f737e4
Show file tree
Hide file tree
Showing 27 changed files with 79 additions and 115 deletions.
1 change: 0 additions & 1 deletion src/mono/mono/metadata/domain.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <mono/utils/atomic.h>
#include <mono/utils/mono-compiler.h>
#include <mono/utils/mono-logger-internals.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-counters.h>
#include <mono/utils/hazard-pointer.h>
#include <mono/utils/mono-tls.h>
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/metadata/icall.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#endif

#include "mono/metadata/icall-internals.h"
#include "mono/utils/mono-membar.h"
#include <mono/metadata/object.h>
#include <mono/metadata/threads.h>
#include <mono/metadata/threads-types.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/jit-info.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <mono/utils/atomic.h>
#include <mono/utils/mono-compiler.h>
#include <mono/utils/mono-logger-internals.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/hazard-pointer.h>
#include <mono/utils/mono-tls.h>
#include <mono/utils/mono-mmap.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
#include <mono/metadata/jit-info.h>
#include <mono/utils/mono-logger-internals.h>
#include <mono/utils/mono-dl.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-counters.h>
#include <mono/utils/mono-error-internals.h>
#include <mono/utils/mono-tls.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/property-bag.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/
#include <mono/metadata/property-bag.h>
#include <mono/utils/atomic.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>

/*
* mono_property_bag_get:
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/reflection.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* Licensed under the MIT license. See LICENSE file in the project root for full license information.
*/
#include <config.h>
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include "mono/metadata/assembly-internals.h"
#include "mono/metadata/reflection-internals.h"
#include "mono/metadata/tabledefs.h"
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/string-icalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include <mono/metadata/string-icalls.h>
#include <mono/metadata/class-internals.h>
#include <mono/metadata/appdomain.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/threads-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <mono/metadata/object.h>
#include "mono/metadata/handle.h"
#include "mono/utils/mono-compiler.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include "mono/utils/mono-threads.h"
#include "mono/metadata/class-internals.h"
#include <mono/metadata/icalls.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#include <mono/utils/monobitset.h>
#include <mono/utils/mono-compiler.h>
#include <mono/utils/mono-mmap.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-time.h>
#include <mono/utils/mono-threads.h>
#include <mono/utils/mono-threads-coop.h>
Expand Down
4 changes: 3 additions & 1 deletion src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include <mono/utils/mono-logger-internals.h>
#include <mono/utils/mono-tls-inline.h>
#include <mono/utils/mono-threads.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>

#ifdef HAVE_ALLOCA_H
# include <alloca.h>
Expand Down Expand Up @@ -3606,6 +3606,8 @@ method_entry (ThreadContext *context, InterpFrame *frame,
frame->stack = (stackval*)context->stack_pointer;
return slow;
}
} else {
mono_memory_read_barrier ();
}

return slow;
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/mini/mini-trampolines.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <mono/metadata/tabledefs.h>
#include <mono/utils/mono-counters.h>
#include <mono/utils/mono-error-internals.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-compiler.h>
#include <mono/utils/mono-threads-coop.h>
#include <mono/utils/unlocked.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/profiler/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include <mono/utils/mono-coop-mutex.h>
#include <mono/utils/mono-logger-internals.h>
#include <mono/utils/mono-linked-list-set.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-mmap.h>
#include <mono/utils/mono-os-mutex.h>
#include <mono/utils/mono-os-semaphore.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/sgen/sgen-fin-weak-hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "mono/sgen/sgen-pointer-queue.h"
#include "mono/sgen/sgen-client.h"
#include "mono/sgen/gc-internal-agnostic.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include "mono/utils/atomic.h"
#include "mono/utils/unlocked.h"

Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/sgen/sgen-gchandles.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "mono/sgen/sgen-gc.h"
#include "mono/sgen/sgen-client.h"
#include "mono/sgen/sgen-array-list.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"

#ifdef HEAVY_STATISTICS
static volatile guint32 stat_gc_handles_allocated = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/sgen/sgen-nursery-allocator.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
#include "mono/sgen/sgen-memory-governor.h"
#include "mono/sgen/sgen-pinning.h"
#include "mono/sgen/sgen-client.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"

/* Enable it so nursery allocation diagnostic data is collected */
//#define NALLOC_DEBUG 1
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/sgen/sgen-protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "sgen-memory-governor.h"
#include "sgen-workers.h"
#include "sgen-client.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include "mono/utils/mono-proclib.h"

#include <errno.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/sgen/sgen-workers.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "mono/sgen/sgen-gc.h"
#include "mono/sgen/sgen-workers.h"
#include "mono/sgen/sgen-thread-pool.h"
#include "mono/utils/mono-membar.h"
#include "mono/utils/mono-memory-model.h"
#include "mono/sgen/sgen-client.h"

#ifndef DISABLE_SGEN_MAJOR_MARKSWEEP_CONC
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/utils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ set(utils_common_sources
mono-forward-internal.h
mono-machine.h
mono-math.h
mono-membar.h
mono-path.h
mono-poll.h
mono-uri.h
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/atomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

#include "config.h"
#include <glib.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-compiler.h>

/*
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/utils/hazard-pointer.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <string.h>

#include <mono/utils/hazard-pointer.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/monobitset.h>
#include <mono/utils/lock-free-array-queue.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/hazard-pointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include <glib.h>
#include <mono/utils/mono-compiler.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/mono-publib.h>

#define HAZARD_POINTER_COUNT 3
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/lock-free-alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
#else
#include <mono/utils/mono-mmap.h>
#endif
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/hazard-pointer.h>
#include <mono/utils/lock-free-queue.h>

Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/lock-free-array-queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include <string.h>

#include <mono/utils/atomic.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#ifdef SGEN_WITHOUT_MONO
#include <mono/sgen/sgen-gc.h>
#include <mono/sgen/sgen-client.h>
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/lock-free-queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
#include <unistd.h>
#endif

#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>
#include <mono/utils/hazard-pointer.h>
#include <mono/utils/atomic.h>

Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/utils/mono-linked-list-set.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#define __MONO_SPLIT_ORDERED_LIST_H__

#include <mono/utils/hazard-pointer.h>
#include <mono/utils/mono-membar.h>
#include <mono/utils/mono-memory-model.h>

typedef struct _MonoLinkedListSetNode MonoLinkedListSetNode;

Expand Down
83 changes: 0 additions & 83 deletions src/mono/mono/utils/mono-membar.h

This file was deleted.

Loading

0 comments on commit 2f737e4

Please sign in to comment.