[master] e1619abc9 vca: Promote shut_mtx to instrumented lock

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Thu Jun 19 17:08:06 UTC 2025


commit e1619abc926505a7f6c8db0c2fdc3ff52efbc05c
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Jul 11 09:25:47 2023 +0200

    vca: Promote shut_mtx to instrumented lock
    
    This would have been a cherry-pick of a commit from #3959, but #3976
    introduced many conflicts so I had to reapply it manually.
    
    This port was motivated by the curious inclusion of pthread.h that
    should have happened via a higher-level include in the cache process.
    Either from cache.h, or indirectly from cache_varnishd.h for internal
    subsystems.
    
    Refs 119832d5e964b01dd672ef6e15f4e2c128bd0c4e
    
    It turns out this include is needed because cache_acceptor.h is
    included from mgt code, which looks like an improvement waiting to
    happen in that area. The mgt process is single-threaded, so we should
    really treat the inclusion of pthread.h as a red flag.
    
    Switching to an instrumented lock means that mgt code can ignore an
    opaque struct lock pointer passed to the update() callback, on top
    of exposing metrics regarding this lock.
    
    The encapsulation problem was already registered in an XXX comment
    in mgt_acceptor.c, and despite this modest improvement, restoring
    proper mgt/cache boundaries in the listen socket department will
    require more work.

diff --git a/bin/varnishd/acceptor/cache_acceptor.c b/bin/varnishd/acceptor/cache_acceptor.c
index 2d6e6350b..57d188644 100644
--- a/bin/varnishd/acceptor/cache_acceptor.c
+++ b/bin/varnishd/acceptor/cache_acceptor.c
@@ -55,7 +55,7 @@ unsigned pool_accepting;
 static pthread_t	VCA_thread;
 static vtim_dur vca_pace = 0.0;
 static struct lock pace_mtx;
-static pthread_mutex_t shut_mtx = PTHREAD_MUTEX_INITIALIZER;
+static struct lock shut_mtx;
 
 /*--------------------------------------------------------------------
  * lacking a better place, we put some generic periodic updates
@@ -211,7 +211,7 @@ ccf_listen_address(struct cli *cli, const char * const *av, void *priv)
 	while (!pool_accepting)
 		VTIM_sleep(.1);
 
-	PTOK(pthread_mutex_lock(&shut_mtx));
+	Lck_Lock(&shut_mtx);
 
 	/*
 	 * Varnishtest expects the list of listen sockets to come out in the
@@ -223,7 +223,7 @@ ccf_listen_address(struct cli *cli, const char * const *av, void *priv)
 		ls->vca->event(cli, ls, VCA_EVENT_LADDR);
 	}
 
-	PTOK(pthread_mutex_unlock(&shut_mtx));
+	Lck_Unlock(&shut_mtx);
 }
 
 /*--------------------------------------------------------------------*/
@@ -240,6 +240,7 @@ VCA_Init(void)
 
 	CLI_AddFuncs(vca_cmds);
 	Lck_New(&pace_mtx, lck_vcapace);
