From 186e96ece0fb5a657f18e3857f1b69fcff8ba6c3 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 28 May 2019 17:21:18 +0200 Subject: [PATCH] MEDIUM: buffers: relax the buffer lock a little bit In lock profiles it's visible that there is a huge contention on the buffer lock. The reason is that when offer_buffers() is called, it systematically takes the lock before verifying if there is any waiter. However doing so doesn't protect against races since a waiter can happen just after we release the lock as well. Similarly in h2 we take the lock every time an h2c is going to be released, even without checking that the h2c belongs to a wait list. These two have now been addressed by verifying non-emptiness of the list prior to taking the lock. --- include/common/buffer.h | 10 +++++----- src/mux_h2.c | 8 +++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/common/buffer.h b/include/common/buffer.h index 960402633..3d6c97ca6 100644 --- a/include/common/buffer.h +++ b/include/common/buffer.h @@ -201,12 +201,12 @@ void __offer_buffer(void *from, unsigned int threshold); static inline void offer_buffers(void *from, unsigned int threshold) { - HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock); - if (LIST_ISEMPTY(&buffer_wq)) { - HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock); + if (LIST_ISEMPTY(&buffer_wq)) return; - } - __offer_buffer(from, threshold); + + HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock); + if (!LIST_ISEMPTY(&buffer_wq)) + __offer_buffer(from, threshold); HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock); } diff --git a/src/mux_h2.c b/src/mux_h2.c index 7d29a7d1f..8b8135d1a 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -662,9 +662,11 @@ static void h2_release(struct h2c *h2c) hpack_dht_free(h2c->ddht); - HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock); - LIST_DEL(&h2c->buf_wait.list); - HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock); + if (LIST_ADDED(&h2c->buf_wait.list)) { + HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock); + LIST_DEL(&h2c->buf_wait.list); + HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock); + } h2_release_buf(h2c, &h2c->dbuf); h2_release_mbuf(h2c);