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 <v...@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 <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Wed Dec 15 01:03:56 UTC 2021 on sn-devel-184 commit b9f06ab3472352d064082a44f7d5077c8c13931c Author: Volker Lendecke <v...@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 <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 62dab3921b335d47a0c9c419714d0e2ea2320f74 Author: Volker Lendecke <v...@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 <v...@samba.org> Reviewed-by: Jeremy Allison <j...@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