[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Wed Dec 15 01:04:01 UTC 2021


The branch, master has been updated
       via  5b526f4533b tdb: Raw performance torture to beat tdb_increment_seqnum
       via  b9f06ab3472 tdb: Use atomic operations for tdb_[increment|get]_seqnum
       via  62dab3921b3 configure: Check for __atomic_add_fetch() and __atomic_load()
      from  1dc803048f8 lib/util: Add signal.h include

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


- Log -----------------------------------------------------------------
commit 5b526f4533bda42b51326c3b60fd771bc1cd88e7
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Dec 13 17:49:51 2021 +0100

    tdb: Raw performance torture to beat tdb_increment_seqnum
    
    Running this on sn-devel-184 takes ~14 seconds with the atomic
    ops. Without them I did not wait for it to finish. After reducing
    NPROCS from 500 to 50 it still ran for more than a minute.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Wed Dec 15 01:03:56 UTC 2021 on sn-devel-184

commit b9f06ab3472352d064082a44f7d5077c8c13931c
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Dec 13 17:42:12 2021 +0100

    tdb: Use atomic operations for tdb_[increment|get]_seqnum
    
    With locking.tdb now based on g_lock.c code, we change locking.tdb a
    lot more often. I have a customer case where LDX tortures smbd very
    hard with 800+ concurrent connections, which now completely falls over
    where 4.12 still worked fine. Some debugging showed a thundering herd
    on fcntl locking.tdb index 48 (TDB_SEQNUM_OFS). We still use fcntl for
    the seqnum, back when we converted the chainlocks to mutexes we did
    not consider it to be a problem. Now it is, but all we need to do with
    the SEQNUM is to increment it, so an __atomic_add_fetch() of one is
    sufficient.
    
    I've taken a look at the C11 standard atomics, but I could not figure
    out how to use them properly, to me they seem more general to be
    initialized first etc. All we need is a X86 "lock incl 48(%rax)" to be
    emitted, and the gcc __atomic_add_fetch seems to do this.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 62dab3921b335d47a0c9c419714d0e2ea2320f74
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Dec 13 17:40:52 2021 +0100

    configure: Check for __atomic_add_fetch() and __atomic_load()
    
    To be used in the tdb_seqnum code soon
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 lib/replace/wscript        |  16 ++++++
 lib/tdb/common/tdb.c       |  24 +++++++++
 lib/tdb/tools/tdbtortseq.c | 123 +++++++++++++++++++++++++++++++++++++++++++++
 lib/tdb/wscript            |   5 ++
 4 files changed, 168 insertions(+)
 create mode 100644 lib/tdb/tools/tdbtortseq.c


Changeset truncated at 500 lines:

diff --git a/lib/replace/wscript b/lib/replace/wscript
index a928b80f2f7..e60ff15f903 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -313,6 +313,22 @@ def configure(conf):
                     headers='stdint.h sys/atomic.h',
                     msg='Checking for atomic_add_32 compiler builtin')
 
+    conf.CHECK_CODE('''
+                    uint32_t i,j;
+                    j = __atomic_add_fetch(&i,1,__ATOMIC_SEQ_CST)
+                    ''',
+                    'HAVE___ATOMIC_ADD_FETCH',
+                    headers='stdint.h',
+                    msg='Checking for __atomic_add_fetch compiler builtin')
+
+    conf.CHECK_CODE('''
+                    uint32_t i,j;
+                    __atomic_load(&i,&j,__ATOMIC_SEQ_CST)
+                    ''',
+                    'HAVE___ATOMIC_ADD_LOAD',
+                    headers='stdint.h',
+                    msg='Checking for __atomic_load compiler builtin')
+
     # Check for thread fence. */
     tf = conf.CHECK_CODE('atomic_thread_fence(memory_order_seq_cst);',
                          'HAVE_ATOMIC_THREAD_FENCE',
diff --git a/lib/tdb/common/tdb.c b/lib/tdb/common/tdb.c
index c56b37be5ca..de829bb48c4 100644
--- a/lib/tdb/common/tdb.c
+++ b/lib/tdb/common/tdb.c
@@ -64,6 +64,15 @@ static void tdb_increment_seqnum(struct tdb_context *tdb)
 		return;
 	}
 
