[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Sat Apr 6 06:09:02 UTC 2019


The branch, master has been updated
       via  b7028c42462 torture: Add test for talloc size accounting in memcache
       via  9ff5c0bab76 memcache: Increase size of default memcache to 512k
       via  a04ca6f3438 memcache: Properly track the size of talloc objects
       via  7c44f2f76ee memcache: Introduce struct for storing talloc pointer
      from  7a410ccb5f6 netcmd: Fix passwordsettings --max-pwd-age command

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit b7028c42462c34cf86cb949bfdb16ebc7ed0a6c6
Author: Christof Schmitt <cs at samba.org>
Date:   Thu Mar 28 10:46:43 2019 -0700

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Sat Apr  6 06:08:42 UTC 2019 on sn-devel-144

commit 9ff5c0bab76c5d3d7bea1fcb79861d0c9a3b9839
Author: Christof Schmitt <cs at samba.org>
Date:   Fri Apr 5 15:43:21 2019 -0700

    memcache: Increase size of default memcache to 512k
    
    With the fixed accounting of talloc objects, the default cache size
    needs to increase. The exact increase required depends on the workloads,
    going form 256k to 512k seems like a reasonable guess.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13865
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit a04ca6f3438595ba7e1a110877f53d1cac0f0402
Author: Christof Schmitt <cs at samba.org>
Date:   Mon Apr 1 16:23:35 2019 -0700

    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.
    
    Also record the initial size of the talloc object for proper adjustment
    of the used space in the cache later. This is in case the size of the
    talloc object is modified while being owned by the cache (e.g.
    allocating talloc child objects). This should never happen, but better
    be safe than ending up with a broken cache usage counter.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13865
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 7c44f2f76eefb9156cb1d170c92b4ff07dd6a3d5
Author: Christof Schmitt <cs at samba.org>
Date:   Mon Apr 1 15:38:59 2019 -0700

    memcache: Introduce struct for storing talloc pointer
    
    This allows extending the additional data stored for talloced objects
    later.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13865
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 docs-xml/smbdotconf/filename/maxstatcachesize.xml |  2 +-
 lib/param/loadparm.c                              |  2 +-
 lib/util/memcache.c                               | 54 ++++++++++++-----
 source3/param/loadparm.c                          |  2 +-
 source3/torture/torture.c                         | 70 ++++++++++++++++++++++-
 5 files changed, 111 insertions(+), 19 deletions(-)


Changeset truncated at 500 lines:

diff --git a/docs-xml/smbdotconf/filename/maxstatcachesize.xml b/docs-xml/smbdotconf/filename/maxstatcachesize.xml
index 63f91e70fd2..866d74d6ffd 100644
--- a/docs-xml/smbdotconf/filename/maxstatcachesize.xml
+++ b/docs-xml/smbdotconf/filename/maxstatcachesize.xml
@@ -13,6 +13,6 @@
 	</para>
 </description>
 <related>stat cache</related>
-<value type="default">256</value>
+<value type="default">512</value>
 <value type="example">100</value>
 </samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 7ef2cc7d3f6..ebbccc22b71 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2945,7 +2945,7 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 
 	lpcfg_do_global_parameter(lp_ctx, "durable handles", "yes");
 
-	lpcfg_do_global_parameter(lp_ctx, "max stat cache size", "256");
+	lpcfg_do_global_parameter(lp_ctx, "max stat cache size", "512");
 
 	lpcfg_do_global_parameter(lp_ctx, "ldap passwd sync", "no");
 
diff --git a/lib/util/memcache.c b/lib/util/memcache.c
index 819ba44b443..1e616bd0e9a 100644
--- a/lib/util/memcache.c
+++ b/lib/util/memcache.c
@@ -27,6 +27,11 @@
 
 static struct memcache *global_cache;
 
+struct memcache_talloc_value {
+	void *ptr;
+	size_t len;
+};
+
 struct memcache_element {
 	struct rb_node rb_node;
 	struct memcache_element *prev, *next;
@@ -180,19 +185,19 @@ void *memcache_lookup_talloc(struct memcache *cache, enum memcache_number n,
 			     DATA_BLOB key)
 {
 	DATA_BLOB value;
-	void *result;
+	struct memcache_talloc_value mtv;
 
 	if (!memcache_lookup(cache, n, key, &value)) {
 		return NULL;
 	}
 
-	if (value.length != sizeof(result)) {
+	if (value.length != sizeof(mtv)) {
 		return NULL;
 	}
 
-	memcpy(&result, value.data, sizeof(result));
+	memcpy(&mtv, value.data, sizeof(mtv));
 
-	return result;
+	return mtv.ptr;
 }
 
 static void memcache_delete_element(struct memcache *cache,
@@ -204,12 +209,13 @@ static void memcache_delete_element(struct memcache *cache,
 
 	if (memcache_is_talloc(e->n)) {
 		DATA_BLOB cache_key, cache_value;
-		void *ptr;
+		struct memcache_talloc_value mtv;
 
 		memcache_element_parse(e, &cache_key, &cache_value);
-		SMB_ASSERT(cache_value.length == sizeof(ptr));
-		memcpy(&ptr, cache_value.data, sizeof(ptr));
-		TALLOC_FREE(ptr);
+		SMB_ASSERT(cache_value.length == sizeof(mtv));
+		memcpy(&mtv, cache_value.data, sizeof(mtv));
+		cache->size -= mtv.len;
+		TALLOC_FREE(mtv.ptr);
 	}
 
 	cache->size -= memcache_element_size(e->keylength, e->valuelength);
@@ -275,16 +281,26 @@ void memcache_add(struct memcache *cache, enum memcache_number n,
 
 		if (value.length <= cache_value.length) {
 			if (memcache_is_talloc(e->n)) {
-				void *ptr;
-				SMB_ASSERT(cache_value.length == sizeof(ptr));
-				memcpy(&ptr, cache_value.data, sizeof(ptr));
-				TALLOC_FREE(ptr);
+				struct memcache_talloc_value mtv;
+
+				SMB_ASSERT(cache_value.length == sizeof(mtv));
+				memcpy(&mtv, cache_value.data, sizeof(mtv));
+				cache->size -= mtv.len;
+				TALLOC_FREE(mtv.ptr);
 			}
 			/*
 			 * We can reuse the existing record
 			 */
 			memcpy(cache_value.data, value.data, value.length);
 			e->valuelength = value.length;
+
+			if (memcache_is_talloc(e->n)) {
+				struct memcache_talloc_value mtv;
+
+				SMB_ASSERT(cache_value.length == sizeof(mtv));
+				memcpy(&mtv, cache_value.data, sizeof(mtv));
+				cache->size += mtv.len;
+			}
 			return;
 		}
 
@@ -328,14 +344,21 @@ void memcache_add(struct memcache *cache, enum memcache_number n,
 	DLIST_ADD(cache->mru, e);
 
 	cache->size += element_size;
+	if (memcache_is_talloc(e->n)) {
+		struct memcache_talloc_value mtv;
+
+		SMB_ASSERT(cache_value.length == sizeof(mtv));
+		memcpy(&mtv, cache_value.data, sizeof(mtv));
+		cache->size += mtv.len;
+	}
 	memcache_trim(cache);
 }
 
 void memcache_add_talloc(struct memcache *cache, enum memcache_number n,
 			 DATA_BLOB key, void *pptr)
 {
+	struct memcache_talloc_value mtv;
 	void **ptr = (void **)pptr;
-	void *p;
 
 	if (cache == NULL) {
 		cache = global_cache;
@@ -344,8 +367,9 @@ void memcache_add_talloc(struct memcache *cache, enum memcache_number n,
 		return;
 	}
 
-	p = talloc_move(cache, ptr);
-	memcache_add(cache, n, key, data_blob_const(&p, sizeof(p)));
+	mtv.len = talloc_total_size(*ptr);
+	mtv.ptr = talloc_move(cache, ptr);
+	memcache_add(cache, n, key, data_blob_const(&mtv, sizeof(mtv)));
 }
 
 void memcache_flush(struct memcache *cache, enum memcache_number n)
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 6aa84018111..703460e4c47 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -698,7 +698,7 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
 	Globals.nt_status_support = true; /* Use NT status by default. */
 	Globals.smbd_profiling_level = 0;
 	Globals.stat_cache = true;	/* use stat cache by default */
-	Globals.max_stat_cache_size = 256; /* 256k by default */
+	Globals.max_stat_cache_size = 512; /* 512k by default */
 	Globals.restrict_anonymous = 0;
 	Globals.client_lanman_auth = false;	/* Do NOT use the LanMan hash if it is available */
 	Globals.client_plaintext_auth = false;	/* Do NOT use a plaintext password even if is requested by the server */
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);
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list