Call poll at a fixed interval. 63/6363/5
authorTim Newsome <tim@sifive.com>
Fri, 9 Jul 2021 19:58:07 +0000 (12:58 -0700)
committerAntonio Borneo <borneo.antonio@gmail.com>
Sat, 31 Jul 2021 09:08:47 +0000 (10:08 +0100)
The existing implementation blocks in select() for a fixed amount of
time. This change tracks when the next event (likely poll()) wants to be
run, and uses a shorter timeout in select() if necessary.

Also track all these timeouts using milliseconds as returned by
timeval_ms() instead of `struct timeval` to simplify the code.

This feature is helpful if poll() wants to do something like sample PCs
or memory values for basically the entire time that otherwise OpenOCD
would be hung in select(). See
https://github.com/riscv/riscv-openocd/pull/541 for an example of that.
The RISC-V code using this change will be upstreamed some day, too.

Signed-off-by: Tim Newsome <tim@sifive.com>
Change-Id: I67104a7cf69ed07c8399c14aa55963fc5116a67d
Reviewed-on: http://openocd.zylin.com/6363
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
src/server/server.c
src/target/target.c
src/target/target.h

index 6f0b23caac407cc092112480bb2ccb2cf580a3b9..64acd36894ab666f82c1667cf32a7d07baa213d0 100644 (file)
@@ -437,6 +437,8 @@ int server_loop(struct command_context *command_context)
        /* used in accept() */
        int retval;
 
+       int64_t next_event = timeval_ms() + polling_period;
+
 #ifndef _WIN32
        if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
                LOG_ERROR("couldn't set SIGPIPE to SIG_IGN");
