From bd42367a1e6f60d35242d2a5165f10e3a8623bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Tue, 3 Apr 2018 17:43:17 +0200 Subject: [PATCH 1/3] Make get_total_system_memory mockable. This patch makes get_total_system_memory mockable, which allows us to alter the return value of the function in tests. See: https://bugs.torproject.org/24782 --- src/common/compat.c | 4 ++-- src/common/compat.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 4cb346dfa..7d9add50b 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -3409,8 +3409,8 @@ get_total_system_memory_impl(void) * Try to find out how much physical memory the system has. On success, * return 0 and set *mem_out to that value. On failure, return -1. */ -int -get_total_system_memory(size_t *mem_out) +MOCK_IMPL(int, +get_total_system_memory, (size_t *mem_out)) { static size_t mem_cached=0; uint64_t m = get_total_system_memory_impl(); diff --git a/src/common/compat.h b/src/common/compat.h index 93301feda..3088e6835 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -699,7 +699,7 @@ char *make_path_absolute(char *fname); char **get_environment(void); -int get_total_system_memory(size_t *mem_out); +MOCK_DECL(int, get_total_system_memory, (size_t *mem_out)); int compute_num_cpus(void); From 5633a633796ca10e2866ff6fd5bc40b5f7895c30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Tue, 3 Apr 2018 17:44:42 +0200 Subject: [PATCH 2/3] Use STATIC for compute_real_max_mem_in_queues This patch makes compute_real_max_mem_in_queues use the STATIC macro, which allows us to test the function. See: https://bugs.torproject.org/24782 --- src/or/config.c | 4 +--- src/or/config.h | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index cec14e0f5..1f8ff5e3a 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -773,8 +773,6 @@ static void config_maybe_load_geoip_files_(const or_options_t *options, static int options_validate_cb(void *old_options, void *options, void *default_options, int from_setconf, char **msg); -static uint64_t compute_real_max_mem_in_queues(const uint64_t val, - int log_guess); static void cleanup_protocol_warning_severity_level(void); static void set_protocol_warning_severity_level(int warning_severity); @@ -4522,7 +4520,7 @@ options_validate(or_options_t *old_options, or_options_t *options, /* Given the value that the user has set for MaxMemInQueues, compute the * actual maximum value. We clip this value if it's too low, and autodetect * it if it's set to 0. */ -static uint64_t +STATIC uint64_t compute_real_max_mem_in_queues(const uint64_t val, int log_guess) { uint64_t result; diff --git a/src/or/config.h b/src/or/config.h index 2f23809b2..f6a659d0c 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -258,6 +258,10 @@ STATIC int parse_port_config(smartlist_t *out, const unsigned flags); STATIC int check_bridge_distribution_setting(const char *bd); + +STATIC uint64_t compute_real_max_mem_in_queues(const uint64_t val, + int log_guess); + #endif /* defined(CONFIG_PRIVATE) */ #endif /* !defined(TOR_CONFIG_H) */ From 31508a0abccfee1cda0869a9a7d22df74d6b67b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Tue, 3 Apr 2018 18:26:52 +0200 Subject: [PATCH 3/3] Use less memory for MaxMemInQueues for machines with more than 8 GB of RAM. This patch changes the algorithm of compute_real_max_mem_in_queues() to use 0.4 * RAM iff the system has more than or equal to 8 GB of RAM, but will continue to use the old value of 0.75 * RAM if the system have less than * GB of RAM available. This patch also adds tests for compute_real_max_mem_in_queues(). See: https://bugs.torproject.org/24782 --- changes/bug24782 | 4 ++ src/or/config.c | 31 +++++++++++----- src/or/config.h | 7 ++++ src/test/test_config.c | 83 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 changes/bug24782 diff --git a/changes/bug24782 b/changes/bug24782 new file mode 100644 index 000000000..59bbdad12 --- /dev/null +++ b/changes/bug24782 @@ -0,0 +1,4 @@ + o Minor features (config options): + - Change the way the default value for MaxMemInQueues is calculated. We now + use 0.4 * RAM if the system have 8 GB RAM or more, otherwise we use the + former value of 0.75 * RAM. Closes ticket 24782. diff --git a/src/or/config.c b/src/or/config.c index 1f8ff5e3a..6b8885521 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4528,11 +4528,6 @@ compute_real_max_mem_in_queues(const uint64_t val, int log_guess) if (val == 0) { #define ONE_GIGABYTE (U64_LITERAL(1) << 30) #define ONE_MEGABYTE (U64_LITERAL(1) << 20) -#if SIZEOF_VOID_P >= 8 -#define MAX_DEFAULT_MAXMEM (8*ONE_GIGABYTE) -#else -#define MAX_DEFAULT_MAXMEM (2*ONE_GIGABYTE) -#endif /* The user didn't pick a memory limit. Choose a very large one * that is still smaller than the system memory */ static int notice_sent = 0; @@ -4547,14 +4542,30 @@ compute_real_max_mem_in_queues(const uint64_t val, int log_guess) result = ONE_GIGABYTE; #endif /* SIZEOF_VOID_P >= 8 */ } else { - /* We detected it, so let's pick 3/4 of the total RAM as our limit. */ - const uint64_t avail = (ram / 4) * 3; + /* We detected the amount of memory available. */ + uint64_t avail = 0; - /* Make sure it's in range from 0.25 GB to 8 GB. */ - if (avail > MAX_DEFAULT_MAXMEM) { + if (ram >= (8 * ONE_GIGABYTE)) { + /* If we have 8 GB, or more, RAM available, we set the MaxMemInQueues + * to 0.4 * RAM. The idea behind this value is that the amount of RAM + * is more than enough for a single relay and should allow the relay + * operator to run two relays if they have additional bandwidth + * available. + */ + avail = (ram / 5) * 2; + } else { + /* If we have less than 8 GB of RAM available, we use the "old" default + * for MaxMemInQueues of 0.75 * RAM. + */ + avail = (ram / 4) * 3; + } + + /* Make sure it's in range from 0.25 GB to 8 GB for 64-bit and 0.25 to 2 + * GB for 32-bit. */ + if (avail > MAX_DEFAULT_MEMORY_QUEUE_SIZE) { /* If you want to use more than this much RAM, you need to configure it yourself */ - result = MAX_DEFAULT_MAXMEM; + result = MAX_DEFAULT_MEMORY_QUEUE_SIZE; } else if (avail < ONE_GIGABYTE / 4) { result = ONE_GIGABYTE / 4; } else { diff --git a/src/or/config.h b/src/or/config.h index f6a659d0c..1d3c27217 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -22,6 +22,13 @@ * expose more information than we're comfortable with. */ #define MIN_HEARTBEAT_PERIOD (30*60) +/** Maximum default value for MaxMemInQueues, in bytes. */ +#if SIZEOF_VOID_P >= 8 +#define MAX_DEFAULT_MEMORY_QUEUE_SIZE (U64_LITERAL(8) << 30) +#else +#define MAX_DEFAULT_MEMORY_QUEUE_SIZE (U64_LITERAL(2) << 30) +#endif + MOCK_DECL(const char*, get_dirportfrontpage, (void)); MOCK_DECL(const or_options_t *, get_options, (void)); MOCK_DECL(or_options_t *, get_options_mutable, (void)); diff --git a/src/test/test_config.c b/src/test/test_config.c index d42335de3..d82f6d21d 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -585,6 +585,22 @@ test_config_parse_transport_options_line(void *arg) } } +/* Mocks needed for the compute_max_mem_in_queues test */ +static int get_total_system_memory_mock(size_t *mem_out); + +static size_t total_system_memory_output = 0; +static int total_system_memory_return = 0; + +static int +get_total_system_memory_mock(size_t *mem_out) +{ + if (! mem_out) + return -1; + + *mem_out = total_system_memory_output; + return total_system_memory_return; +} + /* Mocks needed for the transport plugin line test */ static void pt_kickstart_proxy_mock(const smartlist_t *transport_list, @@ -5596,6 +5612,72 @@ test_config_include_opened_file_list(void *data) tor_free(dir); } +static void +test_config_compute_max_mem_in_queues(void *data) +{ +#define GIGABYTE(x) (U64_LITERAL(x) << 30) +#define MEGABYTE(x) (U64_LITERAL(x) << 20) + (void)data; + MOCK(get_total_system_memory, get_total_system_memory_mock); + + /* We are unable to detect the amount of memory on the system. Tor will try + * to use some sensible default values for 64-bit and 32-bit systems. */ + total_system_memory_return = -1; + +#if SIZEOF_VOID_P >= 8 + /* We are on a 64-bit system. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, GIGABYTE(8)); +#else + /* We are on a 32-bit system. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, GIGABYTE(1)); +#endif + + /* We are able to detect the amount of RAM on the system. */ + total_system_memory_return = 0; + + /* We are running on a system with one gigabyte of RAM. */ + total_system_memory_output = GIGABYTE(1); + + /* We have 0.75 * RAM available. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, + 3 * (GIGABYTE(1) / 4)); + + /* We are running on a tiny machine with 256 MB of RAM. */ + total_system_memory_output = MEGABYTE(256); + + /* We will now enforce a minimum of 256 MB of RAM available for the + * MaxMemInQueues here, even though we should only have had 0.75 * 256 = 192 + * MB available. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, MEGABYTE(256)); + + /* We are running on a machine with 8 GB of RAM. */ + total_system_memory_output = GIGABYTE(8); + + /* We will have 0.4 * RAM available. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, + 2 * (GIGABYTE(8) / 5)); + + /* We are running on a machine with 16 GB of RAM. */ + total_system_memory_output = GIGABYTE(16); + + /* We will have 0.4 * RAM available. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, + 2 * (GIGABYTE(16) / 5)); + + /* We are running on a machine with 32 GB of RAM. */ + total_system_memory_output = GIGABYTE(32); + + /* We will at maximum get MAX_DEFAULT_MEMORY_QUEUE_SIZE here. */ + tt_int_op(compute_real_max_mem_in_queues(0, 0), OP_EQ, + MAX_DEFAULT_MEMORY_QUEUE_SIZE); + + done: + UNMOCK(get_total_system_memory); + +#undef GIGABYTE +#undef MEGABYTE +} + #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@ -5644,6 +5726,7 @@ struct testcase_t config_tests[] = { CONFIG_TEST(check_bridge_distribution_setting_invalid, 0), CONFIG_TEST(check_bridge_distribution_setting_unrecognised, 0), CONFIG_TEST(include_opened_file_list, 0), + CONFIG_TEST(compute_max_mem_in_queues, 0), END_OF_TESTCASES };