From 6eea722d3f3a3ff6176eba4b8416b50f67047b8e Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 27 Jun 2024 08:42:11 +0200 Subject: [PATCH] Forbid locking altogether when RCU reader is active --- lib/locking.h | 5 ++--- lib/netindex.c | 2 +- lib/rcu.c | 4 ---- lib/rcu.h | 1 - nest/rt-attr.c | 2 +- nest/rt-table.c | 4 ++-- proto/bgp/attrs.c | 2 +- sysdep/unix/alloc.c | 10 ++++++++++ sysdep/unix/domain.c | 20 ++++++-------------- sysdep/unix/io-loop.c | 2 +- 10 files changed, 24 insertions(+), 28 deletions(-) diff --git a/lib/locking.h b/lib/locking.h index 0251f876..60928ecc 100644 --- a/lib/locking.h +++ b/lib/locking.h @@ -44,9 +44,8 @@ extern _Thread_local struct domain_generic **last_locked; #define DOMAIN(type) struct domain__##type #define DOMAIN_ORDER(type) OFFSETOF(struct lock_order, type) -#define DOMAIN_NEW(type) (DOMAIN(type)) { .type = domain_new(DOMAIN_ORDER(type), 1) } -#define DOMAIN_NEW_RCU_SYNC(type) (DOMAIN(type)) { .type = domain_new(DOMAIN_ORDER(type), 0) } -struct domain_generic *domain_new(uint order, bool allow_rcu); +#define DOMAIN_NEW(type) (DOMAIN(type)) { .type = domain_new(DOMAIN_ORDER(type)) } +struct domain_generic *domain_new(uint order); #define DOMAIN_FREE(type, d) domain_free((d).type) void domain_free(struct domain_generic *); diff --git a/lib/netindex.c b/lib/netindex.c index 7596bffd..1bc43377 100644 --- a/lib/netindex.c +++ b/lib/netindex.c @@ -56,7 +56,7 @@ net_lock_revive_unlock(netindex_hash *h, struct netindex *i) netindex_hash * netindex_hash_new(pool *sp, event_list *cleanup_target, u8 type) { - DOMAIN(attrs) dom = DOMAIN_NEW_RCU_SYNC(attrs); + DOMAIN(attrs) dom = DOMAIN_NEW(attrs); LOCK_DOMAIN(attrs, dom); pool *p = rp_new(sp, dom.attrs, "Network index"); diff --git a/lib/rcu.c b/lib/rcu.c index 25c575f1..17101489 100644 --- a/lib/rcu.c +++ b/lib/rcu.c @@ -18,7 +18,6 @@ _Atomic u64 rcu_global_phase = RCU_GP_PHASE; _Thread_local struct rcu_thread this_rcu_thread; -_Thread_local uint rcu_blocked; static struct rcu_thread * _Atomic rcu_thread_list = NULL; @@ -36,9 +35,6 @@ rcu_critical(struct rcu_thread *t, u64 phase) void synchronize_rcu(void) { - if (!rcu_blocked && (last_locked > &locking_stack.meta)) - bug("Forbidden to synchronize RCU unless an appropriate lock is taken"); - /* Increment phase */ u64 phase = atomic_fetch_add_explicit(&rcu_global_phase, RCU_GP_PHASE, memory_order_acq_rel); diff --git a/lib/rcu.h b/lib/rcu.h index a9244077..6a771a3a 100644 --- a/lib/rcu.h +++ b/lib/rcu.h @@ -28,7 +28,6 @@ struct rcu_thread { }; extern _Thread_local struct rcu_thread this_rcu_thread; -extern _Thread_local uint rcu_blocked; static inline void rcu_read_lock(void) { diff --git a/nest/rt-attr.c b/nest/rt-attr.c index ff0d54fa..978f423e 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -1843,7 +1843,7 @@ ea_show_list(struct cli *c, ea_list *eal) void rta_init(void) { - attrs_domain = DOMAIN_NEW_RCU_SYNC(attrs); + attrs_domain = DOMAIN_NEW(attrs); RTA_LOCK; rta_pool = rp_new(&root_pool, attrs_domain.attrs, "Attributes"); diff --git a/nest/rt-table.c b/nest/rt-table.c index 4f173815..a401035f 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -3090,7 +3090,7 @@ rt_setup(pool *pp, struct rtable_config *cf) pool *sp = birdloop_pool(loop); /* Create the table domain and pool */ - DOMAIN(rtable) dom = DOMAIN_NEW_RCU_SYNC(rtable); + DOMAIN(rtable) dom = DOMAIN_NEW(rtable); LOCK_DOMAIN(rtable, dom); pool *p = rp_newf(sp, dom.rtable, "Routing table data %s", cf->name); @@ -3256,7 +3256,7 @@ rt_init(void) init_list(&routing_tables); init_list(&deleted_routing_tables); ev_init_list(&rt_cork.queue, &main_birdloop, "Route cork release"); - rt_cork.dom = DOMAIN_NEW_RCU_SYNC(resource); + rt_cork.dom = DOMAIN_NEW(resource); idm_init(&rtable_idm, rt_table_pool, 256); ea_register_init(&ea_roa_aggregated); diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index d8d9d3cf..7cc3e514 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -1948,7 +1948,7 @@ bgp_init_pending_tx(struct bgp_channel *c) ASSERT_DIE(c->c.out_table == NULL); ASSERT_DIE(c->tx == NULL); - DOMAIN(rtable) dom = DOMAIN_NEW_RCU_SYNC(rtable); + DOMAIN(rtable) dom = DOMAIN_NEW(rtable); LOCK_DOMAIN(rtable, dom); pool *p = rp_newf(c->pool, dom.rtable, "%s.%s TX", c->c.proto->name, c->c.name); diff --git a/sysdep/unix/alloc.c b/sysdep/unix/alloc.c index 1cf41a5b..0432b9e2 100644 --- a/sysdep/unix/alloc.c +++ b/sysdep/unix/alloc.c @@ -215,6 +215,14 @@ alloc_page(void) /* Reinstate the stack with zero */ PAGE_STACK_PUT(NULL); + if (rcu_read_active()) + { + /* We can't lock and we actually shouldn't alloc either when rcu is active + * but that's a quest for another day. */ + } + else + { + /* If there is any free page kept cold, we use that. */ LOCK_DOMAIN(resource, empty_pages_domain); if (empty_pages) { @@ -244,6 +252,8 @@ alloc_page(void) if (fp) return fp; + } + /* And in the worst case, allocate some new pages by mmap() */ void *ptr = alloc_sys_page(); ajlog(ptr, NULL, 0, AJT_ALLOC_MMAP); diff --git a/sysdep/unix/domain.c b/sysdep/unix/domain.c index a3104b89..e76ac2fb 100644 --- a/sysdep/unix/domain.c +++ b/sysdep/unix/domain.c @@ -50,29 +50,27 @@ _Thread_local struct domain_generic **last_locked = NULL; struct domain_generic { pthread_mutex_t mutex; uint order; - bool forbidden_when_reading_rcu; struct domain_generic **prev; struct lock_order *locked_by; const char *name; pool *pool; }; -#define DOMAIN_INIT(_order, _allow_rcu) { \ +#define DOMAIN_INIT(_order) { \ .mutex = PTHREAD_MUTEX_INITIALIZER, \ .order = _order, \ - .forbidden_when_reading_rcu = !_allow_rcu, \ } -static struct domain_generic the_bird_domain_gen = DOMAIN_INIT(OFFSETOF(struct lock_order, the_bird), 1); +static struct domain_generic the_bird_domain_gen = DOMAIN_INIT(OFFSETOF(struct lock_order, the_bird)); DOMAIN(the_bird) the_bird_domain = { .the_bird = &the_bird_domain_gen }; struct domain_generic * -domain_new(uint order, bool allow_rcu) +domain_new(uint order) { ASSERT_DIE(order < sizeof(struct lock_order)); struct domain_generic *dg = xmalloc(sizeof(struct domain_generic)); - *dg = (struct domain_generic) DOMAIN_INIT(order, allow_rcu); + *dg = (struct domain_generic) DOMAIN_INIT(order); return dg; } @@ -108,11 +106,8 @@ void do_lock(struct domain_generic *dg, struct domain_generic **lsp) memcpy(&stack_copy, &locking_stack, sizeof(stack_copy)); struct domain_generic **lll = last_locked; - if (dg->forbidden_when_reading_rcu) - if (rcu_read_active()) - bug("Locking of this lock forbidden while RCU reader is active"); - else - rcu_blocked++; + if (rcu_read_active()) + bug("Locking forbidden while RCU reader is active"); if ((char *) lsp - (char *) &locking_stack != dg->order) bug("Trying to lock on bad position: order=%u, lsp=%p, base=%p", dg->order, lsp, &locking_stack); @@ -139,9 +134,6 @@ void do_lock(struct domain_generic *dg, struct domain_generic **lsp) void do_unlock(struct domain_generic *dg, struct domain_generic **lsp) { - if (dg->forbidden_when_reading_rcu) - ASSERT_DIE(rcu_blocked--); - if ((char *) lsp - (char *) &locking_stack != dg->order) bug("Trying to unlock on bad position: order=%u, lsp=%p, base=%p", dg->order, lsp, &locking_stack); diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c index d2a60562..7f45f541 100644 --- a/sysdep/unix/io-loop.c +++ b/sysdep/unix/io-loop.c @@ -1581,7 +1581,7 @@ birdloop_run_timer(timer *tm) static struct birdloop * birdloop_vnew_internal(pool *pp, uint order, struct birdloop_pickup_group *group, const char *name, va_list args) { - struct domain_generic *dg = domain_new(order, 1); + struct domain_generic *dg = domain_new(order); DG_LOCK(dg); pool *p = rp_vnewf(pp, dg, name, args);