From: Martin Quinson Date: Thu, 28 Jul 2022 23:51:52 +0000 (+0200) Subject: Remove the need of pthread_mutex in mmalloc, to allow its use with sthread X-Git-Tag: v3.32~129 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/872cf95ab5a2b08aa1f2c6ebba29b9f86b0ba54e Remove the need of pthread_mutex in mmalloc, to allow its use with sthread Historically, mmalloc was used in all case when MC was activated at build time (even if MC was not involved at runtime), forcing this protection against race conditions for e.g. parallel simulations. But nowadays, mmalloc is only activated at runtime in the verified application. In other runtime setups, mmalloc uses the libc's malloc implem instead of itself (that's why we use dlsym in the initialization of mmalloc, to find the replacement). So, removing this code should have no impact whatsoever. Only maybe speeding up the verification a tiny bit. And now, sthread becomes usable in conjunction with Mc Simgrid!!! Given the current poor coverage of the pthread API, we can't verify any useful program, but at least we have a valid (and rather robust) prototype of such a verification tool. --- diff --git a/src/include/xbt/mmalloc.h b/src/include/xbt/mmalloc.h index 6a872fec87..bdff2c11a2 100644 --- a/src/include/xbt/mmalloc.h +++ b/src/include/xbt/mmalloc.h @@ -50,13 +50,14 @@ XBT_PUBLIC void mfree(xbt_mheap_t md, void* ptr); XBT_PUBLIC xbt_mheap_t xbt_mheap_new(void* baseaddr, int options); -XBT_PUBLIC void xbt_mheap_destroy_no_free(xbt_mheap_t md); - XBT_PUBLIC void* xbt_mheap_destroy(xbt_mheap_t md); /* To get the heap used when using the legacy version malloc/free/realloc and such */ xbt_mheap_t mmalloc_get_current_heap(void); +/* Returns true if we are using the internal mmalloc, and false if we are using the libc's malloc */ +XBT_PUBLIC int malloc_use_mmalloc(void); + #endif SG_END_DECL diff --git a/src/simgrid/sg_config.cpp b/src/simgrid/sg_config.cpp index e60efa33ba..10d160af21 100644 --- a/src/simgrid/sg_config.cpp +++ b/src/simgrid/sg_config.cpp @@ -10,6 +10,7 @@ #include #include "simgrid/sg_config.hpp" +#include "src/include/xbt/mmalloc.h" #include "src/instr/instr_private.hpp" #include "src/internal_config.h" #include "src/kernel/context/Context.hpp" @@ -327,9 +328,17 @@ void sg_config_init(int *argc, char **argv) static simgrid::config::Flag cfg_context_guard_size{ "contexts/guard-size", "Guard size for contexts stacks in memory pages", default_guard_size, [](int value) { simgrid::kernel::context::guard_size = value * xbt_pagesize; }}; - static simgrid::config::Flag cfg_context_nthreads{"contexts/nthreads", - "Number of parallel threads used to execute user contexts", 1, - &simgrid::kernel::context::set_nthreads}; + + static simgrid::config::Flag cfg_context_nthreads{ + "contexts/nthreads", "Number of parallel threads used to execute user contexts", 1, [](int nthreads) { +#if HAVE_MMALLOC + xbt_assert( + nthreads == 1 || !malloc_use_mmalloc(), + "Parallel simulation is forbidden in the verified program, as there is no protection against race " + "conditions in mmalloc itself. Please don't be so greedy and show some mercy for our implementation."); +#endif + simgrid::kernel::context::set_nthreads(nthreads); + }}; /* synchronization mode for parallel user contexts */ #if HAVE_FUTEX_H diff --git a/src/xbt/mmalloc/mfree.c b/src/xbt/mmalloc/mfree.c index 652b3813fa..c6c5a84571 100644 --- a/src/xbt/mmalloc/mfree.c +++ b/src/xbt/mmalloc/mfree.c @@ -42,13 +42,11 @@ void mfree(struct mdesc *mdp, void *ptr) switch (type) { case MMALLOC_TYPE_HEAPINFO: - UNLOCK(mdp); fprintf(stderr, "Asked to free a fragment in a heapinfo block. I'm confused.\n"); abort(); break; case MMALLOC_TYPE_FREE: /* Already free */ - UNLOCK(mdp); fprintf(stderr, "Asked to free a fragment in a block that is already free. I'm puzzled.\n"); abort(); break; @@ -172,7 +170,6 @@ void mfree(struct mdesc *mdp, void *ptr) frag_nb = RESIDUAL(ptr, BLOCKSIZE) >> type; if( mdp->heapinfo[block].busy_frag.frag_size[frag_nb] == -1){ - UNLOCK(mdp); fprintf(stderr, "Asked to free a fragment that is already free. I'm puzzled\n"); abort(); } diff --git a/src/xbt/mmalloc/mm_legacy.c b/src/xbt/mmalloc/mm_legacy.c index 7b946b57a4..ee822fdb26 100644 --- a/src/xbt/mmalloc/mm_legacy.c +++ b/src/xbt/mmalloc/mm_legacy.c @@ -160,10 +160,7 @@ void *malloc(size_t n) if (!mdp) return NULL; - LOCK(mdp); - void *ret = mmalloc(mdp, n); - UNLOCK(mdp); - return ret; + return mmalloc(mdp, n); } void *calloc(size_t nmemb, size_t size) @@ -182,9 +179,7 @@ void *calloc(size_t nmemb, size_t size) if (!mdp) return NULL; - LOCK(mdp); void *ret = mmalloc(mdp, nmemb*size); - UNLOCK(mdp); // This was already done in the callee: if(!(mdp->options & XBT_MHEAP_OPTION_MEMSET)) { memset(ret, 0, nmemb * size); @@ -208,10 +203,7 @@ void *realloc(void *p, size_t s) if (!mdp) return NULL; - LOCK(mdp); - void* ret = mrealloc(mdp, p, s); - UNLOCK(mdp); - return ret; + return mrealloc(mdp, p, s); } void free(void *p) @@ -231,7 +223,5 @@ void free(void *p) return; xbt_mheap_t mdp = GET_HEAP(); - LOCK(mdp); mfree(mdp, p); - UNLOCK(mdp); } diff --git a/src/xbt/mmalloc/mm_module.c b/src/xbt/mmalloc/mm_module.c index 56091d2aa2..05d59b1e01 100644 --- a/src/xbt/mmalloc/mm_module.c +++ b/src/xbt/mmalloc/mm_module.c @@ -68,7 +68,6 @@ xbt_mheap_t xbt_mheap_new(void* baseaddr, int options) mdp->next_mdesc = NULL; mdp->options = options; - pthread_mutex_init(&mdp->mutex, NULL); /* If we have not been passed a valid open file descriptor for the file to map to, then open /dev/zero and use that to map to. */ @@ -89,28 +88,12 @@ xbt_mheap_t xbt_mheap_new(void* baseaddr, int options) while(mdp->next_mdesc) mdp = mdp->next_mdesc; - LOCK(mdp); mdp->next_mdesc = (struct mdesc *)mbase; - UNLOCK(mdp); } return mbase; } - - -/** Terminate access to a mmalloc managed region, but do not free its content. - * - * This is for example useful for the base region where ldl stores its data - * because it leaves the place after us. - */ -void xbt_mheap_destroy_no_free(xbt_mheap_t md) -{ - struct mdesc *mdp = md; - - pthread_mutex_destroy(&mdp->mutex); -} - /** Terminate access to a mmalloc managed region by unmapping all memory pages associated with the region, and closing * the file descriptor if it is one that we opened. @@ -132,7 +115,6 @@ void *xbt_mheap_destroy(xbt_mheap_t mdp) mdptemp->next_mdesc = mdp->next_mdesc; - xbt_mheap_destroy_no_free(mdp); struct mdesc mtemp = *mdp; /* Now unmap all the pages associated with this region by asking for a @@ -158,7 +140,6 @@ static void mmalloc_fork_prepare(void) xbt_mheap_t mdp = NULL; if ((mdp =__mmalloc_default_mdp)){ while(mdp){ - LOCK(mdp); mdp = mdp->next_mdesc; } } @@ -169,7 +150,6 @@ static void mmalloc_fork_parent(void) xbt_mheap_t mdp = NULL; if ((mdp =__mmalloc_default_mdp)){ while(mdp){ - UNLOCK(mdp); mdp = mdp->next_mdesc; } } @@ -180,7 +160,6 @@ static void mmalloc_fork_child(void) struct mdesc* mdp = NULL; if ((mdp =__mmalloc_default_mdp)){ while(mdp){ - UNLOCK(mdp); mdp = mdp->next_mdesc; } } diff --git a/src/xbt/mmalloc/mmprivate.h b/src/xbt/mmalloc/mmprivate.h index cddfd15fff..451c1e6e6a 100644 --- a/src/xbt/mmalloc/mmprivate.h +++ b/src/xbt/mmalloc/mmprivate.h @@ -174,9 +174,6 @@ typedef struct { * if such a file exists. * */ struct mdesc { - /** @brief Mutex locking the access to the heap */ - pthread_mutex_t mutex; - /** @brief Chained lists of mdescs */ struct mdesc *next_mdesc; @@ -261,17 +258,6 @@ XBT_PUBLIC_DATA struct mdesc* __mmalloc_default_mdp; XBT_PUBLIC void* mmorecore(struct mdesc* mdp, ssize_t size); -/** Thread-safety (if the mutex is already created) - * - * This is mandatory in the case where the user runs a parallel simulation - * in a model-checking enabled tree. Without this protection, our malloc - * implementation will not like multi-threading AT ALL. - */ -#define LOCK(mdp) pthread_mutex_lock(&(mdp)->mutex) -#define UNLOCK(mdp) pthread_mutex_unlock(&(mdp)->mutex) - -XBT_PRIVATE int malloc_use_mmalloc(void); - XBT_PRIVATE size_t mmalloc_get_bytes_used_remote(size_t heaplimit, const malloc_info* heapinfo); /* We call dlsym during mmalloc initialization, but dlsym uses malloc.