The branch, master has been updated
via 617c647 Fix valgrind errors with memmove and talloc pools.
via cbfc3ef Add simple limited pool tests to test_memlimit().
via 3d0f717 Remove talloc_memlimit_update(). No longer used.
via 8e2a543 Inside _talloc_realloc(), keep track of size changes over
malloc/realloc/free.
via 314508d Don't call talloc_memlimit_update() inside
_talloc_realloc() when we're just manipulating pool members.
via 0fbcfcc Fix a conditional check. (size - tc->size > 0) is always
true if size and tc->size are unsigned.
via 4386029 In _talloc_steal_internal(), correctly decrement the memory
limit in the source, and increment in the destination.
via 6bc190d Inside _talloc_free_internal(), always call
talloc_memlimit_update_on_free() before we free the real memory.
via 4dfde7d Update memory limits when we call free() on a pool.
via a4ebbe7 Change __talloc() to only call
talloc_memlimit_check()/talloc_memlimit_grow() on actual malloc allocation.
via 4159a78 Change _talloc_total_mem_internal() to ignore memory
allocated from a pool when calculating limit size.
via 7a6beae Remove magic TC_HDR_SIZE handling inside
talloc_memlimit_check().
via fe790f6 Start to fix talloc memlimits with talloc pools.
from 323cccd smbd: Use #defines in smb2_getinfo_send
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 617c647b8ef562ace589a11a15eb460e6db71f2a
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 13:20:43 2013 -0700
Fix valgrind errors with memmove and talloc pools.
bin/smbtorture //127.0.0.1 local.talloc now runs with no valgrind errors.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: "Stefan (metze) Metzmacher" <[email protected]>
Autobuild-User(master): Jeremy Allison <[email protected]>
Autobuild-Date(master): Wed Aug 28 02:44:17 CEST 2013 on sn-devel-104
commit cbfc3efbfd4a3a6f3b031ce8ef375d37f2c545f3
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 13:09:03 2013 -0700
Add simple limited pool tests to test_memlimit().
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 3d0f717d437bb24f430fad788b9eb35e8fe8e0e8
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 13:08:33 2013 -0700
Remove talloc_memlimit_update(). No longer used.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 8e2a543e088cac36a5b6bbab1a6be961fa00cc4d
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 13:07:04 2013 -0700
Inside _talloc_realloc(), keep track of size changes over
malloc/realloc/free.
Replace the last use of talloc_memlimit_update() with
talloc_memlimit_grow()/
talloc_memlimit_shrink().
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 314508dd73105138d756f4ca3dfb65f1d368a9f7
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 13:03:27 2013 -0700
Don't call talloc_memlimit_update() inside _talloc_realloc() when we're
just manipulating pool members.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 0fbcfcc824e474874c15d7c0b2ea0df408448906
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:59:04 2013 -0700
Fix a conditional check. (size - tc->size > 0) is always true if size and
tc->size are unsigned.
Replace with (size > tc->size).
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 43860293225d14ca2c339277b42f8705322463ab
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:57:43 2013 -0700
In _talloc_steal_internal(), correctly decrement the memory limit in the
source, and increment in the destination.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 6bc190d6dd7fd0ab028c39c1463477a863f6943a
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:54:38 2013 -0700
Inside _talloc_free_internal(), always call
talloc_memlimit_update_on_free() before we free the real memory.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 4dfde7d33e7ac6c94833ecc758baff487ab67e4e
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:51:20 2013 -0700
Update memory limits when we call free() on a pool.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit a4ebbe73b4b8dcab4d344e693ad9796ec8997f87
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:49:00 2013 -0700
Change __talloc() to only call
talloc_memlimit_check()/talloc_memlimit_grow() on actual malloc allocation.
Don't check the memlimit if the allocation was successful from a pool. We
already
checked the memory limit when we created the pool.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 4159a78ed7eda340758e22286f16186987a20f2f
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:46:09 2013 -0700
Change _talloc_total_mem_internal() to ignore memory allocated from a pool
when calculating limit size.
We must only count normal tallocs, or a talloc pool itself.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit 7a6beae68ee3f9a97e9e56f4e24a437839fb3e19
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:43:50 2013 -0700
Remove magic TC_HDR_SIZE handling inside talloc_memlimit_check().
Callers already account for TC_HDR_SIZE, do not add it twice.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
commit fe790f6cbc9b888a8d613cfb515f0d0c76daad47
Author: Jeremy Allison <[email protected]>
Date: Tue Aug 27 12:36:23 2013 -0700
Start to fix talloc memlimits with talloc pools.
Add the functions:
talloc_memlimit_grow(), talloc_memlimit_shrink(),
talloc_memlimit_update_on_free().
as replacements for talloc_memlimit_update().
The interface to talloc_memlimit_update() is very
hard to understand and use. The above functions
are (to me) much clearer.
The goal of these changes is to only update
the memlimits on malloc/free/realloc, not
on every pool allocation. That way we only
count pool creation as allocation from any
imposed limits, not allocation from an already
created pool.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
lib/talloc/talloc.c | 211 +++++++++++++++++++++++++++++-------------------
lib/talloc/testsuite.c | 27 ++++++
2 files changed, 155 insertions(+), 83 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 76f0aee..69d5a16 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -236,8 +236,11 @@ struct talloc_memlimit {
};
static bool talloc_memlimit_check(struct talloc_memlimit *limit, size_t size);
-static bool talloc_memlimit_update(struct talloc_memlimit *limit,
- size_t old_size, size_t new_size);
+static void talloc_memlimit_grow(struct talloc_memlimit *limit,
+ size_t size);
+static void talloc_memlimit_shrink(struct talloc_memlimit *limit,
+ size_t size);
+static void talloc_memlimit_update_on_free(struct talloc_chunk *tc);
typedef int (*talloc_destructor_t)(void *);
@@ -581,27 +584,24 @@ static inline void *__talloc(const void *context, size_t
size)
limit = ptc->limit;
}
- if (!talloc_memlimit_check(limit, (TC_HDR_SIZE+size))) {
- errno = ENOMEM;
- return NULL;
- }
-
tc = talloc_alloc_pool(ptc, TC_HDR_SIZE+size);
}
if (tc == NULL) {
+ /*
+ * Only do the memlimit check/update on actual allocation.
+ */
+ if (!talloc_memlimit_check(limit, TC_HDR_SIZE + size)) {
+ errno = ENOMEM;
+ return NULL;
+ }
+
tc = (struct talloc_chunk *)malloc(TC_HDR_SIZE+size);
if (unlikely(tc == NULL)) return NULL;
tc->flags = TALLOC_MAGIC;
tc->pool = NULL;
- }
- if (limit != NULL) {
- struct talloc_memlimit *l;
-
- for (l = limit; l != NULL; l = l->upper) {
- l->cur_size += TC_HDR_SIZE+size;
- }
+ talloc_memlimit_grow(limit, TC_HDR_SIZE + size);
}
tc->limit = limit;
@@ -805,6 +805,8 @@ static inline void _talloc_free_poolmem(struct talloc_chunk
*tc,
*/
pool->hdr.c.name = location;
+ talloc_memlimit_update_on_free(&pool->hdr.c);
+
TC_INVALIDATE_FULL_CHUNK(&pool->hdr.c);
free(pool);
return;
@@ -905,29 +907,6 @@ static inline int _talloc_free_internal(void *ptr, const
char *location)
tc->flags |= TALLOC_FLAG_FREE;
- /*
- * If we are part of a memory limited context hierarchy
- * we need to subtract the memory used from the counters
- */
- if (tc->limit) {
- struct talloc_memlimit *l;
-
- for (l = tc->limit; l != NULL; l = l->upper) {
- if (l->cur_size >= tc->size+TC_HDR_SIZE) {
- l->cur_size -= tc->size+TC_HDR_SIZE;
- } else {
- talloc_abort("cur_size memlimit counter not
correct!");
- return 0;
- }
- }
-
- if (tc->limit->parent == tc) {
- free(tc->limit);
- }
-
- tc->limit = NULL;
- }
-
/* we mark the freed memory with where we called the free
* from. This means on a double free error we can report where
* the first free came from
@@ -948,6 +927,8 @@ static inline int _talloc_free_internal(void *ptr, const
char *location)
return 0;
}
+ talloc_memlimit_update_on_free(tc);
+
TC_INVALIDATE_FULL_CHUNK(tc);
free(tc);
return 0;
@@ -958,6 +939,8 @@ static inline int _talloc_free_internal(void *ptr, const
char *location)
return 0;
}
+ talloc_memlimit_update_on_free(tc);
+
TC_INVALIDATE_FULL_CHUNK(tc);
free(tc);
return 0;
@@ -991,11 +974,8 @@ static void *_talloc_steal_internal(const void *new_ctx,
const void *ptr)
ctx_size = _talloc_total_limit_size(ptr, NULL, NULL);
- if (!talloc_memlimit_update(tc->limit->upper, ctx_size, 0)) {
- talloc_abort("cur_size memlimit counter not correct!");
- errno = EINVAL;
- return NULL;
- }
+ /* Decrement the memory limit from the source .. */
+ talloc_memlimit_shrink(tc->limit->upper, ctx_size);
if (tc->limit->parent == tc) {
tc->limit->upper = NULL;
@@ -1043,13 +1023,9 @@ static void *_talloc_steal_internal(const void *new_ctx,
const void *ptr)
if (tc->limit || new_tc->limit) {
ctx_size = _talloc_total_limit_size(ptr, tc->limit,
new_tc->limit);
- }
-
- if (new_tc->limit) {
- struct talloc_memlimit *l;
-
- for (l = new_tc->limit; l != NULL; l = l->upper) {
- l->cur_size += ctx_size;
+ /* .. and increment it in the destination. */
+ if (new_tc->limit) {
+ talloc_memlimit_grow(new_tc->limit, ctx_size);
}
}
@@ -1501,6 +1477,8 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
void *new_ptr;
bool malloced = false;
union talloc_pool_chunk *pool_tc = NULL;
+ size_t old_size = 0;
+ size_t new_size = 0;
/* size zero is equivalent to free() */
if (unlikely(size == 0)) {
@@ -1529,7 +1507,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
return NULL;
}
- if (tc->limit && (size - tc->size > 0)) {
+ if (tc->limit && (size > tc->size)) {
if (!talloc_memlimit_check(tc->limit, (size - tc->size))) {
errno = ENOMEM;
return NULL;
@@ -1588,6 +1566,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
if (new_ptr == NULL) {
new_ptr = malloc(TC_HDR_SIZE+size);
malloced = true;
+ new_size = size;
}
if (new_ptr) {
@@ -1595,6 +1574,9 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
TC_INVALIDATE_FULL_CHUNK(tc);
}
} else {
+ /* We're doing malloc then free here, so record the difference.
*/
+ old_size = tc->size;
+ new_size = size;
new_ptr = malloc(size + TC_HDR_SIZE);
if (new_ptr) {
memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE);
@@ -1627,6 +1609,27 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
size_t old_used = TC_HDR_SIZE + tc->size;
size_t new_used = TC_HDR_SIZE + size;
new_ptr = start;
+
+#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
+ {
+ /*
+ * The area from
+ * start -> tc may have
+ * been freed and thus been marked as
+ * VALGRIND_MEM_NOACCESS. Set it to
+ * VALGRIND_MEM_UNDEFINED so we can
+ * copy into it without valgrind errors.
+ * We can't just mark
+ * new_ptr -> new_ptr + old_used
+ * as this may overlap on top of tc,
+ * (which is why we use memmove, not
+ * memcpy below) hence the MIN.
+ */
+ size_t undef_len = MIN((((char *)tc) -
((char *)new_ptr)),old_used);
+ VALGRIND_MAKE_MEM_UNDEFINED(new_ptr,
undef_len);
+ }
+#endif
+
memmove(new_ptr, tc, old_used);
tc = (struct talloc_chunk *)new_ptr;
@@ -1651,14 +1654,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
if (new_chunk_size == old_chunk_size) {
TC_UNDEFINE_GROW_CHUNK(tc, size);
tc->flags &= ~TALLOC_FLAG_FREE;
- if (!talloc_memlimit_update(tc->limit,
- tc->size, size)) {
- talloc_abort("cur_size memlimit counter not"
- " correct!");
- errno = EINVAL;
- return NULL;
- }
-
tc->size = size;
return ptr;
}
@@ -1674,13 +1669,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
if (space_left >= space_needed) {
TC_UNDEFINE_GROW_CHUNK(tc, size);
tc->flags &= ~TALLOC_FLAG_FREE;
- if (!talloc_memlimit_update(tc->limit,
- tc->size, size)) {
- talloc_abort("cur_size memlimit "
- "counter not correct!");
- errno = EINVAL;
- return NULL;
- }
tc->size = size;
pool_tc->hdr.c.pool = tc_next_chunk(tc);
return ptr;
@@ -1692,6 +1680,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
if (new_ptr == NULL) {
new_ptr = malloc(TC_HDR_SIZE+size);
malloced = true;
+ new_size = size;
}
if (new_ptr) {
@@ -1701,6 +1690,9 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void
*ptr, size_t size, cons
}
}
else {
+ /* We're doing realloc here, so record the difference. */
+ old_size = tc->size;
+ new_size = size;
new_ptr = realloc(tc, size + TC_HDR_SIZE);
}
got_new_ptr:
@@ -1729,11 +1721,12 @@ got_new_ptr:
tc->next->prev = tc;
}
- if (!talloc_memlimit_update(tc->limit, tc->size, size)) {
- talloc_abort("cur_size memlimit counter not correct!");
- errno = EINVAL;
- return NULL;
+ if (new_size > old_size) {
+ talloc_memlimit_grow(tc->limit, new_size - old_size);
+ } else if (new_size < old_size) {
+ talloc_memlimit_shrink(tc->limit, old_size - new_size);
}
+
tc->size = size;
_talloc_set_name_const(TC_PTR_FROM_CHUNK(tc), name);
@@ -1812,7 +1805,14 @@ static size_t _talloc_total_mem_internal(const void *ptr,
break;
case TOTAL_MEM_LIMIT:
if (likely(tc->name != TALLOC_MAGIC_REFERENCE)) {
- total = tc->size + TC_HDR_SIZE;
+ /*
+ * Don't count memory allocated from a pool
+ * when calculating limits. Only count the
+ * pool itself.
+ */
+ if (!(tc->flags & TALLOC_FLAG_POOLMEM)) {
+ total = tc->size + TC_HDR_SIZE;
+ }
}
break;
}
@@ -2556,7 +2556,7 @@ static bool talloc_memlimit_check(struct talloc_memlimit
*limit, size_t size)
for (l = limit; l != NULL; l = l->upper) {
if (l->max_size != 0 &&
((l->max_size <= l->cur_size) ||
- (l->max_size - l->cur_size < TC_HDR_SIZE+size))) {
+ (l->max_size - l->cur_size < size))) {
return false;
}
}
@@ -2564,26 +2564,71 @@ static bool talloc_memlimit_check(struct
talloc_memlimit *limit, size_t size)
return true;
}
-static bool talloc_memlimit_update(struct talloc_memlimit *limit,
- size_t old_size, size_t new_size)
+/*
+ Update memory limits when freeing a talloc_chunk.
+*/
+static void talloc_memlimit_update_on_free(struct talloc_chunk *tc)
{
- struct talloc_memlimit *l;
- ssize_t d;
+ if (!tc->limit) {
+ return;
+ }
- if (old_size == 0) {
- d = new_size + TC_HDR_SIZE;
- } else {
- d = new_size - old_size;
+ /*
+ * Pool entries don't count. Only the pools
+ * themselves are counted as part of the memory
+ * limits.
+ */
+ if (tc->flags & TALLOC_FLAG_POOLMEM) {
+ return;
}
+
+ /*
+ * If we are part of a memory limited context hierarchy
+ * we need to subtract the memory used from the counters
+ */
+
+ talloc_memlimit_shrink(tc->limit, tc->size+TC_HDR_SIZE);
+
+ if (tc->limit->parent == tc) {
+ free(tc->limit);
+ }
+
+ tc->limit = NULL;
+}
+
+/*
+ Increase memory limit accounting after a malloc/realloc.
+*/
+static void talloc_memlimit_grow(struct talloc_memlimit *limit,
+ size_t size)
+{
+ struct talloc_memlimit *l;
+
for (l = limit; l != NULL; l = l->upper) {
- ssize_t new_cur_size = l->cur_size + d;
- if (new_cur_size < 0) {
- return false;
+ size_t new_cur_size = l->cur_size + size;
+ if (new_cur_size < l->cur_size) {
+ talloc_abort("logic error in talloc_memlimit_grow\n");
+ return;
}
l->cur_size = new_cur_size;
}
+}
- return true;
+/*
+ Decrease memory limit accounting after a free/realloc.
+*/
+static void talloc_memlimit_shrink(struct talloc_memlimit *limit,
+ size_t size)
+{
+ struct talloc_memlimit *l;
+
+ for (l = limit; l != NULL; l = l->upper) {
+ if (l->cur_size < size) {
+ talloc_abort("logic error in talloc_memlimit_shrink\n");
+ return;
+ }
+ l->cur_size = l->cur_size - size;
+ }
}
_PUBLIC_ int talloc_set_memlimit(const void *ctx, size_t max_size)
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index d456cbb..426c31a 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1359,6 +1359,8 @@ static bool test_memlimit(void)
{
void *root;
char *l1, *l2, *l3, *l4, *l5, *t;
+ char *pool;
+ int i;
printf("test: memlimit\n# MEMORY LIMITS\n");
@@ -1520,6 +1522,31 @@ static bool test_memlimit(void)
talloc_report_full(root, stdout);
talloc_free(root);
+ /* Test memlimits with pools. */
+ pool = talloc_pool(NULL, 10*1024);
+ torture_assert("memlimit", pool != NULL,
+ "failed: alloc should not fail due to memory limit\n");
+ talloc_set_memlimit(pool, 10*1024);
+ for (i = 0; i < 9; i++) {
+ l1 = talloc_size(pool, 1024);
+ torture_assert("memlimit", l1 != NULL,
+ "failed: alloc should not fail due to memory limit\n");
+ }
+ /* The next alloc should fail. */
+ l2 = talloc_size(pool, 1024);
+ torture_assert("memlimit", l2 == NULL,
+ "failed: alloc should fail due to memory limit\n");
+
+ /* Moving one of the children shouldn't change the limit,
+ as it's still inside the pool. */
+ root = talloc_new(NULL);
+ talloc_steal(root, l1);
+ l2 = talloc_size(pool, 1024);
+ torture_assert("memlimit", l2 == NULL,
+ "failed: alloc should fail due to memory limit\n");
+
+ talloc_free(pool);
+ talloc_free(root);
printf("success: memlimit\n");
return true;
--
Samba Shared Repository
