[PATCHES BUG 13865] memcache tracking of talloc object sizes
Christof Schmitt
cs at samba.org
Thu Mar 28 18:28:47 UTC 2019
On Thu, Mar 28, 2019 at 11:28:14AM -0700, Christof Schmitt wrote:
> This is a fix for memcache to also add the talloc object size to the
> amount of used space. The objects become part of the memcache, so the
> sizes should be tracked as well.
>
> The third patch is a clarification to the description of the 'max stat
> cache size' parameter.
>
> Pipeline is running at:
> https://gitlab.com/samba-team/devel/samba/pipelines/54120627
>
> Christof
And here are the actual patches.
-------------- next part --------------
From 84c0ff689f4003c94e7acb0010f52c29d86d0d35 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 26 Mar 2019 08:33:08 -0700
Subject: [PATCH 1/3] memcache: Properly track the size of talloc objects
With memcache_add_talloc, the talloc object becomes part of the pool and
the memcache_element stores a pointer to the talloc object. The
size of the the talloc object was not used when tracking the used space,
allowing the cache to grow larger than defined in the memcache_init
call.
Fix this by adding the size of the talloc object to the used space.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13865
Signed-off-by: Christof Schmitt <cs at samba.org>
---
lib/util/memcache.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/lib/util/memcache.c b/lib/util/memcache.c
index 819ba44b443..1dce127dc8a 100644
--- a/lib/util/memcache.c
+++ b/lib/util/memcache.c
@@ -209,6 +209,7 @@ static void memcache_delete_element(struct memcache *cache,
memcache_element_parse(e, &cache_key, &cache_value);
SMB_ASSERT(cache_value.length == sizeof(ptr));
memcpy(&ptr, cache_value.data, sizeof(ptr));
+ cache->size -= talloc_total_size(ptr);
TALLOC_FREE(ptr);
}
@@ -248,8 +249,8 @@ void memcache_delete(struct memcache *cache, enum memcache_number n,
memcache_delete_element(cache, e);
}
-void memcache_add(struct memcache *cache, enum memcache_number n,
- DATA_BLOB key, DATA_BLOB value)
+static void memcache_do_add(struct memcache *cache, enum memcache_number n,
+ DATA_BLOB key, DATA_BLOB value, size_t talloc_size)
{
struct memcache_element *e;
struct rb_node **p;
@@ -328,9 +329,16 @@ void memcache_add(struct memcache *cache, enum memcache_number n,
DLIST_ADD(cache->mru, e);
cache->size += element_size;
+ cache->size += talloc_size;
memcache_trim(cache);
}
+void memcache_add(struct memcache *cache, enum memcache_number n,
+ DATA_BLOB key, DATA_BLOB value)
+{
+ memcache_do_add(cache, n, key, value, 0);
+}
+
void memcache_add_talloc(struct memcache *cache, enum memcache_number n,
DATA_BLOB key, void *pptr)
{
@@ -345,7 +353,8 @@ void memcache_add_talloc(struct memcache *cache, enum memcache_number n,
}
p = talloc_move(cache, ptr);
- memcache_add(cache, n, key, data_blob_const(&p, sizeof(p)));
+ memcache_do_add(cache, n, key, data_blob_const(&p, sizeof(p)),
+ talloc_total_size(p));
}
void memcache_flush(struct memcache *cache, enum memcache_number n)
--
2.17.0
From 79cd1452b0dfa2b7129fc1bd029da6dc51ca5683 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 28 Mar 2019 10:46:43 -0700
Subject: [PATCH 2/3] torture: Add test for talloc size accounting in memcache
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13865
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/torture/torture.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index c7b3bf28041..0e7bb96f69b 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11155,13 +11155,14 @@ static bool data_blob_equal(DATA_BLOB a, DATA_BLOB b)
static bool run_local_memcache(int dummy)
{
struct memcache *cache;
- DATA_BLOB k1, k2, k3;
+ DATA_BLOB k1, k2, k3, k4, k5;
DATA_BLOB d1, d3;
DATA_BLOB v1, v3;
TALLOC_CTX *mem_ctx;
char *ptr1 = NULL;
char *ptr2 = NULL;
+ char *ptr3 = NULL;
char *str1, *str2;
size_t size1, size2;
@@ -11187,6 +11188,8 @@ static bool run_local_memcache(int dummy)
k1 = data_blob_const("d1", 2);
k2 = data_blob_const("d2", 2);
k3 = data_blob_const("d3", 2);
+ k4 = data_blob_const("d4", 2);
+ k5 = data_blob_const("d5", 2);
memcache_add(cache, STAT_CACHE, k1, d1);
@@ -11250,6 +11253,23 @@ static bool run_local_memcache(int dummy)
return false;
}
+ /*
+ * Test that talloc size also is accounted in memcache and
+ * causes purge of other object.
+ */
+
+ str1 = talloc_zero_size(mem_ctx, 100);
+ str2 = talloc_zero_size(mem_ctx, 100);
+
+ memcache_add_talloc(cache, GETWD_CACHE, k4, &str1);
+ memcache_add_talloc(cache, GETWD_CACHE, k5, &str1);
+
+ ptr3 = memcache_lookup_talloc(cache, GETWD_CACHE, k4);
+ if (ptr3 != NULL) {
+ printf("Did find k4, should have been purged\n");
+ return false;
+ }
+
TALLOC_FREE(cache);
TALLOC_FREE(mem_ctx);
--
2.17.0
From e0f831c7db2c796dc088eff717c62df524484649 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 26 Mar 2019 09:43:59 -0700
Subject: [PATCH 3/3] docs: Update description for 'max stat cache'
The size specified here reflects the total size of all caches, not just
the stat cache. It might also make sense to rename this parameter, but
for for now at update the description.
Signed-off-by: Christof Schmitt <cs at samba.org>
---
docs-xml/smbdotconf/filename/maxstatcachesize.xml | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/docs-xml/smbdotconf/filename/maxstatcachesize.xml b/docs-xml/smbdotconf/filename/maxstatcachesize.xml
index 63f91e70fd2..d49b240ba6f 100644
--- a/docs-xml/smbdotconf/filename/maxstatcachesize.xml
+++ b/docs-xml/smbdotconf/filename/maxstatcachesize.xml
@@ -3,13 +3,14 @@
type="integer"
xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
<description>
- <para>This parameter limits the size in memory of any
- <parameter moreinfo="none">stat cache</parameter> being used
- to speed up case insensitive name mappings. It represents
- the number of kilobyte (1024) units the stat cache can use.
- A value of zero, meaning unlimited, is not advisable due to
- increased memory usage. You should not need to change this
- parameter.
+ <para>This parameter limits the total size of memory being
+ used for internal caches. This includes the <parameter
+ moreinfo="none">stat cache</parameter> used to speed up case
+ insensitive name mappings. The value of this parameter
+ represents the number of kilobyte (1024) units the caches can
+ use in total. A value of zero, meaning unlimited, is not
+ advisable due to increased memory usage. You should not need
+ to change this parameter.
</para>
</description>
<related>stat cache</related>
--
2.17.0
More information about the samba-technical
mailing list