From 5ea964a2507e4491b0039a36dfa63e1627946ab2 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Wed, 18 Apr 2018 14:17:39 +0000 Subject: [PATCH 1/6] Lockhammer: improve command line arg parsing Convert argument parsing from only default/full specification to incremental flags and parameters. Additionally allow for test- specific arguments to be provided on the command line following "--". Fixes #7 Change-Id: Ia97684b6b6df72f2fb8f057a73182aae31f6276f Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/include/atomics.h | 3 + benchmarks/lockhammer/scripts/sweep.sh | 2 +- benchmarks/lockhammer/src/lockhammer.c | 138 +++++++++++++++--------- 3 files changed, 94 insertions(+), 49 deletions(-) diff --git a/benchmarks/lockhammer/include/atomics.h b/benchmarks/lockhammer/include/atomics.h index 3a8d6fb..9dafe24 100644 --- a/benchmarks/lockhammer/include/atomics.h +++ b/benchmarks/lockhammer/include/atomics.h @@ -34,6 +34,9 @@ #ifndef initialize_lock #define initialize_lock(lock, thread) #endif +#ifndef parse_test_args + #define parse_test_args(args, argc, argv) +#endif static inline void spin_wait (unsigned long wait_iter) { #if defined(__aarch64__) diff --git a/benchmarks/lockhammer/scripts/sweep.sh b/benchmarks/lockhammer/scripts/sweep.sh index a61f752..f90d534 100755 --- a/benchmarks/lockhammer/scripts/sweep.sh +++ b/benchmarks/lockhammer/scripts/sweep.sh @@ -48,7 +48,7 @@ do fi echo Test: ${1} CPU: exectx=$c Date: `date` 1>&2 - sudo ../build/lh_${1} $c ${acquires} ${2} ${3} + sudo ../build/lh_${1} -t $c -a ${acquires} -c ${2} -p ${3} sleep 5s fi done diff --git a/benchmarks/lockhammer/src/lockhammer.c b/benchmarks/lockhammer/src/lockhammer.c index c0f404e..d35752b 100644 --- a/benchmarks/lockhammer/src/lockhammer.c +++ b/benchmarks/lockhammer/src/lockhammer.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include ATOMIC_TEST @@ -46,7 +47,7 @@ uint64_t test_lock = 0; uint64_t sync_lock = 0; uint64_t ready_lock = 0; -struct arg { +struct thread_args { unsigned long ncores; unsigned long nthrds; unsigned long iter; @@ -57,18 +58,31 @@ struct arg { unsigned long *nstart; unsigned long hold, post; }; -typedef struct arg arg; +typedef struct thread_args thread_args; + +struct test_args { + unsigned long nthrds; + unsigned long nacqrs; + unsigned long ncrit; + unsigned long nparallel; +}; +typedef struct test_args test_args; void* hmr(void *); +void print_usage (char *invoc) { + fprintf(stderr, +"Usage: %s\n\t[-t threads]\n\t[-a acquires per thread]\n\t" \ +"[-c critical iterations]\n\t[-p parallelizable iterations]\n\t" \ +"[-- ]\n", invoc); +} + int main(int argc, char** argv) { struct sched_param sparam; unsigned long i; - unsigned long num_cores, num_threads; - unsigned long locks_per_thread; - unsigned long lock_hold_work, non_lock_work; + unsigned long num_cores; unsigned long result; unsigned long sched_elapsed = 0, real_elapsed = 0; unsigned long start_ns = 0; @@ -76,35 +90,62 @@ int main(int argc, char** argv) num_cores = sysconf(_SC_NPROCESSORS_ONLN); - if (argc == 1) { - num_threads = num_cores; - locks_per_thread = 50000; - lock_hold_work = 0; - non_lock_work = 0; - } - else if (argc == 5) { - num_threads = atoi(argv[1]); - /* Do not allow number of threads to exceed online cores - in order to prevent deadlock ... */ - num_threads = num_threads > num_cores ? num_cores : num_threads; - locks_per_thread = atoi(argv[2]); - lock_hold_work = atoi(argv[3]); - non_lock_work = atoi(argv[4]); - } - else { - fprintf(stderr, "Usage: %s [ ]\n", argv[0]); - return 1; + /* Set defaults for all command line options */ + test_args args = { .nthrds = num_cores, + .nacqrs = 50000, + .ncrit = 0, + .nparallel = 0 }; + + for (i = 1; i < argc; i++) { + if (strlen(argv[i]) == 2 && argv[i][0] == '-' && i < argc) { + switch (argv[i][1]) { + case 't': + i++; + args.nthrds = strtol(argv[i], (char **) NULL, 10); + /* Do not allow number of threads to exceed online cores + in order to prevent deadlock ... */ + args.nthrds > num_cores ? num_cores : args.nthrds; + break; + case 'a': + i++; + args.nacqrs = strtol(argv[i], (char **) NULL, 10); + break; + case 'c': + i++; + args.ncrit = strtol(argv[i], (char **) NULL, 10); + break; + case 'p': + i++; + args.nparallel = strtol(argv[i], (char **) NULL, 10); + break; + case '-': + /* anything after "--" is ignore and passed through to test */ + i++; + parse_test_args(args, argc - i, &argv[i]); + /* skip over remaining args since they are parsed by test */ + i = argc; + break; + default: + print_usage(argv[0]); + } + if (errno == EINVAL) { + print_usage(argv[0]); + return 1; + } + } + else { + print_usage(argv[0]); + return 1; + } } - pthread_t hmr_threads[num_threads]; + pthread_t hmr_threads[args.nthrds]; pthread_attr_t hmr_attr; - unsigned long hmrs[num_threads]; - unsigned long hmrtime[num_threads]; /* can't touch this */ - unsigned long hmrdepth[num_threads]; + unsigned long hmrs[args.nthrds]; + unsigned long hmrtime[args.nthrds]; /* can't touch this */ + unsigned long hmrdepth[args.nthrds]; struct timespec tv_time; - for (i = 0; i < num_threads; ++i) hmrs[i] = 0; - /* Select the FIFO scheduler. This prevents interruption of the lockhammer test threads allowing for more precise measuremnet of lock acquisition rate, especially for mutex type locks where @@ -128,23 +169,24 @@ int main(int argc, char** argv) initialize_lock(&test_lock, num_cores); - arg args[num_threads]; - for (i = 0; i < num_threads; ++i) { - args[i].ncores = num_cores; - args[i].nthrds = num_threads; - args[i].iter = locks_per_thread; - args[i].lock = &test_lock; - args[i].rst = &hmrs[i]; - args[i].nsec = &hmrtime[i]; - args[i].depth = &hmrdepth[i]; - args[i].nstart = &start_ns; - args[i].hold = lock_hold_work; - args[i].post = non_lock_work; - - pthread_create(&hmr_threads[i], &hmr_attr, hmr, (void*)(&args[i])); + thread_args t_args[args.nthrds]; + for (i = 0; i < args.nthrds; ++i) { + hmrs[i] = 0; + t_args[i].ncores = num_cores; + t_args[i].nthrds = args.nthrds; + t_args[i].iter = args.nacqrs; + t_args[i].lock = &test_lock; + t_args[i].rst = &hmrs[i]; + t_args[i].nsec = &hmrtime[i]; + t_args[i].depth = &hmrdepth[i]; + t_args[i].nstart = &start_ns; + t_args[i].hold = args.ncrit; + t_args[i].post = args.nparallel; + + pthread_create(&hmr_threads[i], &hmr_attr, hmr, (void*)(&t_args[i])); } - for (i = 0; i < num_threads; ++i) { + for (i = 0; i < args.nthrds; ++i) { result = pthread_join(hmr_threads[i], NULL); } /* "Marshal" thread will collect start time once all threads have @@ -155,7 +197,7 @@ int main(int argc, char** argv) pthread_attr_destroy(&hmr_attr); result = 0; - for (i = 0; i < num_threads; ++i) { + for (i = 0; i < args.nthrds; ++i) { result += hmrs[i]; sched_elapsed += hmrtime[i]; /* Average lock "depth" is an algorithm-specific auxiliary metric @@ -164,7 +206,7 @@ int main(int argc, char** argv) call to lock_acquire and accumulated per-thread. These results are then aggregated and averaged here so that an overall view of the run's contention level can be determined. */ - avg_lock_depth += ((double) hmrdepth[i] / (double) hmrs[i]) / (double) num_threads; + avg_lock_depth += ((double) hmrdepth[i] / (double) hmrs[i]) / (double) args.nthrds; } fprintf(stderr, "%ld lock loops\n", result); @@ -175,7 +217,7 @@ int main(int argc, char** argv) fprintf(stderr, "%lf average depth\n", avg_lock_depth); printf("%ld, %f, %lf, %lf, %lf\n", - num_threads, + args.nthrds, ((float) sched_elapsed / (float) real_elapsed), ((double) sched_elapsed)/ ((double) result), ((double) real_elapsed) / ((double) result), @@ -185,7 +227,7 @@ int main(int argc, char** argv) void* hmr(void *ptr) { unsigned long nlocks = 0; - arg *x = (arg*)ptr; + thread_args *x = (thread_args*)ptr; int rval; unsigned long *lock = x->lock; unsigned long target_locks = x->iter; From 208a66bbfc0e5268d236d24d8a5b9891c040c097 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Mon, 23 Apr 2018 17:40:54 +0000 Subject: [PATCH 2/6] Lockhammer: move types to header file Move argument types to a header file in part to clean up main source file. This change also exposes the argument containing struct to the test so that the test-specific argument parser can utilize the values of the generic command line arguments. Change-Id: I2fb62e5afa18ccb85c7ee95d8803edf823a1d02f Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/include/lockhammer.h | 56 ++++++++++++++++++++++ benchmarks/lockhammer/src/lockhammer.c | 23 +-------- 2 files changed, 58 insertions(+), 21 deletions(-) create mode 100644 benchmarks/lockhammer/include/lockhammer.h diff --git a/benchmarks/lockhammer/include/lockhammer.h b/benchmarks/lockhammer/include/lockhammer.h new file mode 100644 index 0000000..7fc1963 --- /dev/null +++ b/benchmarks/lockhammer/include/lockhammer.h @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2017, The Linux Foundation. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * * Neither the name of The Linux Foundation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE + * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN + * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef __LOCKHAMMER_H__ +#define __LOCKHAMMER_H__ + +struct thread_args { + unsigned long ncores; + unsigned long nthrds; + unsigned long iter; + unsigned long *lock; + unsigned long *rst; + unsigned long *nsec; + unsigned long *depth; + unsigned long *nstart; + unsigned long hold, post; +}; +typedef struct thread_args thread_args; + +struct test_args { + unsigned long nthrds; + unsigned long nacqrs; + unsigned long ncrit; + unsigned long nparallel; +}; +typedef struct test_args test_args; + +#endif diff --git a/benchmarks/lockhammer/src/lockhammer.c b/benchmarks/lockhammer/src/lockhammer.c index d35752b..01209f9 100644 --- a/benchmarks/lockhammer/src/lockhammer.c +++ b/benchmarks/lockhammer/src/lockhammer.c @@ -41,33 +41,14 @@ #include #include +#include "lockhammer.h" + #include ATOMIC_TEST uint64_t test_lock = 0; uint64_t sync_lock = 0; uint64_t ready_lock = 0; -struct thread_args { - unsigned long ncores; - unsigned long nthrds; - unsigned long iter; - unsigned long *lock; - unsigned long *rst; - unsigned long *nsec; - unsigned long *depth; - unsigned long *nstart; - unsigned long hold, post; -}; -typedef struct thread_args thread_args; - -struct test_args { - unsigned long nthrds; - unsigned long nacqrs; - unsigned long ncrit; - unsigned long nparallel; -}; -typedef struct test_args test_args; - void* hmr(void *); void print_usage (char *invoc) { From 30238600de95633dcee3cc1b674cf8e496cf2651 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Tue, 24 Apr 2018 19:13:45 +0000 Subject: [PATCH 3/6] Lockhammer: fix default case for arg parsing Default (invalid arg) case now exits in addition to printing usage message. Change-Id: I73da89c7bcb0a3b4ee2a70404efdfb17616f4434 Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/src/lockhammer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/benchmarks/lockhammer/src/lockhammer.c b/benchmarks/lockhammer/src/lockhammer.c index 01209f9..27256af 100644 --- a/benchmarks/lockhammer/src/lockhammer.c +++ b/benchmarks/lockhammer/src/lockhammer.c @@ -108,6 +108,7 @@ int main(int argc, char** argv) break; default: print_usage(argv[0]); + return 1; } if (errno == EINVAL) { print_usage(argv[0]); From b1f469fb1a8d6afe5cd8c4ca298f05a1524c3337 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Thu, 3 May 2018 19:00:04 +0000 Subject: [PATCH 4/6] Lockhammer: Convert arg parsing to use getopt Use getopt for more robust argument parsing instead of reimplementing. Change-Id: If65277243d5ac32b71b9d8757dd3516673c184a7 Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/src/lockhammer.c | 69 ++++++++++---------------- 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/benchmarks/lockhammer/src/lockhammer.c b/benchmarks/lockhammer/src/lockhammer.c index 27256af..bfa2f65 100644 --- a/benchmarks/lockhammer/src/lockhammer.c +++ b/benchmarks/lockhammer/src/lockhammer.c @@ -53,9 +53,9 @@ void* hmr(void *); void print_usage (char *invoc) { fprintf(stderr, -"Usage: %s\n\t[-t threads]\n\t[-a acquires per thread]\n\t" \ -"[-c critical iterations]\n\t[-p parallelizable iterations]\n\t" \ -"[-- ]\n", invoc); + "Usage: %s\n\t[-t threads]\n\t[-a acquires per thread]\n\t" + "[-c critical iterations]\n\t[-p parallelizable iterations]\n\t" + "[-- ]\n", invoc); } int main(int argc, char** argv) @@ -77,50 +77,35 @@ int main(int argc, char** argv) .ncrit = 0, .nparallel = 0 }; - for (i = 1; i < argc; i++) { - if (strlen(argv[i]) == 2 && argv[i][0] == '-' && i < argc) { - switch (argv[i][1]) { - case 't': - i++; - args.nthrds = strtol(argv[i], (char **) NULL, 10); - /* Do not allow number of threads to exceed online cores - in order to prevent deadlock ... */ - args.nthrds > num_cores ? num_cores : args.nthrds; - break; - case 'a': - i++; - args.nacqrs = strtol(argv[i], (char **) NULL, 10); - break; - case 'c': - i++; - args.ncrit = strtol(argv[i], (char **) NULL, 10); - break; - case 'p': - i++; - args.nparallel = strtol(argv[i], (char **) NULL, 10); - break; - case '-': - /* anything after "--" is ignore and passed through to test */ - i++; - parse_test_args(args, argc - i, &argv[i]); - /* skip over remaining args since they are parsed by test */ - i = argc; - break; - default: - print_usage(argv[0]); - return 1; - } - if (errno == EINVAL) { - print_usage(argv[0]); - return 1; - } - } - else { + opterr = 0; + + while ((i = getopt(argc, argv, "t:a:c:p:")) != -1) + { + switch (i) { + case 't': + args.nthrds = strtol(optarg, (char **) NULL, 10); + /* Do not allow number of threads to exceed online cores + in order to prevent deadlock ... */ + args.nthrds > num_cores ? num_cores : args.nthrds; + break; + case 'a': + args.nacqrs = strtol(optarg, (char **) NULL, 10); + break; + case 'c': + args.ncrit = strtol(optarg, (char **) NULL, 10); + break; + case 'p': + args.nparallel = strtol(optarg, (char **) NULL, 10); + break; + case '?': + default: print_usage(argv[0]); return 1; } } + parse_test_args(args, argc - optind, &argv[optind]); + pthread_t hmr_threads[args.nthrds]; pthread_attr_t hmr_attr; unsigned long hmrs[args.nthrds]; From 7459726a65c19a1944b8aff0ef31b1635782a703 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Mon, 7 May 2018 16:23:25 +0000 Subject: [PATCH 5/6] Lockhammer: Build hybrid spinlocks for aarch64 only Because the hybrid spinlock requires LL/SC support in order to avoid touching the ticket value if queueing has already begun it cannot be implemented on x86_64 as designed. Only build the two hybrid spinlock tests if the target arch of the compiler is aarch64. Fixes #11 Change-Id: I33c79dbe1a81bc978f2029b2c13a902244415663 Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/Makefile | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/benchmarks/lockhammer/Makefile b/benchmarks/lockhammer/Makefile index 1ee06a4..513267d 100644 --- a/benchmarks/lockhammer/Makefile +++ b/benchmarks/lockhammer/Makefile @@ -6,20 +6,26 @@ LDFLAGS=-lpthread lh_%: tests/%.h include/atomics.h src/lockhammer.c ${CC} ${CFLAGS} -DATOMIC_TEST=\"$<\" src/lockhammer.c -o build/$@ ${LDFLAGS} -all: \ - lh_swap_mutex \ +TARGET_ARCH:=$(shell ${CC} -dumpmachine | cut -d '-' -f 1) + +TEST_TARGETS=lh_swap_mutex \ lh_event_mutex \ lh_cas_event_mutex \ lh_cas_lockref \ lh_cas_rw_lock \ lh_incdec_refcount \ lh_ticket_spinlock \ - lh_hybrid_spinlock \ - lh_hybrid_spinlock_fastdequeue \ lh_queued_spinlock \ lh_empty \ lh_jvm_objectmonitor +ifeq ($(TARGET_ARCH),aarch64) + TEST_TARGETS+=lh_hybrid_spinlock \ + lh_hybrid_spinlock_fastdequeue +endif + +all: ${TEST_TARGETS} + lh_event_mutex: ../../ext/mysql/event_mutex.h include/atomics.h ../../ext/mysql/include/ut_atomics.h src/lockhammer.c ${CC} ${CFLAGS} -DATOMIC_TEST=\"$<\" src/lockhammer.c -o build/$@ ${LDFLAGS} From 7ec51d5ce178bbb8bbd8699b028e454dcbcfa286 Mon Sep 17 00:00:00 2001 From: Lucas Crowthers Date: Mon, 7 May 2018 16:44:04 +0000 Subject: [PATCH 6/6] Lockhammer: Sanitize command-line arguments Refuse to run if any command-line arguments are negative. Issue a warning if requested thread count exceeds online core count but contiue to run with thread count limited to online core count. Change-Id: I36a02e0909e851f8c5c6483b953d0232eab37679 Signed-off-by: Lucas Crowthers --- benchmarks/lockhammer/src/lockhammer.c | 41 ++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/benchmarks/lockhammer/src/lockhammer.c b/benchmarks/lockhammer/src/lockhammer.c index bfa2f65..cb8a8ba 100644 --- a/benchmarks/lockhammer/src/lockhammer.c +++ b/benchmarks/lockhammer/src/lockhammer.c @@ -81,21 +81,52 @@ int main(int argc, char** argv) while ((i = getopt(argc, argv, "t:a:c:p:")) != -1) { + long optval = 0; switch (i) { case 't': - args.nthrds = strtol(optarg, (char **) NULL, 10); + optval = strtol(optarg, (char **) NULL, 10); /* Do not allow number of threads to exceed online cores in order to prevent deadlock ... */ - args.nthrds > num_cores ? num_cores : args.nthrds; + if (optval < 0) { + fprintf(stderr, "ERROR: thread count must be positive.\n"); + return 1; + } + else if (optval <= num_cores) { + args.nthrds = optval; + } + else { + fprintf(stderr, "WARNING: limiting thread count to online cores (%d).\n", num_cores); + } break; case 'a': - args.nacqrs = strtol(optarg, (char **) NULL, 10); + optval = strtol(optarg, (char **) NULL, 10); + if (optval < 0) { + fprintf(stderr, "ERROR: acquire count must be positive.\n"); + return 1; + } + else { + args.nacqrs = optval; + } break; case 'c': - args.ncrit = strtol(optarg, (char **) NULL, 10); + optval = strtol(optarg, (char **) NULL, 10); + if (optval < 0) { + fprintf(stderr, "ERROR: critical iteration count must be positive.\n"); + return 1; + } + else { + args.ncrit = optval; + } break; case 'p': - args.nparallel = strtol(optarg, (char **) NULL, 10); + optval = strtol(optarg, (char **) NULL, 10); + if (optval < 0) { + fprintf(stderr, "ERROR: parallel iteration count must be positive.\n"); + return 1; + } + else { + args.nparallel = optval; + } break; case '?': default: