[sheepdog] [PATCH v2 08/10] sheep: add a checker for thread type
MORITA Kazutaka
morita.kazutaka at gmail.com
Fri Aug 9 05:22:02 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>
---
sheep/Makefile.am | 2 +-
sheep/group.c | 31 ++++++++++++++++---------------
sheep/request.c | 11 ++++-------
sheep/sheep_priv.h | 19 +++++++++++++++++++
sheep/trace/checker.c | 34 ++++++++++++++++++++++++++++++++++
sheep/trace/trace.c | 7 +++++++
sheep/trace/trace.h | 1 +
7 files changed, 82 insertions(+), 23 deletions(-)
create mode 100644 sheep/trace/checker.c
diff --git a/sheep/Makefile.am b/sheep/Makefile.am
index 8b9cc10..bc810be 100644
--- a/sheep/Makefile.am
+++ b/sheep/Makefile.am
@@ -41,7 +41,7 @@ sheep_SOURCES += cluster/zookeeper.c
endif
if BUILD_TRACE
-sheep_SOURCES += trace/trace.c trace/mcount.S trace/graph.c
+sheep_SOURCES += trace/trace.c trace/mcount.S trace/graph.c trace/checker.c
endif
sheep_LDADD = ../lib/libsheepdog.a -lpthread -lm\
diff --git a/sheep/group.c b/sheep/group.c
index 59bb398..8f223f5 100644
--- a/sheep/group.c
+++ b/sheep/group.c
@@ -87,7 +87,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);
@@ -240,7 +240,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;
@@ -268,7 +268,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);
@@ -685,8 +685,8 @@ static void update_cluster_info(const struct cluster_info *cinfo,
* 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);
@@ -733,9 +733,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 cluster_info *cinfo = opaque;
enum sd_status status;
@@ -841,7 +841,7 @@ static void requeue_cluster_request(void)
}
}
-int sd_reconnect_handler(void)
+main_fn int sd_reconnect_handler(void)
{
sys->cinfo.status = SD_STATUS_WAIT;
if (sys->cdrv->init(sys->cdrv_option) != 0)
@@ -873,9 +873,9 @@ static bool cluster_join_check(const struct cluster_info *cinfo)
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 cluster_info *cinfo = opaque;
@@ -901,8 +901,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;
@@ -956,7 +957,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 386390b..bc2a3e7 100644
--- a/sheep/request.c
+++ b/sheep/request.c
@@ -432,13 +432,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);
@@ -502,7 +500,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;
@@ -901,14 +899,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 6c3cc50..ca9c6d9 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_TRACE
+#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;
diff --git a/sheep/trace/checker.c b/sheep/trace/checker.c
new file mode 100644
index 0000000..1c095ef
--- /dev/null
+++ b/sheep/trace/checker.c
@@ -0,0 +1,34 @@
+/*
+ * 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_checker = {
+ .name = "thread_checker",
+
+ .enter = thread_check_enter,
+};
+
+tracer_register(thread_checker);
diff --git a/sheep/trace/trace.c b/sheep/trace/trace.c
index 8742ccf..d7531c5 100644
--- a/sheep/trace/trace.c
+++ b/sheep/trace/trace.c
@@ -331,6 +331,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 */
@@ -347,6 +349,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);
@@ -370,6 +373,10 @@ int trace_init(void)
nop_all_sites();
+#ifdef DEBUG
+ trace_enable("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 e90ee9a..534c3a7 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