From 9e99cfbeb66f4a8c470780aad19a677652d5c04b Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 28 Feb 2024 17:06:39 +0100 Subject: [PATCH] MAJOR: ring: drop the now unneeded lock It was only used to protect the list which is now an mt_list so it doesn't provide any required protection anymore. It obviously also used to provide strict ordering between the writer and the reader when the writer started to update the messages, but that's now covered by the oredered tail updates and updates to the readers count to protect the area. The message rate on small thread counts (up to 12) saw a boost of roughly 5% while on large counts while for large counts it lost about 2% due to some contention now becoming visible elsewhere. Typical measures are 6.13M -> 6.61M at 3C6T, and 1.88 -> 1.92M at 24C48T on the EPYC. --- include/haproxy/ring-t.h | 1 - src/ring.c | 9 --------- src/sink.c | 10 ---------- 3 files changed, 20 deletions(-) diff --git a/include/haproxy/ring-t.h b/include/haproxy/ring-t.h index 3624a00..9367125 100644 --- a/include/haproxy/ring-t.h +++ b/include/haproxy/ring-t.h @@ -122,7 +122,6 @@ struct ring_storage { struct ring { struct ring_storage *storage; // the mapped part struct mt_list waiters; // list of waiters, for now, CLI "show event" - __decl_thread(HA_RWLOCK_T lock); int readers_count; uint flags; // RING_FL_* }; diff --git a/src/ring.c b/src/ring.c index d0c6736..826662a 100644 --- a/src/ring.c +++ b/src/ring.c @@ -45,7 +45,6 @@ struct show_ring_ctx { */ void ring_init(struct ring *ring, void *area, size_t size, int reset) { - HA_RWLOCK_INIT(&ring->lock); MT_LIST_INIT(&ring->waiters); ring->readers_count = 0; ring->flags = 0; @@ -342,10 +341,8 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const struct ist pfx[], siz struct mt_list *elt1, elt2; struct appctx *appctx; - HA_RWLOCK_RDLOCK(RING_LOCK, &ring->lock); mt_list_for_each_entry_safe(appctx, &ring->waiters, wait_entry, elt1, elt2) appctx_wakeup(appctx); - HA_RWLOCK_RDUNLOCK(RING_LOCK, &ring->lock); } leave: @@ -379,7 +376,6 @@ void ring_detach_appctx(struct ring *ring, struct appctx *appctx, size_t ofs) if (!ring) return; - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); HA_ATOMIC_DEC(&ring->readers_count); if (ofs != ~0) { @@ -396,7 +392,6 @@ void ring_detach_appctx(struct ring *ring, struct appctx *appctx, size_t ofs) } while ((readers > RING_MAX_READERS || !_HA_ATOMIC_CAS(area + ofs, &readers, readers - 1)) && __ha_cpu_relax()); } - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); } /* Tries to attach CLI handler as a new reader on ring . This is @@ -583,9 +578,7 @@ int cli_io_handler_show_ring(struct appctx *appctx) if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE)) return 1; - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_DELETE(&appctx->wait_entry); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); ret = ring_dispatch_messages(ring, appctx, &ctx->ofs, &last_ofs, ctx->flags, applet_append_line); @@ -595,10 +588,8 @@ int cli_io_handler_show_ring(struct appctx *appctx) */ if (!sc_oc(sc)->output && !(sc->flags & SC_FL_SHUT_DONE)) { /* let's be woken up once new data arrive */ - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_APPEND(&ring->waiters, &appctx->wait_entry); ofs = ring_tail(ring); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); if (ofs != last_ofs) { /* more data was added into the ring between the * unlock and the lock, and the writer might not diff --git a/src/sink.c b/src/sink.c index 52b9d2a..1195c35 100644 --- a/src/sink.c +++ b/src/sink.c @@ -382,18 +382,14 @@ static void sink_forward_io_handler(struct appctx *appctx) goto close; } - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_DELETE(&appctx->wait_entry); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); ret = ring_dispatch_messages(ring, appctx, &sft->ofs, &last_ofs, 0, applet_append_line); if (ret) { /* let's be woken up once new data arrive */ - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_APPEND(&ring->waiters, &appctx->wait_entry); ofs = ring_tail(ring); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); if (ofs != last_ofs) { /* more data was added into the ring between the * unlock and the lock, and the writer might not @@ -451,17 +447,13 @@ static void sink_forward_oc_io_handler(struct appctx *appctx) goto close; } - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_DELETE(&appctx->wait_entry); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); ret = ring_dispatch_messages(ring, appctx, &sft->ofs, &last_ofs, 0, syslog_applet_append_event); if (ret) { /* let's be woken up once new data arrive */ - HA_RWLOCK_WRLOCK(RING_LOCK, &ring->lock); MT_LIST_APPEND(&ring->waiters, &appctx->wait_entry); ofs = ring_tail(ring); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &ring->lock); if (ofs != last_ofs) { /* more data was added into the ring between the * unlock and the lock, and the writer might not @@ -491,9 +483,7 @@ void __sink_forward_session_deinit(struct sink_forward_target *sft) if (!sink) return; - HA_RWLOCK_WRLOCK(RING_LOCK, &sink->ctx.ring->lock); MT_LIST_DELETE(&sft->appctx->wait_entry); - HA_RWLOCK_WRUNLOCK(RING_LOCK, &sink->ctx.ring->lock); sft->appctx = NULL; task_wakeup(sink->forward_task, TASK_WOKEN_MSG); -- 1.7.10.4