I started this email after reading the thread started by Hongjia Cao
"performance improvement for hostlist". After a bit of digging and
writing it grew beyond the issue pointed out in that thread so I thought
it best to begin a new one since it may take on a discussion of its own.
It also includes a good bit of ancillary information people may find
generally useful. I don't think the solution committed for the problem
Cao highlighted is correct. My proposed fix is in the second to last
paragraph although it somewhat depends on understanding the other items
that lead up to it.
It already looks long, doesn't it? Let's dive in...
First, the timing test provided by Cao is bogus but perfectly
illustrates the bigger problem Cao is trying to fix. (I'm not saying Cao
is wrong or it was a bad test to run because there's something to be
learned and it led to a more careful analysis.)
Second, a bit of background for those that are unfamiliar with
malloc/memset; skip the paragraph if you know this. malloc takes
practically no time on Linux with glibc because it doesn't really do
much. It's not until the memory is actually accessed that it is truly
allocated by Linux. That's why there is such a discrepancy in the
timing; xmalloc is actually accessing the memory to zero it. This is
also why zeroing a bunch of memory unnecessarily can be a big waste of
time as Cao found. (A more realistic test would involve accessing a
portion of the memory between each xmalloc/malloc and xfree/free.)
It's also worth noting at this time that there is a fundamental
difference between malloc+memset and calloc. For a better explanation
than I can include in this space see Dietrich Epp's writeup on Stack
Overflow[1].
I'll say Cao is mostly right. It is probably better to *not* use xmalloc
since it is potentially allocating more memory than necessary and
zeroing otherwise untouched memory is a waste. However, xmalloc does
more than just zero the memory; by switching to malloc those other
features are getting tossed.
The question becomes whether those features of xmalloc are important
enough to be kept for this case. (I have made some nodes about
additional problems to be corrected.) Let's take a look:
>void *slurm_xmalloc
>(size_t size, const char *file, int line, const char *func)
>{
>void *new;
>int *p;
>
>
>xmalloc_assert(size >= 0 && size <= INT_MAX);
Good for debugging. However, size is type size_t. Comparison should be
against SIZE_MAX.
>MALLOC_LOCK();
>p = (int *)malloc(size + 2*sizeof(int));
Don't cast malloc. Ever. It hides problems. The 2*sizeof(int) is a
problem too as seen a little later once we start assigning to it.
>MALLOC_UNLOCK();
*Great* on platforms with non-thread-safe malloc and since we care about
an OOM condition and want to handle it once. Since Slurm is
multithreaded we don't want >1 OOM condition simultaneously. These
reasons are enough in my opinion to *not* replace xmalloc with malloc.
However, I'll continue because there is another separate but interesting
aspect that should be considered while we're here.
>if (!p) {
>/* out of memory */
>log_oom(file, line, func);
>abort();
>}
OK. Handle the OOM condition without attempting to allocate more memory
(see implementation of log_oom for details). However, because the mutex
doesn't cover it another thread could have already triggered and/or
partially handled an OOM condition. It should probably be within the
mutex block.
>p[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
>p[1] = (int)size; /* store size in buffer */
>
Neat. The only problem is size_t and int aren't the same thing and this
feature would not work as intended on some architectures.
>new = &p[2];
>memset(new, 0, size);
Whoops. We can also get an OOM condition here and we can't catch it
thanks to Linux over-committing memory by default. The malloc above
could have succeeded but because it wasn't *actually* allocated. Another
thread (other another program) could have already used enough memory for
memset to fail. It could be moved inside the mutex block to help prevent
another Slurm thread from triggering an OOM condition here but if memory
is already scarce all bets are off because it will probably be caused
soon anyway.
>return new;
>}
Now, there is the issue of malloc+memset vs calloc. For instances where
it's *necessary* to zero the memory (namely ease of string manipulation,
struct field initialization, or security) calloc *could* be used simply
because it's more efficient. However, because of what xmalloc is trying
to accomplish handling OOM conditions calloc will eliminate the memset
call that can't be handled. Not only *could* calloc be used for
efficiency, it *should* be used so xmalloc functions as intended (and
more gracefully handles a memory allocation error).
Now, all the way back to why this originally came up: I think a more
appropriate change is to fix xmalloc to use calloc which should on its
own give a speedup to cases like the one Cao was seeing without changing
its semantics.
If necessary, another malloc wrapper *could* be added that just used
malloc, without zeroing the memory. However, the only benefit over the
calloc-based wrapper would be when a system is under sufficient load
that the kernel has not had time to collect some all-zero pages and
there is a slight delay to calloc. I suspect the system load is really
going to be a bigger slowdown than the small delay in calloc and there
is no benefit to a non-zeroing malloc wrapper.
Please see the attached patches for my proposed fixes to the malloc
wrappers. I have done some limited testing but they should definitely be
well reviewed before committing.
[1]
http://stackoverflow.com/questions/2688466/why-mallocmemset-is-slower-than-calloc
--
Daniel M. Weeks
Systems Programmer
Center for Computational Innovations
Rensselaer Polytechnic Institute
Troy, NY 12180
518-276-4458
>From 5d5247ce2fa4b85c474e4fcaf07e621f80ea4507 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 14:08:55 -0400
Subject: [PATCH 1/6] Do not release malloc mutex before handling error
---
src/common/xmalloc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index de8f20d..043c07c 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -92,12 +92,12 @@ void *slurm_xmalloc(size_t size, const char *file, int line, const char *func)
xmalloc_assert(size >= 0 && size <= INT_MAX);
MALLOC_LOCK();
p = (int *)malloc(size + 2*sizeof(int));
- MALLOC_UNLOCK();
if (!p) {
/* out of memory */
log_oom(file, line, func);
abort();
}
+ MALLOC_UNLOCK();
p[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
p[1] = (int)size; /* store size in buffer */
@@ -154,10 +154,10 @@ void * slurm_xrealloc(void **item, size_t newsize,
MALLOC_LOCK();
p = (int *)realloc(p, newsize + 2*sizeof(int));
- MALLOC_UNLOCK();
if (p == NULL)
goto error;
+ MALLOC_UNLOCK();
if (old_size < newsize) {
char *p_new = (char *)(&p[2]) + old_size;
@@ -169,10 +169,10 @@ void * slurm_xrealloc(void **item, size_t newsize,
/* Initalize new memory */
MALLOC_LOCK();
p = (int *)malloc(newsize + 2*sizeof(int));
- MALLOC_UNLOCK();
if (p == NULL)
goto error;
+ MALLOC_UNLOCK();
memset(&p[2], 0, newsize);
p[0] = XMALLOC_MAGIC;
--
Daniel M. Weeks
>From be7d4f4a09e21f9645d969c1441a12e3caee98f8 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 14:12:13 -0400
Subject: [PATCH 2/6] Do not cast return of malloc/realloc
---
src/common/xmalloc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index 043c07c..df72a84 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -91,7 +91,7 @@ void *slurm_xmalloc(size_t size, const char *file, int line, const char *func)
xmalloc_assert(size >= 0 && size <= INT_MAX);
MALLOC_LOCK();
- p = (int *)malloc(size + 2*sizeof(int));
+ p = malloc(size + 2*sizeof(int));
if (!p) {
/* out of memory */
log_oom(file, line, func);
@@ -117,7 +117,7 @@ void *slurm_try_xmalloc(size_t size, const char *file, int line,
xmalloc_assert(size >= 0 && size <= INT_MAX);
MALLOC_LOCK();
- p = (int *)malloc(size + 2*sizeof(int));
+ p = malloc(size + 2*sizeof(int));
MALLOC_UNLOCK();
if (!p) {
return NULL;
@@ -153,7 +153,7 @@ void * slurm_xrealloc(void **item, size_t newsize,
old_size = p[1];
MALLOC_LOCK();
- p = (int *)realloc(p, newsize + 2*sizeof(int));
+ p = realloc(p, newsize + 2*sizeof(int));
if (p == NULL)
goto error;
@@ -168,7 +168,7 @@ void * slurm_xrealloc(void **item, size_t newsize,
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = (int *)malloc(newsize + 2*sizeof(int));
+ p = malloc(newsize + 2*sizeof(int));
if (p == NULL)
goto error;
@@ -208,7 +208,7 @@ int slurm_try_xrealloc(void **item, size_t newsize,
old_size = p[1];
MALLOC_LOCK();
- p = (int *)realloc(p, newsize + 2*sizeof(int));
+ p = realloc(p, newsize + 2*sizeof(int));
MALLOC_UNLOCK();
if (p == NULL)
@@ -223,7 +223,7 @@ int slurm_try_xrealloc(void **item, size_t newsize,
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = (int *)malloc(newsize + 2*sizeof(int));
+ p = malloc(newsize + 2*sizeof(int));
MALLOC_UNLOCK();
if (p == NULL)
--
Daniel M. Weeks
>From 5bf855b5671c83ce89fc3c577b76ef03f45da447 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 14:14:59 -0400
Subject: [PATCH 3/6] Avoid memset in xmalloc wrappers
---
src/common/xmalloc.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index df72a84..58dbfa2 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -91,7 +91,7 @@ void *slurm_xmalloc(size_t size, const char *file, int line, const char *func)
xmalloc_assert(size >= 0 && size <= INT_MAX);
MALLOC_LOCK();
- p = malloc(size + 2*sizeof(int));
+ p = calloc(1, size + 2*sizeof(int));
if (!p) {
/* out of memory */
log_oom(file, line, func);
@@ -102,7 +102,6 @@ void *slurm_xmalloc(size_t size, const char *file, int line, const char *func)
p[1] = (int)size; /* store size in buffer */
new = &p[2];
- memset(new, 0, size);
return new;
}
@@ -117,7 +116,7 @@ void *slurm_try_xmalloc(size_t size, const char *file, int line,
xmalloc_assert(size >= 0 && size <= INT_MAX);
MALLOC_LOCK();
- p = malloc(size + 2*sizeof(int));
+ p = calloc(1, size + 2*sizeof(int));
MALLOC_UNLOCK();
if (!p) {
return NULL;
@@ -126,7 +125,6 @@ void *slurm_try_xmalloc(size_t size, const char *file, int line,
p[1] = (int)size; /* store size in buffer */
new = &p[2];
- memset(new, 0, size);
return new;
}
@@ -168,13 +166,12 @@ void * slurm_xrealloc(void **item, size_t newsize,
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = malloc(newsize + 2*sizeof(int));
+ p = calloc(1, newsize + 2*sizeof(int));
if (p == NULL)
goto error;
MALLOC_UNLOCK();
- memset(&p[2], 0, newsize);
p[0] = XMALLOC_MAGIC;
}
@@ -223,13 +220,12 @@ int slurm_try_xrealloc(void **item, size_t newsize,
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = malloc(newsize + 2*sizeof(int));
+ p = calloc(1, newsize + 2*sizeof(int));
MALLOC_UNLOCK();
if (p == NULL)
return 0;
- memset(&p[2], 0, newsize);
p[0] = XMALLOC_MAGIC;
}
--
Daniel M. Weeks
>From 424f5821108d702722c92b206dcacd2a93ba28e0 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 16:03:15 -0400
Subject: [PATCH 4/6] Correctly use size_t in malloc wrappers
---
src/common/xmalloc.c | 119 +++++++++++++++++++++++++++++++--------------------
1 file changed, 72 insertions(+), 47 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index 58dbfa2..ec4ee6c 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -48,7 +48,7 @@
#include <errno.h>
#include <string.h>
#include <stdio.h>
-#include <limits.h> /* for INT_MAX */
+#include <stdint.h> /* for SIZE_MAX */
#include "src/common/xmalloc.h"
#include "src/common/log.h"
@@ -77,6 +77,7 @@ static void malloc_assert_failed(char *, const char *, int,
} _STMT_END
#endif /* NDEBUG */
+#define XMALLOC_HEADER_SIZE (sizeof(int) + sizeof(size_t))
/*
* "Safe" version of malloc().
@@ -86,22 +87,26 @@ static void malloc_assert_failed(char *, const char *, int,
void *slurm_xmalloc(size_t size, const char *file, int line, const char *func)
{
void *new;
- int *p;
+ void *p;
+ int *i;
+ size_t *s;
- xmalloc_assert(size >= 0 && size <= INT_MAX);
+ xmalloc_assert(size >= 0 && size <= SIZE_MAX);
MALLOC_LOCK();
- p = calloc(1, size + 2*sizeof(int));
+ p = calloc(1, size + XMALLOC_HEADER_SIZE);
if (!p) {
/* out of memory */
log_oom(file, line, func);
abort();
}
MALLOC_UNLOCK();
- p[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
- p[1] = (int)size; /* store size in buffer */
+ i = p;
+ i[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
+ s = p + sizeof(int);
+ s[0] = size; /* store size in buffer */
- new = &p[2];
+ new = p + XMALLOC_HEADER_SIZE;
return new;
}
@@ -112,19 +117,23 @@ void *slurm_try_xmalloc(size_t size, const char *file, int line,
const char *func)
{
void *new;
- int *p;
+ void *p;
+ int *i;
+ size_t *s;
- xmalloc_assert(size >= 0 && size <= INT_MAX);
+ xmalloc_assert(size >= 0 && size <= SIZE_MAX);
MALLOC_LOCK();
- p = calloc(1, size + 2*sizeof(int));
+ p = calloc(1, size + XMALLOC_HEADER_SIZE);
MALLOC_UNLOCK();
if (!p) {
return NULL;
}
- p[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
- p[1] = (int)size; /* store size in buffer */
+ i = p;
+ i[0] = XMALLOC_MAGIC; /* add "secret" magic cookie */
+ s = p + sizeof(int);
+ s[0] = size; /* store size in buffer */
- new = &p[2];
+ new = p + XMALLOC_HEADER_SIZE;
return new;
}
@@ -137,46 +146,51 @@ void *slurm_try_xmalloc(size_t size, const char *file, int line,
void * slurm_xrealloc(void **item, size_t newsize,
const char *file, int line, const char *func)
{
- int *p = NULL;
+ void *p;
+ int *i;
+ size_t *s;
/* xmalloc_assert(*item != NULL, file, line, func); */
- xmalloc_assert(newsize >= 0 && (int)newsize <= INT_MAX);
+ xmalloc_assert(newsize >= 0 && newsize <= SIZE_MAX);
if (*item != NULL) {
- int old_size;
- p = (int *)*item - 2;
+ size_t old_size;
+ p = *item - XMALLOC_HEADER_SIZE;
+ i = p;
+ s = p + sizeof(int);
/* magic cookie still there? */
- xmalloc_assert(p[0] == XMALLOC_MAGIC);
- old_size = p[1];
+ xmalloc_assert(i[0] == XMALLOC_MAGIC);
+ old_size = s[0];
MALLOC_LOCK();
- p = realloc(p, newsize + 2*sizeof(int));
+ p = realloc(p, newsize + XMALLOC_HEADER_SIZE);
if (p == NULL)
goto error;
MALLOC_UNLOCK();
if (old_size < newsize) {
- char *p_new = (char *)(&p[2]) + old_size;
- memset(p_new, 0, (int)(newsize-old_size));
+ void *p_new = p + XMALLOC_HEADER_SIZE + old_size;
+ memset(p_new, 0, newsize-old_size);
}
xmalloc_assert(p[0] == XMALLOC_MAGIC);
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = calloc(1, newsize + 2*sizeof(int));
-
+ p = calloc(1, newsize + XMALLOC_HEADER_SIZE);
if (p == NULL)
goto error;
MALLOC_UNLOCK();
-
- p[0] = XMALLOC_MAGIC;
+
+ i = p;
+ i[0] = XMALLOC_MAGIC;
}
- p[1] = (int)newsize;
- *item = &p[2];
+ s = p + sizeof(int);
+ s[0] = newsize;
+ *item = p + XMALLOC_HEADER_SIZE;
return *item;
error:
@@ -191,46 +205,52 @@ void * slurm_xrealloc(void **item, size_t newsize,
int slurm_try_xrealloc(void **item, size_t newsize,
const char *file, int line, const char *func)
{
- int *p = NULL;
+ void *p;
+ int *i;
+ size_t *s;
/* xmalloc_assert(*item != NULL, file, line, func); */
- xmalloc_assert(newsize >= 0 && (int)newsize <= INT_MAX);
+ xmalloc_assert(newsize >= 0 && newsize <= SIZE_MAX);
if (*item != NULL) {
- int old_size;
- p = (int *)*item - 2;
+ size_t old_size;
+ p = *item - XMALLOC_HEADER_SIZE;
+ i = p;
+ s = p + sizeof(int);
/* magic cookie still there? */
- xmalloc_assert(p[0] == XMALLOC_MAGIC);
- old_size = p[1];
+ xmalloc_assert(i[0] == XMALLOC_MAGIC);
+ old_size = s[0];
MALLOC_LOCK();
- p = realloc(p, newsize + 2*sizeof(int));
+ p = realloc(p, newsize + XMALLOC_HEADER_SIZE);
MALLOC_UNLOCK();
if (p == NULL)
return 0;
if (old_size < newsize) {
- char *p_new = (char *)(&p[2]) + old_size;
- memset(p_new, 0, (int)(newsize-old_size));
+ void *p_new = p + XMALLOC_HEADER_SIZE + old_size;
+ memset(p_new, 0, newsize-old_size);
}
xmalloc_assert(p[0] == XMALLOC_MAGIC);
} else {
/* Initalize new memory */
MALLOC_LOCK();
- p = calloc(1, newsize + 2*sizeof(int));
+ p = calloc(1, newsize + XMALLOC_HEADER_SIZE);
MALLOC_UNLOCK();
if (p == NULL)
return 0;
- p[0] = XMALLOC_MAGIC;
+ i = p;
+ i[0] = XMALLOC_MAGIC;
}
- p[1] = newsize;
- *item = &p[2];
+ s = p + sizeof(int);
+ s[0] = newsize;
+ *item = p + XMALLOC_HEADER_SIZE;
return 1;
}
@@ -241,10 +261,14 @@ int slurm_try_xrealloc(void **item, size_t newsize,
*/
int slurm_xsize(void *item, const char *file, int line, const char *func)
{
- int *p = (int *)item - 2;
+ void *p = item - XMALLOC_HEADER_SIZE;
+ size_t *s = p + sizeof(int);
+#if !NDEBUG
+ int *i = p;
+#endif
xmalloc_assert(item != NULL);
- xmalloc_assert(p[0] == XMALLOC_MAGIC); /* CLANG false positive here */
- return p[1];
+ xmalloc_assert(i[0] == XMALLOC_MAGIC);
+ return s[0];
}
/*
@@ -255,10 +279,11 @@ int slurm_xsize(void *item, const char *file, int line, const char *func)
void slurm_xfree(void **item, const char *file, int line, const char *func)
{
if (*item != NULL) {
- int *p = (int *)*item - 2;
+ void *p = *item - XMALLOC_HEADER_SIZE;
+ int *i = p;
/* magic cookie still there? */
- xmalloc_assert(p[0] == XMALLOC_MAGIC);
- p[0] = 0; /* make sure xfree isn't called twice */
+ xmalloc_assert(i[0] == XMALLOC_MAGIC);
+ i[0] = 0; /* make sure xfree isn't called twice */
MALLOC_LOCK();
free(p);
MALLOC_UNLOCK();
--
Daniel M. Weeks
>From ba611784c984d649e91ac9065d5d89bc057411a8 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 16:33:19 -0400
Subject: [PATCH 5/6] Make slurm_xsize return size_t
---
src/common/xmalloc.c | 2 +-
src/common/xmalloc.h | 24 ++++++++++++------------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index ec4ee6c..d10c6d4 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -259,7 +259,7 @@ int slurm_try_xrealloc(void **item, size_t newsize,
* Return the size of a buffer.
* item (IN) pointer to allocated space
*/
-int slurm_xsize(void *item, const char *file, int line, const char *func)
+size_t slurm_xsize(void *item, const char *file, int line, const char *func)
{
void *p = item - XMALLOC_HEADER_SIZE;
size_t *s = p + sizeof(int);
diff --git a/src/common/xmalloc.h b/src/common/xmalloc.h
index 93b0812..80464a9 100644
--- a/src/common/xmalloc.h
+++ b/src/common/xmalloc.h
@@ -42,12 +42,12 @@
*****************************************************************************
* Description:
*
- * void *xmalloc(size_t size);
- * void *try_xmalloc(size_t size);
- * void xrealloc(void *p, size_t newsize);
- * int try_xrealloc(void *p, size_t newsize);
- * void xfree(void *p);
- * int xsize(void *p);
+ * void *xmalloc(size_t size);
+ * void *try_xmalloc(size_t size);
+ * void xrealloc(void *p, size_t newsize);
+ * int try_xrealloc(void *p, size_t newsize);
+ * void xfree(void *p);
+ * size_t xsize(void *p);
*
* xmalloc(size) allocates size bytes and returns a pointer to the allocated
* memory. The memory is set to zero. xmalloc() will not return unless
@@ -103,12 +103,12 @@
#define xsize(__p) \
slurm_xsize((void *)__p, __FILE__, __LINE__, __CURRENT_FUNC__)
-void *slurm_xmalloc(size_t, const char *, int, const char *);
-void *slurm_try_xmalloc(size_t , const char *, int , const char *);
-void slurm_xfree(void **, const char *, int, const char *);
-void *slurm_xrealloc(void **, size_t, const char *, int, const char *);
-int slurm_try_xrealloc(void **, size_t, const char *, int, const char *);
-int slurm_xsize(void *, const char *, int, const char *);
+void *slurm_xmalloc(size_t, const char *, int, const char *);
+void *slurm_try_xmalloc(size_t , const char *, int , const char *);
+void slurm_xfree(void **, const char *, int, const char *);
+void *slurm_xrealloc(void **, size_t, const char *, int, const char *);
+int slurm_try_xrealloc(void **, size_t, const char *, int, const char *);
+size_t slurm_xsize(void *, const char *, int, const char *);
#define XMALLOC_MAGIC 0x42
--
Daniel M. Weeks
>From c623c4f9fc5b1d050a18340c824b5f2bc2e302f0 Mon Sep 17 00:00:00 2001
From: "Daniel M. Weeks" <[email protected]>
Date: Fri, 21 Mar 2014 16:34:59 -0400
Subject: [PATCH 6/6] Move XMALLOC_MAGIC out of header
---
src/common/xmalloc.c | 1 +
src/common/xmalloc.h | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/common/xmalloc.c b/src/common/xmalloc.c
index d10c6d4..37c0228 100644
--- a/src/common/xmalloc.c
+++ b/src/common/xmalloc.c
@@ -77,6 +77,7 @@ static void malloc_assert_failed(char *, const char *, int,
} _STMT_END
#endif /* NDEBUG */
+#define XMALLOC_MAGIC 0x42
#define XMALLOC_HEADER_SIZE (sizeof(int) + sizeof(size_t))
/*
diff --git a/src/common/xmalloc.h b/src/common/xmalloc.h
index 80464a9..e5e671e 100644
--- a/src/common/xmalloc.h
+++ b/src/common/xmalloc.h
@@ -110,6 +110,4 @@ void *slurm_xrealloc(void **, size_t, const char *, int, const char *);
int slurm_try_xrealloc(void **, size_t, const char *, int, const char *);
size_t slurm_xsize(void *, const char *, int, const char *);
-#define XMALLOC_MAGIC 0x42
-
#endif /* !_XMALLOC_H */
--
Daniel M. Weeks
