[ovs-dev] [urcu-fixes 3/4] ovs-thread: Make caller provide thread name when creating a thread.
Ben Pfaff
blp at nicira.com
Mon Apr 28 16:06:27 UTC 2014
Thread names are occasionally very useful for debugging, but from time to
time we've forgotten to set one. This commit adds the new thread's name
as a parameter to the function to start a thread, to make that mistake
impossible. This also simplifies code, since two function calls become
only one.
This makes a few other changes to the thread creation function:
* Since it is no longer a direct wrapper around a pthread function,
rename it to avoid giving that impression.
* Remove 'pthread_attr_t *' param that every caller supplied as NULL.
* Change 'pthread *' parameter into a return value, for convenience.
The system-stats code hadn't set a thread name, so this fixes that issue.
This patch is a prerequisite for making RCU report the name of a thread
that is blocking RCU synchronization, because the easiest way to do that
gets the name of the thread in ovsrcu_quiesce_end(), which is called before
the thread function is called (so it won't get a name set within the thread
function itself).
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/dpif-netdev.c | 6 +-----
lib/netdev-dpdk.c | 4 +---
lib/ovs-rcu.c | 3 +--
lib/ovs-thread.c | 15 ++++++++++-----
lib/ovs-thread.h | 2 +-
ofproto/ofproto-dpif-monitor.c | 3 +--
ofproto/ofproto-dpif-upcall.c | 26 ++++++++------------------
vswitchd/system-stats.c | 3 ++-
8 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 07f181d..96c5feb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -323,7 +323,6 @@ struct pmd_thread {
pthread_t thread;
int id;
atomic_uint change_seq;
- char *name;
};
/* Interface to netdev-based datapath. */
@@ -1877,8 +1876,6 @@ pmd_thread_main(void *f_)
int poll_cnt;
int i;
- f->name = xasprintf("pmd_%u", ovsthread_id_self());
- set_subprogram_name("%s", f->name);
poll_cnt = 0;
poll_list = NULL;
@@ -1918,7 +1915,6 @@ reload:
}
free(poll_list);
- free(f->name);
return NULL;
}
@@ -1955,7 +1951,7 @@ dp_netdev_set_pmd_threads(struct dp_netdev *dp, int n)
/* Each thread will distribute all devices rx-queues among
* themselves. */
- xpthread_create(&f->thread, NULL, pmd_thread_main, f);
+ f->thread = ovs_thread_create("pmd", pmd_thread_main, f);
}
}
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d9676ed..fd991ab 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -133,8 +133,6 @@ static struct list dpdk_list OVS_GUARDED_BY(dpdk_mutex)
static struct list dpdk_mp_list OVS_GUARDED_BY(dpdk_mutex)
= LIST_INITIALIZER(&dpdk_mp_list);
-static pthread_t watchdog_thread;
-
struct dpdk_mp {
struct rte_mempool *mp;
int mtu;
@@ -1105,7 +1103,7 @@ dpdk_class_init(void)
"[netdev] up|down", 1, 2,
netdev_dpdk_set_admin_state, NULL);
- xpthread_create(&watchdog_thread, NULL, dpdk_watchdog, NULL);
+ ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
return 0;
}
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index b3c434d..c1ac61a 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -99,7 +99,7 @@ ovsrcu_quiesced(void)
} else {
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
if (ovsthread_once_start(&once)) {
- xpthread_create(NULL, NULL, ovsrcu_postpone_thread, NULL);
+ ovs_thread_create("urcu", ovsrcu_postpone_thread, NULL);
ovsthread_once_done(&once);
}
}
@@ -234,7 +234,6 @@ ovsrcu_call_postponed(void)
static void *
ovsrcu_postpone_thread(void *arg OVS_UNUSED)
{
- set_subprogram_name("urcu");
pthread_detach(pthread_self());
for (;;) {
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index f33f5f1..d835b39 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -256,6 +256,7 @@ DEFINE_EXTERN_PER_THREAD_DATA(ovsthread_id, 0);
struct ovsthread_aux {
void *(*start)(void *);
void *arg;
+ char name[16];
};
static void *
@@ -273,13 +274,16 @@ ovsthread_wrapper(void *aux_)
aux = *auxp;
free(auxp);
+ set_subprogram_name("%s%u", aux.name, id);
+
ovsrcu_quiesce_end();
return aux.start(aux.arg);
}
-void
-xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
- void *(*start)(void *), void *arg)
+/* Starts a thread that calls 'start(arg)'. Sets the thread's name to 'name'
+ * (suffixed by its ovsthread_id()). Returns the new thread's pthread_t. */
+pthread_t
+ovs_thread_create(const char *name, void *(*start)(void *), void *arg)
{
struct ovsthread_aux *aux;
pthread_t thread;
@@ -292,12 +296,13 @@ xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
aux = xmalloc(sizeof *aux);
aux->start = start;
aux->arg = arg;
+ ovs_strlcpy(aux->name, name, sizeof aux->name);
- error = pthread_create(threadp ? threadp : &thread, attr,
- ovsthread_wrapper, aux);
+ error = pthread_create(&thread, NULL, ovsthread_wrapper, aux);
if (error) {
ovs_abort(error, "pthread_create failed");
}
+ return thread;
}
bool
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 02a81f7..9527843 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -156,7 +156,7 @@ void xpthread_key_create(pthread_key_t *, void (*destructor)(void *));
void xpthread_key_delete(pthread_key_t);
void xpthread_setspecific(pthread_key_t, const void *);
-void xpthread_create(pthread_t *, pthread_attr_t *, void *(*)(void *), void *);
+pthread_t ovs_thread_create(const char *name, void *(*)(void *), void *);
void xpthread_join(pthread_t, void **);
/* Per-thread data.
diff --git a/ofproto/ofproto-dpif-monitor.c b/ofproto/ofproto-dpif-monitor.c
index b521735..7c84e4b 100644
--- a/ofproto/ofproto-dpif-monitor.c
+++ b/ofproto/ofproto-dpif-monitor.c
@@ -158,7 +158,6 @@ mport_update(struct mport *mport, struct bfd *bfd, struct cfm *cfm,
static void *
monitor_main(void * args OVS_UNUSED)
{
- set_subprogram_name("monitor");
VLOG_INFO("monitor thread created");
while (!latch_is_set(&monitor_exit_latch)) {
monitor_run();
@@ -253,7 +252,7 @@ ofproto_dpif_monitor_port_update(const struct ofport_dpif *ofport,
* terminates it. */
if (!monitor_running && !hmap_is_empty(&monitor_hmap)) {
latch_init(&monitor_exit_latch);
- xpthread_create(&monitor_tid, NULL, monitor_main, NULL);
+ monitor_tid = ovs_thread_create("monitor", monitor_main, NULL);
monitor_running = true;
} else if (monitor_running && hmap_is_empty(&monitor_hmap)) {
latch_set(&monitor_exit_latch);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 84afc56..13f939d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -52,7 +52,6 @@ COVERAGE_DEFINE(upcall_duplicate_flow);
struct handler {
struct udpif *udpif; /* Parent udpif. */
pthread_t thread; /* Thread ID. */
- char *name; /* Thread name. */
uint32_t handler_id; /* Handler id. */
};
@@ -60,9 +59,8 @@ struct handler {
* updates or removes them if necessary. */
struct revalidator {
struct udpif *udpif; /* Parent udpif. */
- char *name; /* Thread name. */
-
pthread_t thread; /* Thread ID. */
+ unsigned int id; /* ovsthread_id_self(). */
struct hmap *ukeys; /* Points into udpif->ukeys for this
revalidator. Used for GC phase. */
};
@@ -310,15 +308,11 @@ udpif_stop_threads(struct udpif *udpif)
/* Delete ukeys, and delete all flows from the datapath to prevent
* double-counting stats. */
revalidator_purge(revalidator);
- free(revalidator->name);
hmap_destroy(&udpif->ukeys[i].hmap);
ovs_mutex_destroy(&udpif->ukeys[i].mutex);
}
- for (i = 0; i < udpif->n_handlers; i++) {
- free(udpif->handlers[i].name);
- }
latch_poll(&udpif->exit_latch);
xpthread_barrier_destroy(&udpif->reval_barrier);
@@ -354,8 +348,8 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
handler->udpif = udpif;
handler->handler_id = i;
- xpthread_create(&handler->thread, NULL, udpif_upcall_handler,
- handler);
+ handler->thread = ovs_thread_create(
+ "handler", udpif_upcall_handler, handler);
}
xpthread_barrier_init(&udpif->reval_barrier, NULL,
@@ -371,8 +365,8 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
hmap_init(&udpif->ukeys[i].hmap);
ovs_mutex_init(&udpif->ukeys[i].mutex);
revalidator->ukeys = &udpif->ukeys[i].hmap;
- xpthread_create(&revalidator->thread, NULL, udpif_revalidator,
- revalidator);
+ revalidator->thread = ovs_thread_create(
+ "revalidator", udpif_revalidator, revalidator);
}
}
}
@@ -522,9 +516,6 @@ udpif_upcall_handler(void *arg)
struct udpif *udpif = handler->udpif;
struct hmap misses = HMAP_INITIALIZER(&misses);
- handler->name = xasprintf("handler_%u", ovsthread_id_self());
- set_subprogram_name("%s", handler->name);
-
while (!latch_is_set(&handler->udpif->exit_latch)) {
struct upcall upcalls[FLOW_MISS_MAX_BATCH];
struct flow_miss miss_buf[FLOW_MISS_MAX_BATCH];
@@ -569,8 +560,7 @@ udpif_revalidator(void *arg)
unsigned int flow_limit = 0;
size_t n_flows = 0;
- revalidator->name = xasprintf("revalidator_%u", ovsthread_id_self());
- set_subprogram_name("%s", revalidator->name);
+ revalidator->id = ovsthread_id_self();
for (;;) {
if (leader) {
uint64_t reval_seq;
@@ -1618,8 +1608,8 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
struct revalidator *revalidator = &udpif->revalidators[i];
ovs_mutex_lock(&udpif->ukeys[i].mutex);
- ds_put_format(&ds, "\t%s: (keys %"PRIuSIZE")\n", revalidator->name,
- hmap_count(&udpif->ukeys[i].hmap));
+ ds_put_format(&ds, "\t%u: (keys %"PRIuSIZE")\n",
+ revalidator->id, hmap_count(&udpif->ukeys[i].hmap));
ovs_mutex_unlock(&udpif->ukeys[i].mutex);
}
}
diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c
index 6c73933..7789787 100644
--- a/vswitchd/system-stats.c
+++ b/vswitchd/system-stats.c
@@ -542,7 +542,8 @@ system_stats_enable(bool enable)
ovs_mutex_lock(&mutex);
if (enable) {
if (!started) {
- xpthread_create(NULL, NULL, system_stats_thread_func, NULL);
+ ovs_thread_create("system_stats",
+ system_stats_thread_func, NULL);
latch_init(&latch);
started = true;
}
--
1.7.10.4
More information about the dev
mailing list