From 4d22f52f640f5475f0a83fbc163900135d353cca Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 10 Nov 2023 21:33:53 +0100 Subject: [PATCH] Locking data structures If a data structure is associated with a lock, having a public and a private part, there are now useful macros for these data structures. --- configure.ac | 1 + lib/Makefile | 2 +- lib/locking.h | 201 +++++++++++++++++++++++++++++++++++++++++++++ lib/locking_test.c | 91 ++++++++++++++++++++ 4 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 lib/locking_test.c diff --git a/configure.ac b/configure.ac index c71b5eab..fc2ba940 100644 --- a/configure.ac +++ b/configure.ac @@ -147,6 +147,7 @@ if test "$bird_cflags_default" = yes ; then BIRD_CHECK_GCC_OPTION([bird_cv_c_option_wno_pointer_sign], [-Wno-pointer-sign], [-Wall]) BIRD_CHECK_GCC_OPTION([bird_cv_c_option_wno_missing_init], [-Wno-missing-field-initializers], [-Wall -Wextra]) + if test "$enable_debug" = no; then BIRD_CHECK_LTO fi diff --git a/lib/Makefile b/lib/Makefile index 0762b606..cd40bade 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -2,6 +2,6 @@ src := a-path.c a-set.c bitmap.c bitops.c blake2s.c blake2b.c checksum.c event.c obj := $(src-o-files) $(all-daemon) -tests_src := a-set_test.c a-path_test.c attribute_cleanup_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c tlists_test.c type_test.c +tests_src := a-set_test.c a-path_test.c attribute_cleanup_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c locking_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c tlists_test.c type_test.c tests_targets := $(tests_targets) $(tests-target-files) tests_objs := $(tests_objs) $(src-o-files) diff --git a/lib/locking.h b/lib/locking.h index fbc8fd8b..6cec2ff3 100644 --- a/lib/locking.h +++ b/lib/locking.h @@ -72,4 +72,205 @@ extern DOMAIN(the_bird) the_bird_domain; #define ASSERT_THE_BIRD_LOCKED ({ if (!the_bird_locked()) bug("The BIRD lock must be locked here: %s:%d", __FILE__, __LINE__); }) +/** + * Objects bound with domains + * + * First, we need some object to have its locked and unlocked part. + * This is accomplished typically by the following pattern: + * + * struct foo_public { + * ... // Public fields + * DOMAIN(bar) lock; // The assigned domain + * }; + * + * struct foo_private { + * struct foo_public; // Importing public fields + * struct foo_private **locked_at; // Auxiliary field for locking routines + * ... // Private fields + * }; + * + * typedef union foo { + * struct foo_public; + * struct foo_private priv; + * } foo; + * + * All persistently stored object pointers MUST point to the public parts. + * If accessing the locked object from embedded objects, great care must + * be applied to always SKIP_BACK to the public object version, not the + * private one. + * + * To access the private object parts, either the private object pointer + * is explicitly given to us, therefore assuming somewhere else the domain + * has been locked, or we have to lock the domain ourselves. To do that, + * there are some handy macros. + */ + +#define LOBJ_LOCK_SIMPLE(_obj, _level) \ + ({ LOCK_DOMAIN(_level, (_obj)->lock); &(_obj)->priv; }) + +#define LOBJ_UNLOCK_SIMPLE(_obj, _level) \ + UNLOCK_DOMAIN(_level, (_obj)->lock) + +/* + * These macros can be used to define specific macros for given class. + * + * #define FOO_LOCK_SIMPLE(foo) LOBJ_LOCK_SIMPLE(foo, bar) + * #define FOO_UNLOCK_SIMPLE(foo) LOBJ_UNLOCK_SIMPLE(foo, bar) + * + * Then these can be used like this: + * + * void foo_frobnicate(foo *f) + * { + * // Unlocked context + * ... + * struct foo_private *fp = FOO_LOCK_SIMPLE(f); + * // Locked context + * ... + * FOO_UNLOCK_SIMPLE(f); + * // Unlocked context + * ... + * } + * + * These simple calls have two major drawbacks. First, if you return + * from locked context, you don't unlock, which may lock you dead. + * And second, the foo_private pointer is still syntactically valid + * even after unlocking. + * + * To fight this, we need more magic and the switch should stay in that + * position. + * + * First, we need an auxiliary _function_ for unlocking. This function + * is intended to be called in a local variable cleanup context. + */ + +#define LOBJ_UNLOCK_CLEANUP_NAME(_stem) _lobj__##_stem##_unlock_cleanup + +#define LOBJ_UNLOCK_CLEANUP(_stem, _level) \ + static inline void LOBJ_UNLOCK_CLEANUP_NAME(_stem)(struct _stem##_private **obj) { \ + if (!*obj) return; \ + ASSERT_DIE(LOBJ_IS_LOCKED((*obj), _level)); \ + ASSERT_DIE((*obj)->locked_at == obj); \ + (*obj)->locked_at = NULL; \ + UNLOCK_DOMAIN(_level, (*obj)->lock); \ + } + +#define LOBJ_LOCK(_obj, _pobj, _stem, _level) \ + CLEANUP(LOBJ_UNLOCK_CLEANUP_NAME(_stem)) struct _stem##_private *_pobj = LOBJ_LOCK_SIMPLE(_obj, _level); _pobj->locked_at = &_pobj; + +/* + * And now the usage of these macros. You first need to declare the auxiliary + * cleanup function. + * + * LOBJ_UNLOCK_CLEANUP(foo, bar); + * + * And then declare the lock-local macro: + * + * #define FOO_LOCK(foo, fpp) LOBJ_LOCK(foo, fpp, foo, bar) + * + * This construction then allows you to lock much more safely: + * + * void foo_frobnicate_safer(foo *f) + * { + * // Unlocked context + * ... + * do { + * FOO_LOCK(foo, fpp); + * // Locked context, fpp is valid here + * + * if (something) return; // This implicitly unlocks + * if (whatever) break; // This unlocks too + * + * // Finishing context with no unlock at all + * } while (0); + * + * // Here is fpp invalid and the object is back unlocked. + * ... + * } + * + * There is no explicit unlock statement. To unlock, simply leave the block + * with locked context. + * + * This may be made even nicer to use by employing a for-cycle. + */ + +#define LOBJ_LOCKED(_obj, _pobj, _stem, _level) \ + for (CLEANUP(LOBJ_UNLOCK_CLEANUP_NAME(_stem)) struct _stem##_private *_pobj = LOBJ_LOCK_SIMPLE(_obj, _level); \ + _pobj ? (_pobj->locked_at = &_pobj) : NULL; \ + LOBJ_UNLOCK_CLEANUP_NAME(_stem)(&_pobj), _pobj = NULL) + +/* + * This for-cycle employs heavy magic to hide as much of the boilerplate + * from the user as possibly needed. Here is how it works. + * + * First, the for-1 clause is executed, setting up _pobj, to the private + * object pointer. It has a cleanup hook set. + * + * Then, the for-2 clause is checked. As _pobj is non-NULL, _pobj->locked_at + * is initialized to the _pobj address to ensure that the cleanup hook unlocks + * the right object. + * + * Now the user block is executed. If it ends by break or return, the cleanup + * hook fires for _pobj, triggering object unlock. + * + * If the user block executed completely, the for-3 clause is run, executing + * the cleanup hook directly and then deactivating it by setting _pobj to NULL. + * + * Finally, the for-2 clause is checked again but now with _pobj being NULL, + * causing the loop to end. As the object has already been unlocked, nothing + * happens after leaving the context. + * + * #define FOO_LOCKED(foo, fpp) LOBJ_LOCKED(foo, fpp, foo, bar) + * + * Then the previous code can be modified like this: + * + * void foo_frobnicate_safer(foo *f) + * { + * // Unlocked context + * ... + * FOO_LOCKED(foo, fpp) + * { + * // Locked context, fpp is valid here + * + * if (something) return; // This implicitly unlocks + * if (whatever) break; // This unlocks too + * + * // Finishing context with no unlock at all + * } + * + * // Unlocked context + * ... + * + * // Locking once again without an explicit block + * FOO_LOCKED(foo, fpp) + * do_something(fpp); + * + * // Here is fpp invalid and the object is back unlocked. + * ... + * } + * + * + * For many reasons, a lock-check macro is handy. + * + * #define FOO_IS_LOCKED(foo) LOBJ_IS_LOCKED(foo, bar) + */ + +#define LOBJ_IS_LOCKED(_obj, _level) DOMAIN_IS_LOCKED(_level, (_obj)->lock) + +/* + * An example implementation is available in lib/locking_test.c + */ + + +/* + * Please don't use this macro unless you at least try to prove that + * it's completely safe. It's a can of worms. + * + * NEVER RETURN OR BREAK FROM THIS MACRO, it will crash. + */ + +#define LOBJ_UNLOCKED_TEMPORARILY(_obj, _pobj, _stem, _level) \ + for (union _stem *_obj = SKIP_BACK(union _stem, priv, _pobj), **_lataux = (union _stem **) _pobj->locked_at; \ + _obj ? (_pobj->locked_at = NULL, LOBJ_UNLOCK_SIMPLE(_obj, _level), _obj) : NULL; \ + LOBJ_LOCK_SIMPLE(_obj, _level), _pobj->locked_at = (struct _stem##_private **) _lataux, _obj = NULL) + #endif diff --git a/lib/locking_test.c b/lib/locking_test.c new file mode 100644 index 00000000..0d99b189 --- /dev/null +++ b/lib/locking_test.c @@ -0,0 +1,91 @@ +#include "test/birdtest.h" +#include "test/bt-utils.h" + +#include "lib/locking.h" +#include +#include + +DEFINE_DOMAIN(proto); + +#define FOO_PUBLIC \ + const char *name; \ + _Atomic uint counter; \ + DOMAIN(proto) lock; \ + +struct foo_private { + struct { FOO_PUBLIC; }; + struct foo_private **locked_at; + uint private_counter; +}; + +typedef union foo { + struct { FOO_PUBLIC; }; + struct foo_private priv; +} foo; + +LOBJ_UNLOCK_CLEANUP(foo, proto); +#define FOO_LOCK(_foo, _fpp) LOBJ_LOCK(_foo, _fpp, foo, proto) +#define FOO_LOCKED(_foo, _fpp) LOBJ_LOCKED(_foo, _fpp, foo, proto) +#define FOO_IS_LOCKED(_foo) LOBJ_IS_LOCKED(_foo, proto) + +static uint +inc_public(foo *f) +{ + return atomic_fetch_add_explicit(&f->counter, 1, memory_order_relaxed) + 1; +} + +static uint +inc_private(foo *f) +{ + FOO_LOCKED(f, fp) return ++fp->private_counter; + bug("Returning always"); +} + +#define BLOCKCOUNT 4096 +#define THREADS 16 +#define REPEATS 128 + +static void * +thread_run(void *_foo) +{ + foo *f = _foo; + + for (int i=0; i