On Fri, Jul 11, 2014 at 08:11, Ted Unangst wrote:
>> I also think there's one simple case that can be added: the MMAP call
>> at the bottom of map().
On further inspection, I think this needed a slight reordering to be
safe.
I have also been seeing random munmap errors running jruby:
java(3451) in free(): error: munmap 0x41cb4307000
Abort trap
I don't see where the race is, but I am now running with this slightly
more conservative patch. The only enabled unlock sections are for
mmap() on the way to return when internal state won't change. (On the
bright side, I guess this means the malloc lock really does work
normally. :))
Index: malloc.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.170
diff -u -p -r1.170 malloc.c
--- malloc.c 9 Jul 2014 19:11:00 -0000 1.170
+++ malloc.c 12 Jul 2014 16:52:17 -0000
@@ -93,6 +93,15 @@
#define MQUERY(a, sz) mquery((a), (size_t)(sz), PROT_READ | PROT_WRITE, \
MAP_ANON | MAP_PRIVATE | MAP_FIXED, -1, (off_t)0)
+#define KERNENTER() if (__isthreaded) do { \
+ malloc_active--; \
+ _MALLOC_UNLOCK(); \
+} while (0)
+#define KERNEXIT() if (__isthreaded) do { \
+ _MALLOC_LOCK(); \
+ malloc_active++; \
+} while (0)
+
struct region_info {
void *p; /* page; low bits used to mark chunks */
uintptr_t size; /* size for pages, or chunk_info pointer */
@@ -312,7 +321,10 @@ unmap(struct dir_info *d, void *p, size_
}
if (psz > mopts.malloc_cache) {
- if (munmap(p, sz))
+ /* KERNENTER(); */
+ i = munmap(p, sz);
+ /* KERNEXIT(); */
+ if (i)
wrterror("munmap", p);
STATS_SUB(d->malloc_used, sz);
return;
@@ -396,7 +408,9 @@ map(struct dir_info *d, size_t sz, int z
return MAP_FAILED;
}
if (psz > d->free_regions_size) {
+ KERNENTER();
p = MMAP(sz);
+ KERNEXIT();
if (p != MAP_FAILED)
STATS_ADD(d->malloc_used, sz);
/* zero fill not needed */
@@ -408,18 +422,20 @@ map(struct dir_info *d, size_t sz, int z
if (r->p != NULL) {
if (r->size == psz) {
p = r->p;
+ r->p = NULL;
+ r->size = 0;
+ d->free_regions_size -= psz;
+ /* KERNENTER(); */
if (mopts.malloc_freeunmap)
mprotect(p, sz, PROT_READ | PROT_WRITE);
if (mopts.malloc_hint)
madvise(p, sz, MADV_NORMAL);
- r->p = NULL;
- r->size = 0;
- d->free_regions_size -= psz;
if (zero_fill)
memset(p, 0, sz);
else if (mopts.malloc_junk == 2 &&
mopts.malloc_freeunmap)
memset(p, SOME_FREEJUNK, sz);
+ /* KERNEXIT(); */
return p;
} else if (r->size > psz)
big = r;
@@ -440,11 +456,13 @@ map(struct dir_info *d, size_t sz, int z
memset(p, SOME_FREEJUNK, sz);
return p;
}
+ if (d->free_regions_size > mopts.malloc_cache)
+ wrterror("malloc cache", NULL);
+ KERNENTER();
p = MMAP(sz);
+ KERNEXIT();
if (p != MAP_FAILED)
STATS_ADD(d->malloc_used, sz);
- if (d->free_regions_size > mopts.malloc_cache)
- wrterror("malloc cache", NULL);
/* zero fill not needed */
return p;
}