+	Lck_New(&shut_mtx, lck_vcashut);
 
 	VCA_Foreach(vca) {
 		CHECK_OBJ_NOTNULL(vca, ACCEPTOR_MAGIC);
@@ -252,14 +253,14 @@ VCA_Shutdown(void)
 {
 	struct acceptor *vca;
 
-	PTOK(pthread_mutex_lock(&shut_mtx));
+	Lck_Lock(&shut_mtx);
 
 	VCA_Foreach(vca) {
 		CHECK_OBJ_NOTNULL(vca, ACCEPTOR_MAGIC);
 		vca->shutdown();
 	}
 
-	PTOK(pthread_mutex_unlock(&shut_mtx));
+	Lck_Unlock(&shut_mtx);
 }
 
 /*--------------------------------------------------------------------
diff --git a/bin/varnishd/acceptor/cache_acceptor.h b/bin/varnishd/acceptor/cache_acceptor.h
index 3815b8289..11f143a22 100644
--- a/bin/varnishd/acceptor/cache_acceptor.h
+++ b/bin/varnishd/acceptor/cache_acceptor.h
@@ -36,6 +36,7 @@
 struct listen_sock;
 struct listen_arg;
 struct pool;
+struct lock;
 
 void VCA_Init(void);
 void VCA_Start(struct cli *cli);
@@ -54,7 +55,7 @@ typedef void acceptor_start_f(struct cli *);
 typedef void acceptor_event_f(struct cli *, struct listen_sock *,
     enum vca_event);
 typedef void acceptor_accept_f(struct pool *);
-typedef void acceptor_update_f(pthread_mutex_t *);
+typedef void acceptor_update_f(struct lock *);
 typedef void acceptor_shutdown_f(void);
 
 struct acceptor {
diff --git a/bin/varnishd/acceptor/cache_acceptor_tcp.c b/bin/varnishd/acceptor/cache_acceptor_tcp.c
index af2ceda90..8efac3694 100644
--- a/bin/varnishd/acceptor/cache_acceptor_tcp.c
+++ b/bin/varnishd/acceptor/cache_acceptor_tcp.c
@@ -552,14 +552,14 @@ vca_tcp_accept(struct pool *pp)
 }
 
 static void
-vca_tcp_update(pthread_mutex_t *shut_mtx)
+vca_tcp_update(struct lock *shut_mtx)
 {
 	struct listen_sock *ls;
 
 	if (!vca_tcp_sockopt_init())
 		return;
 
-	PTOK(pthread_mutex_lock(shut_mtx));
+	Lck_Lock(shut_mtx);
 
 	VTAILQ_FOREACH(ls, &TCP_acceptor.socks, vcalist) {
 		CHECK_OBJ_NOTNULL(ls, LISTEN_SOCK_MAGIC);
@@ -581,7 +581,7 @@ vca_tcp_update(pthread_mutex_t *shut_mtx)
 		ls->test_heritage = 1;
 	}
 
-	PTOK(pthread_mutex_unlock(shut_mtx));
+	Lck_Unlock(shut_mtx);
 }
 
 static void
diff --git a/bin/varnishd/acceptor/cache_acceptor_uds.c b/bin/varnishd/acceptor/cache_acceptor_uds.c
index 75462416b..d67f552a5 100644
--- a/bin/varnishd/acceptor/cache_acceptor_uds.c
+++ b/bin/varnishd/acceptor/cache_acceptor_uds.c
@@ -495,14 +495,14 @@ vca_uds_accept(struct pool *pp)
 }
 
 static void
-vca_uds_update(pthread_mutex_t *shut_mtx)
+vca_uds_update(struct lock *shut_mtx)
 {
 	struct listen_sock *ls;
 
 	if (!vca_uds_sockopt_init())
 		return;
 
-	PTOK(pthread_mutex_lock(shut_mtx));
+	Lck_Lock(shut_mtx);
 
 	VTAILQ_FOREACH(ls, &UDS_acceptor.socks, vcalist) {
 		CHECK_OBJ_NOTNULL(ls, LISTEN_SOCK_MAGIC);
@@ -524,7 +524,7 @@ vca_uds_update(pthread_mutex_t *shut_mtx)
 		ls->test_heritage = 1;
 	}
 
-	PTOK(pthread_mutex_unlock(shut_mtx));
+	Lck_Unlock(shut_mtx);
 }
 
 static void
diff --git a/include/tbl/locks.h b/include/tbl/locks.h
index 773c31ffa..2199270e2 100644
--- a/include/tbl/locks.h
+++ b/include/tbl/locks.h
@@ -48,6 +48,7 @@ LOCK(conn_pool)
 LOCK(dead_pool)
 LOCK(vbe)
 LOCK(vcapace)
+LOCK(vcashut)
 LOCK(vcl)
 LOCK(vxid)
 LOCK(waiter)


More information about the varnish-commit mailing list