10:35 a.m.
There was a "dormant" (ie. never implemented) feature of the generator
for calls which don't need to take the handle lock. This implements
it.
By making h->state be an atomic field (which should be nearly free on
most normal architectures) we can modify calls which only read
h->state and do nothing else with the handle so they never acquire the
lock.
---
generator/generator | 46 +++++++++++++++++++++++++++------------------
lib/aio.c | 6 ++++++
lib/internal.h | 3 ++-
3 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/generator/generator b/generator/generator
index e04c9e1..f0de6b0 100755
--- a/generator/generator
+++ b/generator/generator
@@ -807,9 +807,13 @@ and structured_reply_state_machine = [
type call = {
args : arg list; (* parameters (except handle) *)
ret : ret; (* return value *)
- is_locked : bool; (* most functions need to take a lock *)
shortdesc : string; (* short description *)
longdesc : string; (* long description *)
+ (* Most functions must take a lock. The only known exception is
+ * for a function which {b only} reads from the atomic [h->state]
+ * field and does nothing else with the handle.
+ *)
+ is_locked : bool;
}
and arg =
| ArrayAndLen of arg * string (* array + number of entries *)
@@ -841,11 +845,18 @@ and ret =
| RInt64 (* 64 bit int, -1 = error *)
| RString (* return a newly allocated string, caller frees *)
-let default_call = { args = []; ret = RErr; is_locked = true;
- shortdesc = ""; longdesc = "" }
+let default_call = { args = []; ret = RErr;
+ shortdesc = ""; longdesc = "";
+ is_locked = true }
-(* Calls - first parameter [struct nbd_handle *nbd] is implicit. *)
-let handle_calls = [
+(* Calls.
+ *
+ * The first parameter [struct nbd_handle *nbd] is implicit.
+ *
+ * Disable:
+ * Warning 23: all the fields are explicitly listed in this record:
+ *)
+let [@warning "-23"] handle_calls = [
"set_debug", {
default_call with
args = [ Bool "debug" ]; ret = RErr;
@@ -1668,7 +1679,7 @@ connection is writable.";
"aio_is_created", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection has just been created";
longdesc = "\
Return true if this connection has just been created. This
@@ -1679,7 +1690,7 @@ by calling functions such as C<nbd_aio_connect>.";
"aio_is_connecting", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection is connecting or handshaking";
longdesc = "\
Return true if this connection is connecting to the server
@@ -1690,7 +1701,7 @@ issue commands (see C<nbd_aio_is_ready>).";
"aio_is_ready", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection is in the ready state";
longdesc = "\
Return true if this connection is connected to the NBD server,
@@ -1701,7 +1712,7 @@ issue commands.";
"aio_is_processing", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection is processing a command";
longdesc = "\
Return true if this connection is connected to the NBD server,
@@ -1715,7 +1726,7 @@ is processing them), but libnbd is not processing them.";
"aio_is_dead", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection is dead";
longdesc = "\
Return true if the connection has encountered a fatal
@@ -1725,7 +1736,7 @@ There is no way to recover a handle from the dead state.";
"aio_is_closed", {
default_call with
- args = []; ret = RBool;
+ args = []; ret = RBool; is_locked = false;
shortdesc = "check if the connection is closed";
longdesc = "\
Return true if the connection has closed. There is no way to
@@ -2650,7 +2661,7 @@ let generate_lib_unlocked_h () =
* grab the thread mutex (lock) and do logging.
*)
let generate_lib_api_c () =
- let print_wrapper name args ret =
+ let print_wrapper (name, {args; ret; is_locked}) =
(match ret with
| RBool
| RErr
@@ -2674,13 +2685,15 @@ let generate_lib_api_c () =
| RString -> pr " char *ret;\n"
);
pr "\n";
- pr " pthread_mutex_lock (&h->lock);\n";
+ if is_locked then
+ pr " pthread_mutex_lock (&h->lock);\n";
pr " nbd_internal_reset_error (\"nbd_%s\");\n" name;
pr " ret = nbd_unlocked_%s (h" name;
let argnames = List.flatten (List.map name_of_arg args) in
List.iter (pr ", %s") argnames;
pr ");\n";
- pr " pthread_mutex_unlock (&h->lock);\n";
+ if is_locked then
+ pr " pthread_mutex_unlock (&h->lock);\n";
pr " return ret;\n";
pr "}\n";
pr "\n";
@@ -2698,10 +2711,7 @@ let generate_lib_api_c () =
pr "#include \"libnbd.h\"\n";
pr "#include \"internal.h\"\n";
pr "\n";
- List.iter (
- fun (name, { args; ret }) ->
- print_wrapper name args ret
- ) handle_calls
+ List.iter print_wrapper handle_calls
let print_api (name, { args; ret; shortdesc; longdesc }) =
pr "=head2 %s —\n" name;
diff --git a/lib/aio.c b/lib/aio.c
index 5adc3cd..65b1ef3 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -48,6 +48,7 @@ nbd_unlocked_aio_notify_write (struct nbd_handle *h)
return nbd_internal_run (h, notify_write);
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_created (struct nbd_handle *h)
{
@@ -72,6 +73,7 @@ is_connecting_group (enum state_group group)
}
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_connecting (struct nbd_handle *h)
{
@@ -80,6 +82,7 @@ nbd_unlocked_aio_is_connecting (struct nbd_handle *h)
return is_connecting_group (group);
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_ready (struct nbd_handle *h)
{
@@ -100,6 +103,7 @@ is_processing_group (enum state_group group)
}
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_processing (struct nbd_handle *h)
{
@@ -108,12 +112,14 @@ nbd_unlocked_aio_is_processing (struct nbd_handle *h)
return is_processing_group (group);
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_dead (struct nbd_handle *h)
{
return h->state == STATE_DEAD;
}
+/* NB: is_locked = false. */
int
nbd_unlocked_aio_is_closed (struct nbd_handle *h)
{
diff --git a/lib/internal.h b/lib/internal.h
index 419795f..73cb3f9 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -20,6 +20,7 @@
#define LIBNBD_INTERNAL_H
#include <stdbool.h>
+#include <stdatomic.h>
#include <string.h>
#include <netdb.h>
#include <sys/types.h>
@@ -79,7 +80,7 @@ struct nbd_handle {
/* Linked list of close callbacks. */
struct close_callback *close_callbacks;
- enum state state; /* State machine. */
+ _Atomic enum state state; /* State machine. */
bool structured_replies; /* If we negotiated NBD_OPT_STRUCTURED_REPLY */
--
2.21.0