lib: some frr_pthread fixes

* Use atomic fixed-width thread identifiers
* Add ability to change thread name at runtime

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index c06339aad9e4616ee477607e07f8902db553f83c..ed0a12f052513e520aff284d9a7ce8b802a1aeaa 100644 (file)
--- 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 72b47ae5c32ad76a1c6bbabc17abc988d98820db..ba6d89429aa3c58575537ccb387f72d9cbda58bc 100644 (file)
--- 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 2cc50196a897b81ca103c0c6f3c6ba56314653ae..3990e2a4ceb4a728b156f05a00a3aa6f267729f2 100644 (file)
--- 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 9d64663d9c0de829aba030b68c61569e856a7a77..868bfd0e9b4073291191aff98d4210cb455924db 100644 (file)
--- 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 c830446e10209f4ead167c06729fae3d757eef41..f7c110914dcaa1105615b523d9c9fc0add8f3dc7 100644 (file)
--- 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 *);