+#if defined(HAVE___ATOMIC_ADD_FETCH) && defined(HAVE___ATOMIC_ADD_LOAD)
+	if (tdb->map_ptr != NULL) {
+		uint32_t *pseqnum = (uint32_t *)(
+			TDB_SEQNUM_OFS + (char *)tdb->map_ptr);
+		__atomic_add_fetch(pseqnum, 1, __ATOMIC_SEQ_CST);
+		return;
+	}
+#endif
+
 	if (tdb_nest_lock(tdb, TDB_SEQNUM_OFS, F_WRLCK,
 			  TDB_LOCK_WAIT|TDB_LOCK_PROBE) != 0) {
 		return;
@@ -838,6 +847,21 @@ _PUBLIC_ int tdb_get_seqnum(struct tdb_context *tdb)
 {
 	tdb_off_t seqnum=0;
 
+	if (tdb->transaction != NULL) {
+		tdb_ofs_read(tdb, TDB_SEQNUM_OFS, &seqnum);
+		return seqnum;
+	}
+
+#if defined(HAVE___ATOMIC_ADD_FETCH) && defined(HAVE___ATOMIC_ADD_LOAD)
+	if (tdb->map_ptr != NULL) {
+		uint32_t *pseqnum = (uint32_t *)(
+			TDB_SEQNUM_OFS + (char *)tdb->map_ptr);
+		uint32_t ret;
+		__atomic_load(pseqnum, &ret,__ATOMIC_SEQ_CST);
+		return ret;
+	}
+#endif
+
 	tdb_ofs_read(tdb, TDB_SEQNUM_OFS, &seqnum);
 	return seqnum;
 }
diff --git a/lib/tdb/tools/tdbtortseq.c b/lib/tdb/tools/tdbtortseq.c
new file mode 100644
index 00000000000..9423b46d996
--- /dev/null
+++ b/lib/tdb/tools/tdbtortseq.c
@@ -0,0 +1,123 @@
+/*
+ * Unix SMB/CIFS implementation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "replace.h"
+#include "system/filesys.h"
+#include <tdb.h>
+#include <stdio.h>
+#include <errno.h>
+#include "system/wait.h"
+
+#define NPROCS 500
+#define NUMOPS 1000000
+
+int main(void)
+{
+	pid_t pids[NPROCS];
+	struct tdb_context *tdb = NULL;
+	int i;
+	uint32_t seqnum_before, seqnum_after;
+
+	tdb = tdb_open("seqnum_test.tdb",
+		       10000,
+		       TDB_CLEAR_IF_FIRST|
+		       TDB_SEQNUM|
+		       TDB_INCOMPATIBLE_HASH|
+		       TDB_MUTEX_LOCKING,
+		       O_CREAT|O_RDWR,
+		       0644);
+	if (tdb == NULL) {
+		perror("tdb_open failed");
+		return 1;
+	}
+	seqnum_before = tdb_get_seqnum(tdb);
+
+	for (i=0; i<NPROCS; i++) {
+		pids[i] = fork();
+		if (pids[i] == -1) {
+			perror("fork failed");
+			return 1;
+		}
+		if (pids[i] == 0) {
+			pid_t mypid = getpid();
+			int ret;
+			int j;
+
+			ret = tdb_reopen(tdb);
+			if (ret != 0) {
+				perror("tdb_reopen failed");
+				return 1;
+			}
+
+			for (j=0; j<NUMOPS; j++) {
+				TDB_DATA key = {
+					.dptr = (uint8_t *)&mypid,
+					.dsize = sizeof(mypid),
+				};
+				TDB_DATA value = {
+					.dptr = (uint8_t *)&j,
+					.dsize = sizeof(j),
+				};
+				ret = tdb_store(tdb, key, value, 0);
+				if (ret == -1) {
+					perror("tdb_store failed");
+					return 1;
+				}
+			}
+
+			return 0;
+		}
+	}
+
+	for (i=0; i<NPROCS; i++) {
+		int wstatus;
+		pid_t ret = waitpid(pids[i], &wstatus, 0);
+
+		if (ret == -1) {
+			perror("waitpid failed");
+			return 1;
+		}
+
+		if (!WIFEXITED(wstatus)) {
+			fprintf(stderr,
+				"pid %d did not exit properly\n",
+				(int)pids[i]);
+			return 1;
+		}
+
+		if (WEXITSTATUS(wstatus) != 0) {
+			fprintf(stderr,
+				"pid %d returned %d\n",
+				(int)pids[i],
+				WEXITSTATUS(wstatus));
+			return 1;
+		}
+	}
+
+	seqnum_after = tdb_get_seqnum(tdb);
+
+	printf("seqnum_before=%"PRIu32", seqnum_after=%"PRIu32"\n",
+	       seqnum_before,
+	       seqnum_after);
+
+	if ((seqnum_after - seqnum_before) != (NPROCS*NUMOPS)) {
+		perror("incrementing seqnum failed");
+		return 1;
+	}
+
+	return 0;
+}
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 19b256f037c..81132dc3276 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -145,6 +145,11 @@ def build(bld):
                          'tdb',
                          install=False)
 
+        bld.SAMBA_BINARY('tdbtortseq',
+                         'tools/tdbtortseq.c',
+                         'tdb',
+                         install=False)
+
         bld.SAMBA_BINARY('tdbrestore',
                          'tools/tdbrestore.c',
                          'tdb', manpages='man/tdbrestore.8')


-- 
Samba Shared Repository



More information about the samba-cvs mailing list