From: Remi Tricot-Le Breton <rlebreton@haproxy.com>
Date: Thu, 16 Nov 2023 16:38:23 +0000 (+0100)
Subject: MEDIUM: shctx: Descend shctx_lock calls into the shctx_row_reserve_hot
X-Git-Tag: v2.9-dev10~52
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=11df806c88d2a3bb18c2ded9587053e4e29ddde4;p=thirdparty%2Fhaproxy.git

MEDIUM: shctx: Descend shctx_lock calls into the shctx_row_reserve_hot

Descend the shctx_lock calls into the shctx_row_reserve_hot so that the
cases when we don't need to lock anything (enough space in the current
row or not enough space in the 'avail' list) do not take the lock at
all.
In sh_ssl_sess_new_cb the lock had to be descended into
sh_ssl_sess_store in order not to cover the shctx_row_reserve_hot call
anymore.
---

diff --git a/src/cache.c b/src/cache.c
index caa71e8a39..03651c8e24 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -770,13 +770,10 @@ cache_store_http_payload(struct stream *s, struct filter *filter, struct http_ms
 
   end:
 
-	shctx_wrlock(shctx);
 	fb = shctx_row_reserve_hot(shctx, st->first_block, trash.data);
 	if (!fb) {
-		shctx_wrunlock(shctx);
 		goto no_cache;
 	}
-	shctx_wrunlock(shctx);
 
 	ret = shctx_row_data_append(shctx, st->first_block,
 				    (unsigned char *)b_head(&trash), b_data(&trash));
@@ -1225,9 +1222,7 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
 	}
 	cache_wrunlock(cache);
 
-	shctx_wrlock(shctx);
 	first = shctx_row_reserve_hot(shctx, NULL, sizeof(struct cache_entry));
-	shctx_wrunlock(shctx);
 	if (!first) {
 		goto out;
 	}
@@ -1327,12 +1322,9 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
 		if (set_secondary_key_encoding(htx, object->secondary_key))
 		    goto out;
 
-	shctx_wrlock(shctx);
 	if (!shctx_row_reserve_hot(shctx, first, trash.data)) {
-		shctx_wrunlock(shctx);
 		goto out;
 	}
-	shctx_wrunlock(shctx);
 
 	/* cache the headers in a http action because it allows to chose what
 	 * to cache, for example you might want to cache a response before
diff --git a/src/shctx.c b/src/shctx.c
index a9ea4c36cc..daf7866c9e 100644
--- a/src/shctx.c
+++ b/src/shctx.c
@@ -35,10 +35,6 @@ struct shared_block *shctx_row_reserve_hot(struct shared_context *shctx,
 
 	BUG_ON(data_len < 0);
 
-	/* not enough usable blocks */
-	if (data_len > shctx->nbav * shctx->block_size)
-		goto out;
-
 	/* Check the object size limit. */
 	if (shctx->max_obj_size > 0) {
 		if ((first && first->len + data_len > shctx->max_obj_size) ||
@@ -66,8 +62,19 @@ struct shared_block *shctx_row_reserve_hot(struct shared_context *shctx,
 		}
 	}
 
+	shctx_wrlock(shctx);
+
+	/* not enough usable blocks */
+	if (data_len > shctx->nbav * shctx->block_size) {
+		shctx_wrunlock(shctx);
+		goto out;
+	}
+
+
 	if (data_len <= 0 || LIST_ISEMPTY(&shctx->avail)) {
-		return NULL;
+		ret = NULL;
+		shctx_wrunlock(shctx);
+		goto out;
 	}
 
 	list_for_each_entry_safe(block, sblock, &shctx->avail, list) {
@@ -101,6 +108,8 @@ struct shared_block *shctx_row_reserve_hot(struct shared_context *shctx,
 		}
 	}
 
+	shctx_wrunlock(shctx);
+
 out:
 	return ret;
 }
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bcbcb8470a..3a8f2481dd 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4239,6 +4239,8 @@ static int sh_ssl_sess_store(unsigned char *s_id, unsigned char *data, int data_
 		return 0;
 	}
 
+	shctx_wrlock(ssl_shctx);
+
 	/* STORE the key in the first elem */
 	sh_ssl_sess = (struct sh_ssl_sess_hdr *)first->data;
 	memcpy(sh_ssl_sess->key_data, s_id, SSL_MAX_SSL_SESSION_ID_LENGTH);
@@ -4267,6 +4269,8 @@ static int sh_ssl_sess_store(unsigned char *s_id, unsigned char *data, int data_
 
 	shctx_row_reattach(ssl_shctx, first);
 
+	shctx_wrunlock(ssl_shctx);
+
 	return 1;
 }
 
@@ -4408,10 +4412,8 @@ int sh_ssl_sess_new_cb(SSL *ssl, SSL_SESSION *sess)
 	i2d_SSL_SESSION(sess, &p);
 
 
-	shctx_wrlock(ssl_shctx);
 	/* store to cache */
 	sh_ssl_sess_store(encid, encsess, data_len);
-	shctx_wrunlock(ssl_shctx);
 err:
 	/* reset original length values */
 	SSL_SESSION_set1_id(sess, encid, sid_length);