[sheepdog] [PATCH 09/10] sheep: add a checker for thread type
MORITA Kazutaka
morita.kazutaka at gmail.com
Wed Aug 7 17:49:07 CEST 2013
From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
This introduces a two maker "main_fn" and "worker_fn". If the
function must be called in the main thread, add "main_fn" to it. If
the function must be called in the worker thread, add "worker_fn" to
it. The markers are used only when we run configure with
--enable-thread-checker. The check is done at the entry point of the
function upon tracer infrastracture.
Adding the marker is simpler and easier than writing assert. In
addition, programmers can easily understand which function must be
called in the main/worker thread.
Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---
configure.ac | 15 ++++++++++++++-
sheep/Makefile.am | 4 ++++
sheep/group.c | 31 ++++++++++++++++---------------
sheep/request.c | 11 ++++-------
sheep/sheep_priv.h | 22 ++++++++++++++++++++++
sheep/thread_check.c | 37 +++++++++++++++++++++++++++++++++++++
sheep/trace/trace.c | 6 ++++++
sheep/trace/trace.h | 1 +
8 files changed, 104 insertions(+), 23 deletions(-)
create mode 100644 sheep/thread_check.c
diff --git a/configure.ac b/configure.ac
index 49544a4..645f82b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -208,8 +208,13 @@ AC_ARG_WITH([initddir],
[ INITDDIR="$withval" ],
[ INITDDIR="$sysconfdir/init.d" ])
+AC_ARG_ENABLE([thread-check],
+ [ --enable-thread-check : enable thread check],,
+ [ enable_thread_check="${enable_debug}" ],)
+AM_CONDITIONAL(BUILD_THREAD_CHECK, test x$enable_thread_check = xyes)
+
AC_ARG_ENABLE([trace],
- [ --enable-trace : enable trace],,
+ [ --enable-trace : enable trace],,
[ enable_trace="no" ],)
AM_CONDITIONAL(BUILD_TRACE, test x$enable_trace = xyes)
@@ -281,6 +286,14 @@ if test "x${enable_zookeeper}" = xyes; then
PACKAGE_FEATURES="$PACKAGE_FEATURES zookeeper"
fi
+if test "x${enable_thread_check}" = xyes; then
+ AC_MSG_NOTICE([thread check requires tracer feature])
+ enable_trace="yes"
+ AM_CONDITIONAL(BUILD_TRACE, test x$enable_trace = xyes)
+ AC_DEFINE_UNQUOTED([HAVE_THREAD_CHECK], 1, [have thread check])
+ PACKAGE_FEATURES="$PACKAGE_FEATURES thread_check"
+fi
+
if test "x${enable_trace}" = xyes; then
if test "x${enable_coverage}" = xyes; then
AC_MSG_ERROR(tracer cannot be used with coverage options)
diff --git a/sheep/Makefile.am b/sheep/Makefile.am
index 8b9cc10..2ca27a8 100644
--- a/sheep/Makefile.am
+++ b/sheep/Makefile.am
@@ -44,6 +44,10 @@ if BUILD_TRACE
sheep_SOURCES += trace/trace.c trace/mcount.S trace/graph.c
endif
+if BUILD_THREAD_CHECK
+sheep_SOURCES += thread_check.c
+endif
+
sheep_LDADD = ../lib/libsheepdog.a -lpthread -lm\
$(libcpg_LIBS) $(libcfg_LIBS) $(libacrd_LIBS) $(LIBS)
sheep_DEPENDENCIES = ../lib/libsheepdog.a
diff --git a/sheep/group.c b/sheep/group.c
index 474e48a..92d0671 100644
--- a/sheep/group.c
+++ b/sheep/group.c
@@ -116,7 +116,7 @@ struct vnode_info *grab_vnode_info(struct vnode_info *vnode_info)
* this must only be called from the main thread.
* This can return NULL if cluster is not started yet.
*/
-struct vnode_info *get_vnode_info(void)
+main_fn struct vnode_info *get_vnode_info(void)
{
struct vnode_info *cur_vinfo = main_thread_get(current_vnode_info);
@@ -269,7 +269,7 @@ drop:
* Must run in the main thread as it accesses unlocked state like
* sys->pending_list.
*/
-bool sd_block_handler(const struct sd_node *sender)
+main_fn bool sd_block_handler(const struct sd_node *sender)
{
struct request *req;
@@ -297,7 +297,7 @@ bool sd_block_handler(const struct sd_node *sender)
* Must run in the main thread as it access unlocked state like
* sys->pending_list.
*/
-void queue_cluster_request(struct request *req)
+main_fn void queue_cluster_request(struct request *req)
{
int ret;
sd_dprintf("%s (%p)", op_name(req->op), req);
@@ -735,8 +735,8 @@ static void update_cluster_info(const struct join_message *msg,
* Must run in the main thread as it accesses unlocked state like
* sys->pending_list.
*/
-void sd_notify_handler(const struct sd_node *sender, void *data,
- size_t data_len)
+main_fn void sd_notify_handler(const struct sd_node *sender, void *data,
+ size_t data_len)
{
struct vdi_op_message *msg = data;
const struct sd_op_template *op = get_sd_op(msg->req.opcode);
@@ -783,9 +783,9 @@ void sd_notify_handler(const struct sd_node *sender, void *data,
* Return true if the joining node is accepted. At least one nodes in the
* cluster must call this function and succeed in accept of the joining node.
*/
-bool sd_join_handler(const struct sd_node *joining,
- const struct sd_node *nodes, size_t nr_nodes,
- void *opaque)
+main_fn bool sd_join_handler(const struct sd_node *joining,
+ const struct sd_node *nodes, size_t nr_nodes,
+ void *opaque)
{
struct join_message *jm = opaque;
char str[MAX_NODE_STR_LEN];
@@ -901,7 +901,7 @@ static void requeue_cluster_request(void)
}
}
-int sd_reconnect_handler(void)
+main_fn int sd_reconnect_handler(void)
{
sys->status = SD_STATUS_WAIT;
if (sys->cdrv->init(sys->cdrv_option) != 0)
@@ -933,9 +933,9 @@ static bool cluster_join_check(const struct join_message *jm)
return true;
}
-void sd_accept_handler(const struct sd_node *joined,
- const struct sd_node *members, size_t nr_members,
- const void *opaque)
+main_fn void sd_accept_handler(const struct sd_node *joined,
+ const struct sd_node *members, size_t nr_members,
+ const void *opaque)
{
int i;
const struct join_message *jm = opaque;
@@ -961,8 +961,9 @@ void sd_accept_handler(const struct sd_node *joined,
sd_printf(SDOG_DEBUG, "join Sheepdog cluster");
}
-void sd_leave_handler(const struct sd_node *left, const struct sd_node *members,
- size_t nr_members)
+main_fn void sd_leave_handler(const struct sd_node *left,
+ const struct sd_node *members,
+ size_t nr_members)
{
struct vnode_info *old_vnode_info;
int i, ret;
@@ -1023,7 +1024,7 @@ static void kick_node_recover(void)
put_vnode_info(old);
}
-void sd_update_node_handler(struct sd_node *node)
+main_fn void sd_update_node_handler(struct sd_node *node)
{
update_node_size(node);
kick_node_recover();
diff --git a/sheep/request.c b/sheep/request.c
index 1cc5a9d..ebd4f26 100644
--- a/sheep/request.c
+++ b/sheep/request.c
@@ -438,13 +438,11 @@ static void free_local_request(struct request *req)
* This function takes advantage of gateway's retry mechanism and can be only
* called from worker thread.
*/
-int exec_local_req(struct sd_req *rq, void *data)
+worker_fn int exec_local_req(struct sd_req *rq, void *data)
{
struct request *req;
int ret;
- assert(is_worker_thread());
-
req = alloc_local_request(data, rq->data_length);
req->rq = *rq;
req->local_req_efd = eventfd(0, 0);
@@ -508,7 +506,7 @@ static void free_request(struct request *req)
free(req);
}
-void put_request(struct request *req)
+main_fn void put_request(struct request *req)
{
struct client_info *ci = req->ci;
@@ -907,14 +905,13 @@ void local_req_init(void)
register_event(sys->local_req_efd, local_req_handler, NULL);
}
-int sheep_exec_req(const struct node_id *nid, struct sd_req *hdr, void *buf)
+worker_fn int sheep_exec_req(const struct node_id *nid, struct sd_req *hdr,
+ void *buf)
{
struct sd_rsp *rsp = (struct sd_rsp *)hdr;
struct sockfd *sfd;
int ret;
- assert(is_worker_thread());
-
sfd = sockfd_cache_get(nid);
if (!sfd)
return SD_RES_NETWORK_ERROR;
diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h
index d54b85e..19d0b12 100644
--- a/sheep/sheep_priv.h
+++ b/sheep/sheep_priv.h
@@ -43,6 +43,25 @@
#include "config.h"
#include "sockfd_cache.h"
+ /*
+ * Functions that update global info must be called in the main
+ * thread. Add main_fn markers to such functions.
+ *
+ * Functions that can sleep (e.g. disk I/Os or network I/Os) must be
+ * called in the worker threads. Add worker_fn markers to such
+ * functions.
+ */
+#ifdef HAVE_THREAD_CHECK
+#define MAIN_FN_SECTION ".sd_main"
+#define WORKER_FN_SECTION ".sd_worker"
+
+#define main_fn __attribute__((section(MAIN_FN_SECTION)))
+#define worker_fn __attribute__((section(WORKER_FN_SECTION)))
+#else
+#define main_fn
+#define worker_fn
+#endif
+
struct client_info {
struct connection conn;
@@ -435,4 +454,7 @@ static inline int http_init(const char *address)
}
#endif /* END BUILD_HTTP */
+/* thread_check.c */
+void register_thread_checker(void);
+
#endif
diff --git a/sheep/thread_check.c b/sheep/thread_check.c
new file mode 100644
index 0000000..8a63b0d
--- /dev/null
+++ b/sheep/thread_check.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2013 Nippon Telegraph and Telephone Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "sheep_priv.h"
+#include "trace/trace.h"
+
+static void thread_check_enter(const struct caller *this_fn, int depth)
+{
+ if (strcmp(this_fn->section, MAIN_FN_SECTION) == 0) {
+ if (!is_main_thread())
+ panic("%s must be called in main thread",
+ this_fn->name);
+ } else if (strcmp(this_fn->section, WORKER_FN_SECTION) == 0) {
+ if (!is_worker_thread())
+ panic("%s must be called in worker thread",
+ this_fn->name);
+ }
+}
+
+static struct tracer thread_check_tracer = {
+ .name = "thread check",
+
+ .enter = thread_check_enter,
+};
+
+void register_thread_checker(void)
+{
+ trace_register(&thread_check_tracer);
+}
diff --git a/sheep/trace/trace.c b/sheep/trace/trace.c
index 10acfe1..a5911a3 100644
--- a/sheep/trace/trace.c
+++ b/sheep/trace/trace.c
@@ -344,6 +344,8 @@ static int init_callers(void)
asymbol *sym = symtab[i];
unsigned long ip, addr = bfd_asymbol_value(sym);
const char *name = bfd_asymbol_name(sym);
+ const char *section =
+ bfd_get_section_name(abfd, bfd_get_section(sym));
if (addr == 0 || !(sym->flags & BSF_FUNCTION))
/* sym is not a function */
@@ -360,6 +362,7 @@ static int init_callers(void)
callers[nr_callers].addr = addr;
callers[nr_callers].mcount = ip;
callers[nr_callers].name = strdup(name);
+ callers[nr_callers].section = strdup(section);
nr_callers++;
}
xqsort(callers, nr_callers, caller_cmp);
@@ -383,6 +386,9 @@ int trace_init(void)
nop_all_sites();
+#ifdef HAVE_THREAD_CHECK
+ register_thread_checker();
+#endif
nr_cpu = sysconf(_SC_NPROCESSORS_ONLN);
buffer = xzalloc(sizeof(*buffer) * nr_cpu);
buffer_lock = xzalloc(sizeof(*buffer_lock) * nr_cpu);
diff --git a/sheep/trace/trace.h b/sheep/trace/trace.h
index a6e7fc8..dc85298 100644
--- a/sheep/trace/trace.h
+++ b/sheep/trace/trace.h
@@ -11,6 +11,7 @@ struct caller {
unsigned long addr;
unsigned long mcount;
const char *name;
+ const char *section;
};
struct tracer {
--
1.7.9.5
More information about the sheepdog
mailing list