Attachment 15021 Details for Bug 13865 – git-am fix for master.

[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:

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
>

Actions: View

Attachments on bug 13865: 15021 | 15023 | 15050 | 15051