@@ -478,7 +480,12 @@ int server_loop(struct command_context *command_context)
                        retval = socket_select(fd_max + 1, &read_fds, NULL, NULL, &tv);
                } else {
                        /* Every 100ms, can be changed with "poll_period" command */
-                       tv.tv_usec = polling_period * 1000;
+                       int timeout_ms = next_event - timeval_ms();
+                       if (timeout_ms < 0)
+                               timeout_ms = 0;
+                       else if (timeout_ms > polling_period)
+                               timeout_ms = polling_period;
+                       tv.tv_usec = timeout_ms * 1000;
                        /* Only while we're sleeping we'll let others run */
                        openocd_sleep_prelude();
                        kept_alive();
@@ -511,7 +518,8 @@ int server_loop(struct command_context *command_context)
                if (retval == 0) {
                        /* We only execute these callbacks when there was nothing to do or we timed
                         *out */
-                       target_call_timer_callbacks();
+                       target_call_timer_callbacks_now();
+                       next_event = target_timer_next_event();
                        process_jim_events(command_context);
 
                        FD_ZERO(&read_fds);     /* eCos leaves read_fds unchanged in this case!  */
index cf1873c5ea1946adcd750906d9522c7303d8e83e..a67712009d3d24320baa491d434350aa35e3490b 100644 (file)
@@ -154,9 +154,10 @@ static struct target_type *target_types[] = {
 struct target *all_targets;
 static struct target_event_callback *target_event_callbacks;
 static struct target_timer_callback *target_timer_callbacks;
+static int64_t target_timer_next_event_value;
 static LIST_HEAD(target_reset_callback_list);
 static LIST_HEAD(target_trace_callback_list);
-static const int polling_interval = 100;
+static const int polling_interval = TARGET_DEFAULT_POLLING_INTERVAL;
 
 static const struct jim_nvp nvp_assert[] = {
        { .name = "assert", NVP_ASSERT },
@@ -1733,8 +1734,8 @@ int target_register_timer_callback(int (*callback)(void *priv),
        (*callbacks_p)->time_ms = time_ms;
        (*callbacks_p)->removed = false;
 
-       gettimeofday(&(*callbacks_p)->when, NULL);
-       timeval_add_time(&(*callbacks_p)->when, 0, time_ms * 1000);
+       (*callbacks_p)->when = timeval_ms() + time_ms;
+       target_timer_next_event_value = MIN(target_timer_next_event_value, (*callbacks_p)->when);
 
        (*callbacks_p)->priv = priv;
        (*callbacks_p)->next = NULL;
@@ -1868,15 +1869,14 @@ int target_call_trace_callbacks(struct target *target, size_t len, uint8_t *data
 }
 
 static int target_timer_callback_periodic_restart(
-               struct target_timer_callback *cb, struct timeval *now)
+               struct target_timer_callback *cb, int64_t *now)
 {
-       cb->when = *now;
-       timeval_add_time(&cb->when, 0, cb->time_ms * 1000L);
+       cb->when = *now + cb->time_ms;
        return ERROR_OK;
 }
 
 static int target_call_timer_callback(struct target_timer_callback *cb,
-               struct timeval *now)
+               int64_t *now)
 {
        cb->callback(cb->priv);
 
@@ -1898,8 +1898,12 @@ static int target_call_timer_callbacks_check_time(int checktime)
 
        keep_alive();
 
-       struct timeval now;
-       gettimeofday(&now, NULL);
+       int64_t now = timeval_ms();
+
+       /* Initialize to a default value that's a ways into the future.
+        * The loop below will make it closer to now if there are
+        * callbacks that want to be called sooner. */
+       target_timer_next_event_value = now + 1000;
 
        /* Store an address of the place containing a pointer to the
         * next item; initially, that's a standalone "root of the
@@ -1915,11 +1919,14 @@ static int target_call_timer_callbacks_check_time(int checktime)
 
                bool call_it = (*callback)->callback &&
                        ((!checktime && (*callback)->type == TARGET_TIMER_TYPE_PERIODIC) ||
-                        timeval_compare(&now, &(*callback)->when) >= 0);
+                        now >= (*callback)->when);
 
                if (call_it)
                        target_call_timer_callback(*callback, &now);
 
+               if (!(*callback)->removed && (*callback)->when < target_timer_next_event_value)
+                       target_timer_next_event_value = (*callback)->when;
+
                callback = &(*callback)->next;
        }
 
@@ -1927,17 +1934,22 @@ static int target_call_timer_callbacks_check_time(int checktime)
        return ERROR_OK;
 }
 
-int target_call_timer_callbacks(void)
+int target_call_timer_callbacks()
 {
        return target_call_timer_callbacks_check_time(1);
 }
 
 /* invoke periodic callbacks immediately */
-int target_call_timer_callbacks_now(void)
+int target_call_timer_callbacks_now()
 {
        return target_call_timer_callbacks_check_time(0);
 }
 
+int64_t target_timer_next_event(void)
+{
+       return target_timer_next_event_value;
+}
+
 /* Prints the working area layout for debug purposes */
 static void print_wa_layout(struct target *target)
 {
index 18a9516f52867a9518c5a33504bd2a698c643a0e..1e19434e45ded370fee77c6a7945397853cc6de5 100644 (file)
@@ -333,7 +333,7 @@ struct target_timer_callback {
        unsigned int time_ms;
        enum target_timer_type type;
        bool removed;
-       struct timeval when;
+       int64_t when;   /* output of timeval_ms() */
        void *priv;
        struct target_timer_callback *next;
 };
@@ -407,6 +407,11 @@ int target_call_timer_callbacks(void);
  * a synchronous command completes.
  */
 int target_call_timer_callbacks_now(void);
+/**
+ * Returns when the next registered event will take place. Callers can use this
+ * to go to sleep until that time occurs.
+ */
+int64_t target_timer_next_event(void);
 
 struct target *get_target_by_num(int num);
 struct target *get_current_target(struct command_context *cmd_ctx);
@@ -790,4 +795,6 @@ int target_profiling_default(struct target *target, uint32_t *samples, uint32_t
 
 extern bool get_target_reset_nag(void);
 
+#define TARGET_DEFAULT_POLLING_INTERVAL                100
+
 #endif /* OPENOCD_TARGET_TARGET_H */

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)