From: Quentin Young Date: Mon, 5 Mar 2018 22:58:22 +0000 (-0500) Subject: lib: some frr_pthread fixes X-Git-Tag: frr-5.0-dev~166^2~2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=d8a8a8de00ca28879eb865cd4f25e362570a910e;p=matthieu%2Ffrr.git lib: some frr_pthread fixes * Use atomic fixed-width thread identifiers * Add ability to change thread name at runtime Signed-off-by: Quentin Young --- diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index c06339aad9..ed0a12f052 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -7528,16 +7528,14 @@ static void bgp_pthreads_init() .id = PTHREAD_IO, .start = frr_pthread_attr_default.start, .stop = frr_pthread_attr_default.stop, - .name = "BGP I/O thread", }; struct frr_pthread_attr ka = { .id = PTHREAD_KEEPALIVES, .start = bgp_keepalives_start, .stop = bgp_keepalives_stop, - .name = "BGP Keepalives thread", }; - frr_pthread_new(&io); - frr_pthread_new(&ka); + frr_pthread_new(&io, "BGP I/O thread"); + frr_pthread_new(&ka, "BGP Keepalives thread"); } void bgp_pthreads_run() diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c index 72b47ae5c3..ba6d89429a 100644 --- a/lib/frr_pthread.c +++ b/lib/frr_pthread.c @@ -1,6 +1,6 @@ /* * Utilities and interfaces for managing POSIX threads within FRR. - * Copyright (C) 2017 Cumulus Networks + * Copyright (C) 2017 Cumulus Networks, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -29,7 +29,7 @@ DEFINE_MTYPE(LIB, FRR_PTHREAD, "FRR POSIX Thread"); DEFINE_MTYPE(LIB, PTHREAD_PRIM, "POSIX synchronization primitives"); /* id for next created pthread */ -static unsigned int next_id = 0; +static _Atomic uint32_t next_id = 0; /* default frr_pthread start/stop routine prototypes */ static void *fpt_run(void *arg); @@ -40,7 +40,6 @@ struct frr_pthread_attr frr_pthread_attr_default = { .id = 0, .start = fpt_run, .stop = fpt_halt, - .name = "Anonymous", }; /* hash table to keep track of all frr_pthreads */ @@ -85,9 +84,10 @@ void frr_pthread_finish() pthread_mutex_unlock(&frr_pthread_hash_mtx); } -struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr) +struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, + const char *name) { - static struct frr_pthread holder = {0}; + static struct frr_pthread holder = {}; struct frr_pthread *fpt = NULL; attr = attr ? attr : &frr_pthread_attr_default; @@ -99,10 +99,14 @@ struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr) if (!hash_lookup(frr_pthread_hash, &holder)) { fpt = XCALLOC(MTYPE_FRR_PTHREAD, sizeof(struct frr_pthread)); + /* initialize mutex */ + pthread_mutex_init(&fpt->mtx, NULL); /* create new thread master */ - fpt->master = thread_master_create(attr->name); + fpt->master = thread_master_create(name); /* set attributes */ fpt->attr = *attr; + name = (name ? name : "Anonymous thread"); + fpt->name = XSTRDUP(MTYPE_FRR_PTHREAD, name); if (attr == &frr_pthread_attr_default) fpt->attr.id = frr_pthread_get_id(); /* initialize startup synchronization primitives */ @@ -126,16 +130,31 @@ void frr_pthread_destroy(struct frr_pthread *fpt) { thread_master_free(fpt->master); + pthread_mutex_destroy(&fpt->mtx); pthread_mutex_destroy(fpt->running_cond_mtx); pthread_cond_destroy(fpt->running_cond); + if (fpt->name) + XFREE(MTYPE_FRR_PTHREAD, fpt->name); XFREE(MTYPE_PTHREAD_PRIM, fpt->running_cond_mtx); XFREE(MTYPE_PTHREAD_PRIM, fpt->running_cond); XFREE(MTYPE_FRR_PTHREAD, fpt); } +void frr_pthread_set_name(struct frr_pthread *fpt, const char *name) +{ + pthread_mutex_lock(&fpt->mtx); + { + if (fpt->name) + XFREE(MTYPE_FRR_PTHREAD, fpt->name); + fpt->name = XSTRDUP(MTYPE_FRR_PTHREAD, name); + } + pthread_mutex_unlock(&fpt->mtx); + thread_master_set_name(fpt->master, name); +} + struct frr_pthread *frr_pthread_get(unsigned int id) { - static struct frr_pthread holder = {0}; + static struct frr_pthread holder = {}; struct frr_pthread *fpt; pthread_mutex_lock(&frr_pthread_hash_mtx); @@ -210,11 +229,13 @@ void frr_pthread_stop_all() pthread_mutex_unlock(&frr_pthread_hash_mtx); } -unsigned int frr_pthread_get_id() +uint32_t frr_pthread_get_id() { + _Atomic uint32_t nxid; + nxid = atomic_fetch_add_explicit(&next_id, 1, memory_order_seq_cst); /* just a sanity check, this should never happen */ - assert(next_id <= INT_MAX - 1); - return next_id++; + assert(nxid <= (INT_MAX - 1)); + return nxid; } void frr_pthread_yield(void) diff --git a/lib/frr_pthread.h b/lib/frr_pthread.h index 2cc50196a8..3990e2a4ce 100644 --- a/lib/frr_pthread.h +++ b/lib/frr_pthread.h @@ -1,6 +1,6 @@ /* * Utilities and interfaces for managing POSIX threads within FRR. - * Copyright (C) 2017 Cumulus Networks + * Copyright (C) 2017 Cumulus Networks, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -32,14 +32,19 @@ struct frr_pthread; struct frr_pthread_attr; struct frr_pthread_attr { - int id; + _Atomic uint32_t id; void *(*start)(void *); int (*stop)(struct frr_pthread *, void **); - const char *name; }; struct frr_pthread { + /* + * Mutex protecting this structure. Must be taken for reading some + * fields, denoted by a 'Requires: mtx'. + */ + pthread_mutex_t mtx; + /* pthread id */ pthread_t thread; @@ -73,8 +78,17 @@ struct frr_pthread { * Fake thread-specific storage. No constraints on usage. Helpful when * creating reentrant pthread implementations. Can be used to pass * argument to pthread entry function. + * + * Requires: mtx */ void *data; + + /* + * Human-readable thread name. + * + * Requires: mtx + */ + char *name; }; extern struct frr_pthread_attr frr_pthread_attr_default; @@ -107,9 +121,19 @@ void frr_pthread_finish(void); * frr_pthread will cause them to run on that pthread. * * @param attr - the thread attributes + * @param name - Human-readable name * @return the created frr_pthread upon success, or NULL upon failure */ -struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr); +struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, + const char *name); + +/* + * Changes the name of the frr_pthread. + * + * @param fpt - the frr_pthread to operate on + * @param name - Human-readable name + */ +void frr_pthread_set_name(struct frr_pthread *fpt, const char *name); /* * Destroys an frr_pthread. @@ -198,6 +222,6 @@ void frr_pthread_yield(void); * * @return unique identifier */ -unsigned int frr_pthread_get_id(void); +uint32_t frr_pthread_get_id(void); #endif /* _FRR_PTHREAD_H */ diff --git a/lib/thread.c b/lib/thread.c index 9d64663d9c..868bfd0e9b 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -343,7 +343,6 @@ static void initializer() pthread_key_create(&thread_current, NULL); } -/* Allocate new thread master. */ struct thread_master *thread_master_create(const char *name) { struct thread_master *rv; @@ -427,6 +426,17 @@ struct thread_master *thread_master_create(const char *name) return rv; } +void thread_master_set_name(struct thread_master *master, const char *name) +{ + pthread_mutex_lock(&master->mtx); + { + if (master->name) + XFREE(MTYPE_THREAD_MASTER, master->name); + master->name = XSTRDUP(MTYPE_THREAD_MASTER, name); + } + pthread_mutex_unlock(&master->mtx); +} + /* Add a new thread to the list. */ static void thread_list_add(struct thread_list *list, struct thread *thread) { diff --git a/lib/thread.h b/lib/thread.h index c830446e10..f7c110914d 100644 --- a/lib/thread.h +++ b/lib/thread.h @@ -174,6 +174,7 @@ struct cpu_thread_history { /* Prototypes. */ extern struct thread_master *thread_master_create(const char *); +void thread_master_set_name(struct thread_master *master, const char *name); extern void thread_master_free(struct thread_master *); extern void thread_master_free_unused(struct thread_master *);