]> git.puffer.fish Git - mirror/frr.git/commitdiff
zebra: do not load/store wider-than-ptr atomics
authorDavid Lamparter <equinox@opensourcerouting.org>
Fri, 6 Jan 2023 15:55:38 +0000 (16:55 +0100)
committerDavid Lamparter <equinox@opensourcerouting.org>
Fri, 6 Jan 2023 15:59:02 +0000 (16:59 +0100)
Most 32-bit architectures cannot do atomic loads and stores of data
wider than their pointer size, i.e. 32 bit.  Funnily enough they
generally *can* do a CAS2, i.e., 64-bit compare-and-swap, but while a
CAS can emulate atomic add/bitops, loads and stores aren't available.

Replace with a mutex;  since this is 99% used from the zserv thread, the
mutex should take the local-to-thread fast path anyway.  And while one
atomic might be faster than a mutex lock/unlock, we're doing several
here, and at some point a mutex wins on speed anyway.

This fixes build on armel, mipsel, m68k, powerpc, and sh4.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra/zserv.c
zebra/zserv.h

index d4fa6dadae0422e02f91b37d45d3dc8cffee55be..2024f3453468c8d58c701edb4349d3b80c5a6fe9 100644 (file)
@@ -232,14 +232,16 @@ static void zserv_write(struct thread *thread)
        struct stream *msg;
        uint32_t wcmd = 0;
        struct stream_fifo *cache;
+       uint64_t time_now = monotime(NULL);
 
        /* If we have any data pending, try to flush it first */
        switch (buffer_flush_all(client->wb, client->sock)) {
        case BUFFER_ERROR:
                goto zwrite_fail;
        case BUFFER_PENDING:
-               atomic_store_explicit(&client->last_write_time, monotime(NULL),
-                                     memory_order_relaxed);
+               frr_with_mutex (&client->stats_mtx) {
+                       client->last_write_time = time_now;
+               }
                zserv_client_event(client, ZSERV_CLIENT_WRITE);
                return;
        case BUFFER_EMPTY:
@@ -273,20 +275,19 @@ static void zserv_write(struct thread *thread)
        case BUFFER_ERROR:
                goto zwrite_fail;
        case BUFFER_PENDING:
-               atomic_store_explicit(&client->last_write_time, monotime(NULL),
-                                     memory_order_relaxed);
+               frr_with_mutex (&client->stats_mtx) {
+                       client->last_write_time = time_now;
+               }
                zserv_client_event(client, ZSERV_CLIENT_WRITE);
                return;
        case BUFFER_EMPTY:
                break;
        }
 
-       atomic_store_explicit(&client->last_write_cmd, wcmd,
-                             memory_order_relaxed);
-
-       atomic_store_explicit(&client->last_write_time, monotime(NULL),
-                             memory_order_relaxed);
-
+       frr_with_mutex (&client->stats_mtx) {
+               client->last_write_cmd = wcmd;
+               client->last_write_time = time_now;
+       }
        return;
 
 zwrite_fail:
