From 9f8b60d74c91f4028b68ea8345a5562768acc81e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 27 Apr 2018 10:05:53 -0400 Subject: [PATCH 1/2] Move or_state_mark_dirty into statefile.c Previously it was an inline function in or.h --- src/or/or.h | 9 --------- src/or/rephist.c | 2 +- src/or/statefile.c | 12 +++++++++++- src/or/statefile.h | 1 + 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index e27f25197b..06262e430a 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4745,15 +4745,6 @@ typedef struct { time_t LastRotatedOnionKey; } or_state_t; -/** Change the next_write time of state to when, unless the - * state is already scheduled to be written to disk earlier than when. - */ -static inline void or_state_mark_dirty(or_state_t *state, time_t when) -{ - if (state->next_write > when) - state->next_write = when; -} - #define MAX_SOCKS_REPLY_LEN 1024 #define MAX_SOCKS_ADDR_LEN 256 #define SOCKS_NO_AUTH 0x00 diff --git a/src/or/rephist.c b/src/or/rephist.c index ac3e9f502e..bac2efb1f4 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -85,8 +85,8 @@ #include "routerlist.h" #include "ht.h" #include "channelpadding.h" - #include "connection_or.h" +#include "statefile.h" static void bw_arrays_init(void); static void predicted_ports_alloc(void); diff --git a/src/or/statefile.c b/src/or/statefile.c index cc114f0a2b..26a631ced0 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -472,7 +472,7 @@ or_state_save(time_t now) tor_assert(global_state); - if (global_state->next_write > now) + if (global_state->next_write >= now) return 0; /* Call everything else that might dirty the state even more, in order @@ -680,6 +680,16 @@ save_transport_to_state(const char *transport, tor_free(transport_addrport); } +/** Change the next_write time of state to when, unless the + * state is already scheduled to be written to disk earlier than when. + */ +void +or_state_mark_dirty(or_state_t *state, time_t when) +{ + if (state->next_write > when) + state->next_write = when; +} + STATIC void or_state_free_(or_state_t *state) { diff --git a/src/or/statefile.h b/src/or/statefile.h index b4cc4d1dc6..5aa2ca9320 100644 --- a/src/or/statefile.h +++ b/src/or/statefile.h @@ -17,6 +17,7 @@ char *get_stored_bindaddr_for_server_transport(const char *transport); int or_state_load(void); int or_state_loaded(void); void or_state_free_all(void); +void or_state_mark_dirty(or_state_t *state, time_t when); #ifdef STATEFILE_PRIVATE STATIC config_line_t *get_transport_in_state_by_name(const char *transport); From 987a7f667698576d745674965973871363ffd719 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 27 Apr 2018 10:30:30 -0400 Subject: [PATCH 2/2] Move responsibility for or_state_save() to a scheduled callback Closes ticket 25948. --- changes/ticket25948 | 9 +++++++++ src/or/main.c | 44 ++++++++++++++++++++++++++++++++++++++------ src/or/main.h | 1 + src/or/statefile.c | 7 +++++-- 4 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 changes/ticket25948 diff --git a/changes/ticket25948 b/changes/ticket25948 new file mode 100644 index 0000000000..7851d0b65b --- /dev/null +++ b/changes/ticket25948 @@ -0,0 +1,9 @@ + o Minor features (mainloop): + - Move responsibility for + saving the state file to disk + from a once-per-second callback to a callback that is only scheduled as + needed. Once enough items are removed from our once-per-second + callback, we can eliminate it entirely to conserve CPU when idle. + Closes ticket + 25948. + diff --git a/src/or/main.c b/src/or/main.c index c1103edb3a..6b166d096b 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1355,6 +1355,7 @@ CALLBACK(retry_listeners); CALLBACK(rotate_onion_key); CALLBACK(rotate_x509_certificate); CALLBACK(save_stability); +CALLBACK(save_state); CALLBACK(write_bridge_ns); CALLBACK(write_stats_file); @@ -1376,6 +1377,7 @@ STATIC periodic_event_item_t periodic_events[] = { CALLBACK(reset_padding_counts, PERIODIC_EVENT_ROLE_ALL, 0), CALLBACK(retry_listeners, PERIODIC_EVENT_ROLE_ALL, PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(save_state, PERIODIC_EVENT_ROLE_ALL, 0), CALLBACK(rotate_x509_certificate, PERIODIC_EVENT_ROLE_ALL, 0), CALLBACK(write_stats_file, PERIODIC_EVENT_ROLE_ALL, 0), @@ -1432,6 +1434,7 @@ static periodic_event_item_t *check_descriptor_event=NULL; static periodic_event_item_t *fetch_networkstatus_event=NULL; static periodic_event_item_t *launch_descriptor_fetches_event=NULL; static periodic_event_item_t *check_dns_honesty_event=NULL; +static periodic_event_item_t *save_state_event=NULL; /** Reset all the periodic events so we'll do all our actions again as if we * just started up. @@ -1528,6 +1531,7 @@ initialize_periodic_events(void) NAMED_CALLBACK(fetch_networkstatus); NAMED_CALLBACK(launch_descriptor_fetches); NAMED_CALLBACK(check_dns_honesty); + NAMED_CALLBACK(save_state); struct timeval one_second = { 1, 0 }; initialize_periodic_events_event = tor_evtimer_new( @@ -1598,8 +1602,9 @@ periodic_events_on_new_options(const or_options_t *options) void reschedule_descriptor_update_check(void) { - tor_assert(check_descriptor_event); - periodic_event_reschedule(check_descriptor_event); + if (check_descriptor_event) { + periodic_event_reschedule(check_descriptor_event); + } } /** @@ -1745,10 +1750,6 @@ run_scheduled_events(time_t now) run_connection_housekeeping(i, now); } - /* 8b. And if anything in our state is ready to get flushed to disk, we - * flush it. */ - or_state_save(now); - /* 11b. check pending unconfigured managed proxies */ if (!net_is_disabled() && pt_proxies_configuration_pending()) pt_configure_remaining_proxies(); @@ -1982,6 +1983,37 @@ check_expired_networkstatus_callback(time_t now, const or_options_t *options) return CHECK_EXPIRED_NS_INTERVAL; } +/** + * Scheduled callback: Save the state file to disk if appropriate. + */ +static int +save_state_callback(time_t now, const or_options_t *options) +{ + (void) options; + (void) or_state_save(now); // only saves if appropriate + const time_t next_write = get_or_state()->next_write; + if (next_write == TIME_MAX) { + return 86400; + } else if (BUG(next_write <= now)) { + /* This can't happen due to clock jumps, since the value of next_write + * is based on the same "now" that we passed to or_state_save(). + */ + return PERIODIC_EVENT_NO_UPDATE; + } else { + return next_write - now; + } +} + +/** Reschedule the event for saving the state file. + * + * Run this when the state becomes dirty. */ +void +reschedule_or_state_save(void) +{ + tor_assert(save_state_event); + periodic_event_reschedule(save_state_event); +} + /** * Periodic callback: Write statistics to disk if appropriate. */ diff --git a/src/or/main.h b/src/or/main.h index 2447339fb5..836dbf1cad 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -61,6 +61,7 @@ void dns_servers_relaunch_checks(void); void reset_all_main_loop_timers(void); void reschedule_descriptor_update_check(void); void reschedule_directory_downloads(void); +void reschedule_or_state_save(void); void mainloop_schedule_postloop_cleanup(void); void rescan_periodic_events(const or_options_t *options); diff --git a/src/or/statefile.c b/src/or/statefile.c index 26a631ced0..c81ea44e06 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -37,6 +37,7 @@ #include "control.h" #include "entrynodes.h" #include "hibernate.h" +#include "main.h" #include "rephist.h" #include "router.h" #include "sandbox.h" @@ -472,7 +473,7 @@ or_state_save(time_t now) tor_assert(global_state); - if (global_state->next_write >= now) + if (global_state->next_write > now) return 0; /* Call everything else that might dirty the state even more, in order @@ -686,8 +687,10 @@ save_transport_to_state(const char *transport, void or_state_mark_dirty(or_state_t *state, time_t when) { - if (state->next_write > when) + if (state->next_write > when) { state->next_write = when; + reschedule_or_state_save(); + } } STATIC void