From: Ben Pfaff Date: Sat, 26 Apr 2014 00:46:21 +0000 (-0700) Subject: ovs-thread: Make caller provide thread name when creating a thread. X-Git-Tag: sliver-openvswitch-2.2.90-1~3^2~63 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=8ba0a5227f6e6b50838c157bd303c2d5bf6f4e59 ovs-thread: Make caller provide thread name when creating a thread. 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 is for ovsrcu_quiesce_end() to record the current thread's name. ovsrcu_quiesce_end() is called before the thread function is called, so it won't get a name set within the thread function itself. Setting the thread name earlier, as in this patch, avoids the problem. Signed-off-by: Ben Pfaff Acked-by: Alex Wang --- diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 07f181d1e..96c5feb42 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 d9676eda9..fd991abd6 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 b3c434dbb..c1ac61a4b 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 f33f5f110..d835b3937 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 02a81f7df..9527843f0 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 b5217357a..7c84e4b3a 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 2c37781f7..a2159afa2 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; @@ -1619,8 +1609,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 6c7393368..778978731 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; }