The Samba-Bugzilla – Attachment 15021 Details for
Bug 13865
memcache grows larger then allowed maximum size
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for master.
patches (text/plain), 7.81 KB, created by
Jeremy Allison
on 2019-03-29 21:57:04 UTC
(
hide
)
Description:
git-am fix for master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2019-03-29 21:57:04 UTC
Size:
7.81 KB
patch
obsolete
>From feb6cbff4d586bec1545f731377217b06241744f Mon Sep 17 00:00:00 2001 >From: Christof Schmitt <cs@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@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >--- > lib/util/memcache.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > >diff --git a/lib/util/memcache.c b/lib/util/memcache.c >index 819ba44b443..2066fc5b2dd 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; >@@ -278,6 +279,7 @@ void memcache_add(struct memcache *cache, enum memcache_number n, > void *ptr; > SMB_ASSERT(cache_value.length == sizeof(ptr)); > memcpy(&ptr, cache_value.data, sizeof(ptr)); >+ cache->size -= talloc_total_size(ptr); > TALLOC_FREE(ptr); > } > /* >@@ -328,9 +330,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 +354,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.21.0.392.gf8f6787159e-goog > > >From 54f3e08690db17a3ff8dfd2ff011c46ad4966875 Mon Sep 17 00:00:00 2001 >From: Christof Schmitt <cs@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@samba.org> >--- > source3/torture/torture.c | 70 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 69 insertions(+), 1 deletion(-) > >diff --git a/source3/torture/torture.c b/source3/torture/torture.c >index 219ac4a370c..782980dec3a 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,71 @@ 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; >+ } >+ >+ /* >+ * Test that adding a duplicate non-talloced >+ * key/value on top of a talloced key/value takes account >+ * of the talloc_freed value size. >+ */ >+ TALLOC_FREE(cache); >+ TALLOC_FREE(mem_ctx); >+ >+ mem_ctx = talloc_init("key_replace"); >+ if (mem_ctx == NULL) { >+ return false; >+ } >+ >+ cache = memcache_init(NULL, sizeof(void *) == 8 ? 200 : 100); >+ if (cache == NULL) { >+ return false; >+ } >+ >+ /* >+ * Add a 100 byte talloced string. This will >+ * store a (4 or 8 byte) pointer and record the >+ * total talloced size. >+ */ >+ str1 = talloc_zero_size(mem_ctx, 100); >+ memcache_add_talloc(cache, GETWD_CACHE, k4, &str1); >+ /* >+ * Now overwrite with a small talloced >+ * value. This should fit in the existing size >+ * and the total talloced size should be removed >+ * from the cache size. >+ */ >+ str1 = talloc_zero_size(mem_ctx, 2); >+ memcache_add_talloc(cache, GETWD_CACHE, k4, &str1); >+ /* >+ * Now store a 20 byte string. If the >+ * total talloced size wasn't accounted for >+ * and removed in the overwrite, then this >+ * will evict k4. >+ */ >+ str2 = talloc_zero_size(mem_ctx, 20); >+ memcache_add_talloc(cache, GETWD_CACHE, k5, &str2); >+ >+ ptr3 = memcache_lookup_talloc(cache, GETWD_CACHE, k4); >+ if (ptr3 == NULL) { >+ printf("Did not find k4, should not have been purged\n"); >+ return false; >+ } >+ > TALLOC_FREE(cache); > TALLOC_FREE(mem_ctx); > >-- >2.21.0.392.gf8f6787159e-goog > > >From 2dc57a3141d236c968cbbcfd8fa62c73382eeb9e Mon Sep 17 00:00:00 2001 >From: Christof Schmitt <cs@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@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.21.0.392.gf8f6787159e-goog >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 13865
:
15021
|
15023
|
15050
|
15051