[Libguestfs] [nbdkit PATCH 2/2] server: Better caching of .thread_model
Eric Blake
eblake at redhat.com
Thu Mar 19 01:21:14 UTC 2020
Repeatedly calling out to the plugin to determine the thread model is
not only potentially expensive (for a plugin that takes its time
answering, as is easy to do with 'nbdkit eval thread_model="sleep 1"
...'), but risks confusion if the plugin changes its answer over time.
Expose the cache variable in locks.c to the rest of the server code so
that we can learn the thread model exactly once, with testsuite
coverage to ensure we don't regress.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
server/internal.h | 1 +
server/connections.c | 8 +++-----
server/filters.c | 8 ++++----
server/locks.c | 7 +++----
server/plugins.c | 12 ++++++------
server/protocol-handshake.c | 5 ++---
server/sockets.c | 3 +--
tests/test-eflags.sh | 4 +---
8 files changed, 21 insertions(+), 27 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index c1177b5d..e39db8a8 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -468,6 +468,7 @@ extern struct backend *filter_register (struct backend *next, size_t index,
__attribute__((__nonnull__ (1, 3, 4, 5)));
/* locks.c */
+extern unsigned thread_model;
extern void lock_init_thread_model (void);
extern const char *name_of_thread_model (int model);
extern void lock_connection (void);
diff --git a/server/connections.c b/server/connections.c
index 5be3266a..c54c71c1 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -147,8 +147,7 @@ handle_single_connection (int sockin, int sockout)
return;
}
- if (top->thread_model (top) < NBDKIT_THREAD_MODEL_PARALLEL ||
- nworkers == 1)
+ if (thread_model < NBDKIT_THREAD_MODEL_PARALLEL || nworkers == 1)
nworkers = 0;
conn = new_connection (sockin, sockout, nworkers);
if (!conn)
@@ -277,8 +276,7 @@ new_connection (int sockin, int sockout, int nworkers)
* we aren't accepting until the plugin is not running, making
* non-atomicity okay.
*/
- assert (top->thread_model (top) <=
- NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
+ assert (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
lock_request (NULL);
if (pipe (conn->status_pipe)) {
perror ("pipe");
diff --git a/server/filters.c b/server/filters.c
index f0371066..b4f05236 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -66,7 +66,7 @@ filter_thread_model (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
int filter_thread_model = NBDKIT_THREAD_MODEL_PARALLEL;
- int thread_model = b->next->thread_model (b->next);
+ int model = b->next->thread_model (b->next);
if (f->filter.thread_model) {
filter_thread_model = f->filter.thread_model ();
@@ -74,10 +74,10 @@ filter_thread_model (struct backend *b)
exit (EXIT_FAILURE);
}
- if (filter_thread_model < thread_model) /* more serialized */
- thread_model = filter_thread_model;
+ if (filter_thread_model < model) /* more serialized */
+ model = filter_thread_model;
- return thread_model;
+ return model;
}
/* This is actually passing the request through to the final plugin,
diff --git a/server/locks.c b/server/locks.c
index 6211648d..5d54d311 100644
--- a/server/locks.c
+++ b/server/locks.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -40,7 +40,7 @@
/* Note that the plugin's thread model cannot change after being
* loaded, so caching it here is safe.
*/
-static int thread_model = -1;
+unsigned thread_model = -1;
static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -71,12 +71,12 @@ lock_init_thread_model (void)
{
thread_model = top->thread_model (top);
debug ("using thread model: %s", name_of_thread_model (thread_model));
+ assert (thread_model <= NBDKIT_THREAD_MODEL_PARALLEL);
}
void
lock_connection (void)
{
- assert (thread_model >= 0);
if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS &&
pthread_mutex_lock (&connection_lock))
abort ();
@@ -95,7 +95,6 @@ lock_request (void)
{
GET_CONN;
- assert (thread_model >= 0);
if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS &&
pthread_mutex_lock (&all_requests_lock))
abort ();
diff --git a/server/plugins.c b/server/plugins.c
index 708e1cfa..fa572a6a 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -65,14 +65,14 @@ static int
plugin_thread_model (struct backend *b)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
- int thread_model = p->plugin._thread_model;
+ int model = p->plugin._thread_model;
int r;
#if !(defined SOCK_CLOEXEC && defined HAVE_MKOSTEMP && defined HAVE_PIPE2 && \
defined HAVE_ACCEPT4)
- if (thread_model > NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
+ if (model > NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
debug ("system lacks atomic CLOEXEC, serializing to avoid fd leaks");
- thread_model = NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS;
+ model = NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS;
}
#endif
@@ -80,11 +80,11 @@ plugin_thread_model (struct backend *b)
r = p->plugin.thread_model ();
if (r == -1)
exit (EXIT_FAILURE);
- if (r < thread_model)
- thread_model = r;
+ if (r < model)
+ model = r;
}
- return thread_model;
+ return model;
}
static const char *
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 70ea4933..5e4b5e6e 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -144,8 +144,7 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
fl = backend_can_multi_conn (top);
if (fl == -1)
return -1;
- if (fl && (top->thread_model (top) >
- NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS))
+ if (fl && (thread_model > NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS))
eflags |= NBD_FLAG_CAN_MULTI_CONN;
fl = backend_can_cache (top);
diff --git a/server/sockets.c b/server/sockets.c
index 15aacf52..79ee704e 100644
--- a/server/sockets.c
+++ b/server/sockets.c
@@ -393,8 +393,7 @@ accept_connection (int listen_sock)
* at which point, we can use a mutex to ensure we aren't accepting
* until the plugin is not running, making non-atomicity okay.
*/
- assert (top->thread_model (top) <=
- NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
+ assert (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
lock_request ();
thread_data->sock = set_cloexec (accept (listen_sock, NULL, NULL));
unlock_request ();
diff --git a/tests/test-eflags.sh b/tests/test-eflags.sh
index 3228bbf6..aebcaf68 100755
--- a/tests/test-eflags.sh
+++ b/tests/test-eflags.sh
@@ -74,9 +74,7 @@ do_nbdkit ()
{
# Prepend a check for internal caching to the script on stdin.
{ printf %s '
- if test $1 = thread_model; then
- :
- elif test -f $tmpdir/seen_$1; then
+ if test -f $tmpdir/seen_$1; then
echo "repeat call to $1" >>'"$PWD/eflags.err"'
else
touch $tmpdir/seen_$1
--
2.25.1
More information about the Libguestfs
mailing list