From a58af5b0a1b2826725c33a81a0fd0b1fb891dd38 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 16 Nov 2022 11:37:29 +0100 Subject: [PATCH] MINOR: dynbuf: switch allocation and release to macros to better track users When building with DEBUG_MEM_STATS, we only see b_alloc() and b_free() as users of the "buffer" pool, because all call places rely on these more convenient functions. It's annoying because it makes it very hard to see which parts of the code are consuming buffers. By switching the b_alloc() and b_free() inline functions to macros, we can now finally track the users of struct buffer, e.g: mux_h1.c:513 P_FREE size: 1275002880 calls: 38910 size/call: 32768 buffer mux_h1.c:498 P_ALLOC size: 1912438784 calls: 58363 size/call: 32768 buffer stream.c:763 P_FREE size: 4121493504 calls: 125778 size/call: 32768 buffer stream.c:759 P_FREE size: 2061697024 calls: 62918 size/call: 32768 buffer stream.c:742 P_ALLOC size: 3341123584 calls: 101963 size/call: 32768 buffer stream.c:632 P_FREE size: 1275068416 calls: 38912 size/call: 32768 buffer stream.c:631 P_FREE size: 637435904 calls: 19453 size/call: 32768 buffer channel.h:850 P_ALLOC size: 4116480000 calls: 125625 size/call: 32768 buffer channel.h:850 P_ALLOC size: 720896 calls: 22 size/call: 32768 buffer dynbuf.c:55 P_FREE size: 65536 calls: 2 size/call: 32768 buffer Let's do this since it doesn't change anything for the output code (beyond adding the call places). Interestingly the code even got slightly smaller now. --- include/haproxy/dynbuf.h | 69 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/include/haproxy/dynbuf.h b/include/haproxy/dynbuf.h index 45bd4cf8c..c0a460d05 100644 --- a/include/haproxy/dynbuf.h +++ b/include/haproxy/dynbuf.h @@ -60,46 +60,47 @@ static inline int buffer_almost_full(const struct buffer *buf) * ((char *)1) is assigned instead with a zero size. The allocated buffer is * returned, or NULL in case no memory is available. */ -static inline struct buffer *b_alloc(struct buffer *buf) -{ - char *area; - - if (buf->size) - return buf; - - *buf = BUF_WANTED; - area = pool_alloc_flag(pool_head_buffer, POOL_F_NO_POISON); - if (unlikely(!area)) { - activity[tid].buf_wait++; - return NULL; - } - - buf->area = area; - buf->size = pool_head_buffer->size; - return buf; -} +#define b_alloc(_buf) \ +({ \ + char *_area; \ + struct buffer *_retbuf = _buf; \ + \ + if (!_retbuf->size) { \ + *_retbuf = BUF_WANTED; \ + _area = pool_alloc_flag(pool_head_buffer, POOL_F_NO_POISON); \ + if (unlikely(!_area)) { \ + activity[tid].buf_wait++; \ + _retbuf = NULL; \ + } \ + else { \ + _retbuf->area = _area; \ + _retbuf->size = pool_head_buffer->size; \ + } \ + } \ + _retbuf; \ + }) /* Releases buffer (no check of emptiness). The buffer's head is marked * empty. */ -static inline void __b_free(struct buffer *buf) -{ - char *area = buf->area; - - /* let's first clear the area to save an occasional "show sess all" - * glancing over our shoulder from getting a dangling pointer. - */ - *buf = BUF_NULL; - __ha_barrier_store(); - pool_free(pool_head_buffer, area); -} +#define __b_free(_buf) \ + do { \ + char *area = (_buf)->area; \ + \ + /* let's first clear the area to save an occasional "show sess all" \ + * glancing over our shoulder from getting a dangling pointer. \ + */ \ + *(_buf) = BUF_NULL; \ + __ha_barrier_store(); \ + pool_free(pool_head_buffer, area); \ + } while (0) \ /* Releases buffer if allocated, and marks it empty. */ -static inline void b_free(struct buffer *buf) -{ - if (buf->size) - __b_free(buf); -} +#define b_free(_buf) \ + do { \ + if ((_buf)->size) \ + __b_free((_buf)); \ + } while (0) /* Offer one or multiple buffer currently belonging to target to whoever * needs one. Any pointer is valid for , including NULL. Its purpose is