@@ -433,11 +434,13 @@ static void zserv_read(struct thread *thread)
        }
 
        if (p2p < p2p_orig) {
+               uint64_t time_now = monotime(NULL);
+
                /* update session statistics */
-               atomic_store_explicit(&client->last_read_time, monotime(NULL),
-                                     memory_order_relaxed);
-               atomic_store_explicit(&client->last_read_cmd, hdr.command,
-                                     memory_order_relaxed);
+               frr_with_mutex (&client->stats_mtx) {
+                       client->last_read_time = time_now;
+                       client->last_read_cmd = hdr.command;
+               }
 
                /* publish read packets on client's input queue */
                frr_with_mutex (&client->ibuf_mtx) {
@@ -631,6 +634,7 @@ static void zserv_client_free(struct zserv *client)
                buffer_free(client->wb);
 
        /* Free buffer mutexes */
+       pthread_mutex_destroy(&client->stats_mtx);
        pthread_mutex_destroy(&client->obuf_mtx);
        pthread_mutex_destroy(&client->ibuf_mtx);
 
@@ -751,14 +755,13 @@ static struct zserv *zserv_client_create(int sock)
        client->obuf_fifo = stream_fifo_new();
        client->ibuf_work = stream_new(stream_size);
        client->obuf_work = stream_new(stream_size);
+       client->connect_time = monotime(NULL);
        pthread_mutex_init(&client->ibuf_mtx, NULL);
        pthread_mutex_init(&client->obuf_mtx, NULL);
+       pthread_mutex_init(&client->stats_mtx, NULL);
        client->wb = buffer_new(0);
        TAILQ_INIT(&(client->gr_info_queue));
 
-       atomic_store_explicit(&client->connect_time, monotime(NULL),
-                             memory_order_relaxed);
-
        /* Initialize flags */
        for (afi = AFI_IP; afi < AFI_MAX; afi++) {
                for (i = 0; i < ZEBRA_ROUTE_MAX; i++)
@@ -1019,8 +1022,14 @@ static void zebra_show_client_detail(struct vty *vty, struct zserv *client)
        vty_out(vty, "------------------------ \n");
        vty_out(vty, "FD: %d \n", client->sock);
 
-       connect_time = (time_t) atomic_load_explicit(&client->connect_time,
-                                                    memory_order_relaxed);
+       frr_with_mutex (&client->stats_mtx) {
+               connect_time = client->connect_time;
+               last_read_time = client->last_read_time;
+               last_write_time = client->last_write_time;
+
+               last_read_cmd = client->last_read_cmd;
+               last_write_cmd = client->last_write_cmd;
+       }
 
        vty_out(vty, "Connect Time: %s \n",
                zserv_time_buf(&connect_time, cbuf, ZEBRA_TIME_BUF));
@@ -1041,16 +1050,6 @@ static void zebra_show_client_detail(struct vty *vty, struct zserv *client)
                "Client will %sbe notified about the status of its routes.\n",
                client->notify_owner ? "" : "Not ");
 
-       last_read_time = (time_t)atomic_load_explicit(&client->last_read_time,
-                                                     memory_order_relaxed);
-       last_write_time = (time_t)atomic_load_explicit(&client->last_write_time,
-                                                      memory_order_relaxed);
-
-       last_read_cmd = atomic_load_explicit(&client->last_read_cmd,
-                                            memory_order_relaxed);
-       last_write_cmd = atomic_load_explicit(&client->last_write_cmd,
-                                             memory_order_relaxed);
-
        vty_out(vty, "Last Msg Rx Time: %s \n",
                zserv_time_buf(&last_read_time, rbuf, ZEBRA_TIME_BUF));
        vty_out(vty, "Last Msg Tx Time: %s \n",
@@ -1184,12 +1183,11 @@ static void zebra_show_client_brief(struct vty *vty, struct zserv *client)
        char wbuf[ZEBRA_TIME_BUF];
        time_t connect_time, last_read_time, last_write_time;
 
-       connect_time = (time_t)atomic_load_explicit(&client->connect_time,
-                                                   memory_order_relaxed);
-       last_read_time = (time_t)atomic_load_explicit(&client->last_read_time,
-                                                     memory_order_relaxed);
-       last_write_time = (time_t)atomic_load_explicit(&client->last_write_time,
-                                                      memory_order_relaxed);
+       frr_with_mutex (&client->stats_mtx) {
+               connect_time = client->connect_time;
+               last_read_time = client->last_read_time;
+               last_write_time = client->last_write_time;
+       }
 
        if (client->instance || client->session_id)
                snprintfrr(client_string, sizeof(client_string), "%s[%u:%u]",
index de784e382a09d94cef208bcd9839ccd0a39d8305..cfc33f91699bee75e6ef1eb3261bdc38f2515509 100644 (file)
@@ -215,16 +215,21 @@ struct zserv {
         * relative to last_read_time.
         */
 
+       pthread_mutex_t stats_mtx;
+       /* BEGIN covered by stats_mtx */
+
        /* monotime of client creation */
-       _Atomic uint64_t connect_time;
+       uint64_t connect_time;
        /* monotime of last message received */
-       _Atomic uint64_t last_read_time;
+       uint64_t last_read_time;
        /* monotime of last message sent */
-       _Atomic uint64_t last_write_time;
+       uint64_t last_write_time;
        /* command code of last message read */
-       _Atomic uint64_t last_read_cmd;
+       uint64_t last_read_cmd;
        /* command code of last message written */
-       _Atomic uint64_t last_write_cmd;
+       uint64_t last_write_cmd;
+
+       /* END covered by stats_mtx */
 
        /*
         * Number of instances configured with