corosync-notifyd has exposed an issue with confdb notifications.

The normal state of affairs is:
IPC thread > lock > objdb > lock

objdb notification whilst really useful turn things around:
<middle of big call chain>
objdb > lock > confdb > ipc > lock

This reverse ordering of locks causes a horrible dead lock.

I see this patch as a work around until corosync-2.0
when most of the threads and locking disappear.

This patch adds a pipe to confdb service. When we get a
objdb notification a struct gets written to the pipe.
The poll loop then runs the dispatch in the main thread.
In the dispatch we call the real ipc_dispatch_send().

Signed-off-by: Angus Salkeld <asalk...@redhat.com>
---
 services/confdb.c |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/services/confdb.c b/services/confdb.c
index 56e3ae1..3187718 100644
--- a/services/confdb.c
+++ b/services/confdb.c
@@ -40,6 +40,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <unistd.h>
+#include <poll.h>
 
 #include <corosync/corotypes.h>
 #include <corosync/coroipc_types.h>
@@ -51,6 +52,7 @@
 #include <corosync/lcr/lcr_comp.h>
 #include <corosync/engine/logsys.h>
 #include <corosync/engine/coroapi.h>
+#include <corosync/totem/coropoll.h>
 
 LOGSYS_DECLARE_SUBSYS ("CONFDB");
 
@@ -65,8 +67,20 @@ m2h (mar_uint64_t *m)
 
 static struct corosync_api_v1 *api;
 
+static int notify_pipe[2];
+
+struct confdb_ipc_message_holder {
+       void *conn;
+       void *msg;
+       size_t mlen;
+};
+
 static int confdb_exec_init_fn (
        struct corosync_api_v1 *corosync_api);
+static int confdb_exec_exit_fn(void);
+
+static int objdb_notify_dispatch(hdb_handle_t handle,
+               int fd, int revents, void *data);
 
 static int confdb_lib_init_fn (void *conn);
 static int confdb_lib_exit_fn (void *conn);
@@ -248,6 +262,7 @@ struct corosync_service_engine confdb_service_engine = {
        .lib_engine                             = confdb_lib_engine,
        .lib_engine_count                       = sizeof (confdb_lib_engine) / 
sizeof (struct corosync_lib_handler),
        .exec_init_fn                           = confdb_exec_init_fn,
+       .exec_exit_fn                           = confdb_exec_exit_fn,
 };
 
 /*
@@ -296,6 +311,14 @@ __attribute__ ((constructor)) static void 
corosync_lcr_component_register (void)
        lcr_component_register (&confdb_comp_ver0);
 }
 
+static int confdb_exec_exit_fn(void)
+{
+       poll_dispatch_delete(api->poll_handle_get(), notify_pipe[0]);
+       close(notify_pipe[0]);
+       close(notify_pipe[1]);
+       return 0;
+}
+
 static int confdb_exec_init_fn (
        struct corosync_api_v1 *corosync_api)
 {
@@ -303,7 +326,13 @@ static int confdb_exec_init_fn (
        logsys_subsys_init();
 #endif
        api = corosync_api;
-       return 0;
+
+       if (pipe(notify_pipe) != 0) {
+               return -1;
+       }
+
+       return poll_dispatch_add(api->poll_handle_get(), notify_pipe[0],
+               POLLIN, NULL, objdb_notify_dispatch);
 }
 
 static int confdb_lib_init_fn (void *conn)
@@ -782,6 +811,56 @@ static void message_handler_req_lib_confdb_reload (void 
*conn,
        api->ipc_response_send(conn, &res_lib_confdb_reload, 
sizeof(res_lib_confdb_reload));
 }
 
+static int objdb_notify_dispatch(hdb_handle_t handle,
+               int fd, int revents, void *data)
+{
+       struct confdb_ipc_message_holder holder;
+       ssize_t rc;
+
+       if (revents & POLLHUP) {
+               return -1;
+       }
+retry_read:
+       rc = read(fd, &holder, sizeof(struct confdb_ipc_message_holder));
+       if (rc == -1 && errno == EINTR) {
+               goto retry_read;
+       }
+       if (rc != sizeof(struct confdb_ipc_message_holder)) {
+               return 0;
+       }
+
+       api->ipc_dispatch_send(holder.conn, holder.msg, holder.mlen);
+
+       api->ipc_refcnt_dec(holder.conn);
+
+       free(holder.msg);
+       return 0;
+}
+
+static int32_t ipc_dispatch_send_from_poll_thread(void *conn, const void *msg, 
size_t mlen)
+{
+       struct confdb_ipc_message_holder holder;
+       ssize_t written;
+
+       api->ipc_refcnt_inc(conn);
+
+       holder.conn = conn;
+       holder.msg = malloc(mlen);
+       memcpy(holder.msg, msg, mlen);
+       holder.mlen = mlen;
+
+retry_write:
+       written = write(notify_pipe[1], &holder, sizeof(struct 
confdb_ipc_message_holder));
+       if (written == -1 && errno == EINTR) {
+               goto retry_write;
+       }
+       if (written == sizeof(struct confdb_ipc_message_holder)) {
+               return 0;
+       } else {
+               return -1;
+       }
+}
+
 static void confdb_notify_lib_of_key_change(object_change_type_t change_type,
        hdb_handle_t parent_object_handle,
        hdb_handle_t object_handle,
@@ -809,7 +888,7 @@ static void 
confdb_notify_lib_of_key_change(object_change_type_t change_type,
        memcpy(res.key_value.value, key_value_pt, key_value_len);
        res.key_value.length = key_value_len;
 
-       api->ipc_dispatch_send(priv_data_pt, &res, sizeof(res));
+       ipc_dispatch_send_from_poll_thread(priv_data_pt, &res, sizeof(res));
 }
 
 static void confdb_notify_lib_of_new_object(hdb_handle_t parent_object_handle,
@@ -827,7 +906,7 @@ static void confdb_notify_lib_of_new_object(hdb_handle_t 
parent_object_handle,
        memcpy(res.name.value, name_pt, name_len);
        res.name.length = name_len;
 
-       api->ipc_dispatch_send(priv_data_pt, &res, sizeof(res));
+       ipc_dispatch_send_from_poll_thread(priv_data_pt, &res, sizeof(res));
 }
 
 static void confdb_notify_lib_of_destroyed_object(
@@ -844,7 +923,7 @@ static void confdb_notify_lib_of_destroyed_object(
        memcpy(res.name.value, name_pt, name_len);
        res.name.length = name_len;
 
-       api->ipc_dispatch_send(priv_data_pt, &res, sizeof(res));
+       ipc_dispatch_send_from_poll_thread(priv_data_pt, &res, sizeof(res));
 }
 
 static void confdb_notify_lib_of_reload(objdb_reload_notify_type_t notify_type,
@@ -858,7 +937,7 @@ static void 
confdb_notify_lib_of_reload(objdb_reload_notify_type_t notify_type,
        res.header.error = CS_OK;
        res.type = notify_type;
 
-       api->ipc_dispatch_send(priv_data_pt, &res, sizeof(res));
+       ipc_dispatch_send_from_poll_thread(priv_data_pt, &res, sizeof(res));
 }
 
 
-- 
1.7.4

_______________________________________________
Openais mailing list
Openais@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to