From 48e1c63c6617558cd06b344bf68b9a770d3de5c2 Mon Sep 17 00:00:00 2001 From: ylavic Date: Tue, 28 Nov 2023 21:42:25 +0100 Subject: [PATCH] Use new [ap_]atomics for proxy_worker_shared->busy/lbstatus. --- .../proxy/balancers/mod_lbmethod_bybusyness.c | 48 ++++--- .../proxy/balancers/mod_lbmethod_byrequests.c | 31 +++-- .../proxy/balancers/mod_lbmethod_bytraffic.c | 19 ++- modules/proxy/mod_proxy_balancer.c | 35 +++-- modules/proxy/proxy_util.c | 123 +----------------- modules/proxy/proxy_util.h | 35 ----- 6 files changed, 85 insertions(+), 206 deletions(-) diff --git a/modules/proxy/balancers/mod_lbmethod_bybusyness.c b/modules/proxy/balancers/mod_lbmethod_bybusyness.c index c2efa2f9a9d..edd6a8d27f0 100644 --- a/modules/proxy/balancers/mod_lbmethod_bybusyness.c +++ b/modules/proxy/balancers/mod_lbmethod_bybusyness.c @@ -19,6 +19,7 @@ #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" +#include "ap_atomics.h" #include "ap_hooks.h" module AP_MODULE_DECLARE_DATA lbmethod_bybusyness_module; @@ -30,23 +31,30 @@ static APR_OPTIONAL_FN_TYPE(proxy_balancer_get_best_worker) static int is_best_bybusyness(proxy_worker *current, proxy_worker *prev_best, void *baton) { int *total_factor = (int *)baton; - apr_size_t current_busy = ap_proxy_get_busy_count(current); - apr_size_t prev_best_busy = 0; - - current->s->lbstatus += current->s->lbfactor; - *total_factor += current->s->lbfactor; - if (prev_best) - prev_best_busy = ap_proxy_get_busy_count(prev_best); - - - return ( - !prev_best - || (current_busy < prev_best_busy) - || ( - (current_busy == prev_best_busy) - && (current->s->lbstatus > prev_best->s->lbstatus) - ) - ); + int lbfactor = current->s->lbfactor, lbstatus; + + *total_factor += lbfactor; + lbstatus = ap_atomic_int_add_sat(¤t->s->lbstatus, lbfactor); + if (prev_best) { + apr_size_t current_busy = ap_atomic_size_get(¤t->s->busy); + apr_size_t prev_busy = ap_atomic_size_get(&prev_best->s->busy); + if (current_busy > prev_busy) { + return 0; + } + if (current_busy == prev_busy) { + /* lbstatus is the value before atomic add */ + if (lbstatus < APR_INT32_MAX - lbfactor) { + lbstatus += lbfactor; + } + else { + lbstatus = APR_INT32_MAX; + } + if (lbstatus <= ap_atomic_int_get(&prev_best->s->lbstatus)) { + return 0; + } + } + } + return 1; } static proxy_worker *find_best_bybusyness(proxy_balancer *balancer, @@ -58,7 +66,7 @@ static proxy_worker *find_best_bybusyness(proxy_balancer *balancer, &total_factor); if (worker) { - worker->s->lbstatus -= total_factor; + ap_atomic_int_sub_sat(&worker->s->lbstatus, total_factor); } return worker; @@ -71,8 +79,8 @@ static apr_status_t reset(proxy_balancer *balancer, server_rec *s) proxy_worker **worker; worker = (proxy_worker **)balancer->workers->elts; for (i = 0; i < balancer->workers->nelts; i++, worker++) { - (*worker)->s->lbstatus = 0; - ap_proxy_set_busy_count(*worker, 0); + ap_atomic_int_set(&(*worker)->s->lbstatus, 0); + ap_atomic_size_set(&(*worker)->s->busy, 0); } return APR_SUCCESS; } diff --git a/modules/proxy/balancers/mod_lbmethod_byrequests.c b/modules/proxy/balancers/mod_lbmethod_byrequests.c index b6cd8d8b8bd..cd3bf1bc0ef 100644 --- a/modules/proxy/balancers/mod_lbmethod_byrequests.c +++ b/modules/proxy/balancers/mod_lbmethod_byrequests.c @@ -18,6 +18,7 @@ #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" +#include "ap_atomics.h" #include "ap_hooks.h" module AP_MODULE_DECLARE_DATA lbmethod_byrequests_module; @@ -28,11 +29,23 @@ static APR_OPTIONAL_FN_TYPE(proxy_balancer_get_best_worker) static int is_best_byrequests(proxy_worker *current, proxy_worker *prev_best, void *baton) { int *total_factor = (int *)baton; - - current->s->lbstatus += current->s->lbfactor; - *total_factor += current->s->lbfactor; - - return (!prev_best || (current->s->lbstatus > prev_best->s->lbstatus)); + int lbfactor = current->s->lbfactor, lbstatus; + + *total_factor += lbfactor; + lbstatus = ap_atomic_int_add_sat(¤t->s->lbstatus, lbfactor); + if (prev_best) { + /* lbstatus is the value before atomic add */ + if (lbstatus < APR_INT32_MAX - lbfactor) { + lbstatus += lbfactor; + } + else { + lbstatus = APR_INT32_MAX; + } + if (lbstatus <= ap_atomic_int_get(&prev_best->s->lbstatus)) { + return 0; + } + } + return 1; } /* @@ -84,10 +97,12 @@ static proxy_worker *find_best_byrequests(proxy_balancer *balancer, request_rec *r) { int total_factor = 0; - proxy_worker *worker = ap_proxy_balancer_get_best_worker_fn(balancer, r, is_best_byrequests, &total_factor); + proxy_worker *worker = ap_proxy_balancer_get_best_worker_fn(balancer, r, + is_best_byrequests, + &total_factor); if (worker) { - worker->s->lbstatus -= total_factor; + ap_atomic_int_sub_sat(&worker->s->lbstatus, total_factor); } return worker; @@ -100,7 +115,7 @@ static apr_status_t reset(proxy_balancer *balancer, server_rec *s) proxy_worker **worker; worker = (proxy_worker **)balancer->workers->elts; for (i = 0; i < balancer->workers->nelts; i++, worker++) { - (*worker)->s->lbstatus = 0; + ap_atomic_int_set(&(*worker)->s->lbstatus, 0); } return APR_SUCCESS; } diff --git a/modules/proxy/balancers/mod_lbmethod_bytraffic.c b/modules/proxy/balancers/mod_lbmethod_bytraffic.c index 6cfab94c057..a087fa49552 100644 --- a/modules/proxy/balancers/mod_lbmethod_bytraffic.c +++ b/modules/proxy/balancers/mod_lbmethod_bytraffic.c @@ -18,6 +18,7 @@ #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" +#include "ap_atomics.h" #include "ap_hooks.h" module AP_MODULE_DECLARE_DATA lbmethod_bytraffic_module; @@ -28,16 +29,14 @@ static APR_OPTIONAL_FN_TYPE(proxy_balancer_get_best_worker) static int is_best_bytraffic(proxy_worker *current, proxy_worker *prev_best, void *baton) { apr_off_t *min_traffic = (apr_off_t *)baton; - apr_off_t traffic = (current->s->transferred / current->s->lbfactor) - + (current->s->read / current->s->lbfactor); + apr_off_t traffic = (current->s->transferred / current->s->lbfactor + + current->s->read / current->s->lbfactor); - if (!prev_best || (traffic < *min_traffic)) { - *min_traffic = traffic; - - return TRUE; + if (prev_best && traffic >= *min_traffic) { + return 0; } - - return FALSE; + *min_traffic = traffic; + return 1; } /* @@ -73,8 +72,8 @@ static apr_status_t reset(proxy_balancer *balancer, server_rec *s) proxy_worker **worker; worker = (proxy_worker **)balancer->workers->elts; for (i = 0; i < balancer->workers->nelts; i++, worker++) { - (*worker)->s->lbstatus = 0; - (*worker)->s->busy = 0; + ap_atomic_int_set(&(*worker)->s->lbstatus, 0); + ap_atomic_size_set(&(*worker)->s->busy, 0); (*worker)->s->transferred = 0; (*worker)->s->read = 0; } diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index ea1b034d00e..e61fe1721ad 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -21,6 +21,7 @@ #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" +#include "ap_atomics.h" #include "ap_hooks.h" #include "apr_date.h" #include "apr_escape.h" @@ -485,6 +486,13 @@ static void force_recovery(proxy_balancer *balancer, server_rec *s) } } +static apr_status_t proxy_worker_decrement_busy(void *arg) +{ + proxy_worker *worker = arg; + ap_atomic_size_sub_sat(&worker->s->busy, 1); + return APR_SUCCESS; +} + static int proxy_balancer_pre_request(proxy_worker **worker, proxy_balancer **balancer, request_rec *r, @@ -545,12 +553,12 @@ static int proxy_balancer_pre_request(proxy_worker **worker, * not in error state or not disabled. */ if (PROXY_WORKER_IS_USABLE(*workers)) { - (*workers)->s->lbstatus += (*workers)->s->lbfactor; + ap_atomic_int_add_sat(&(*workers)->s->lbstatus, (*workers)->s->lbfactor); total_factor += (*workers)->s->lbfactor; } workers++; } - runtime->s->lbstatus -= total_factor; + ap_atomic_int_sub_sat(&runtime->s->lbstatus, total_factor); } runtime->s->elected++; @@ -623,8 +631,8 @@ static int proxy_balancer_pre_request(proxy_worker **worker, *worker = runtime; } - ap_proxy_increment_busy_count(*worker); - apr_pool_cleanup_register(r->pool, *worker, ap_proxy_decrement_busy_count, + ap_atomic_size_add_sat(&(*worker)->s->busy, 1); + apr_pool_cleanup_register(r->pool, *worker, proxy_worker_decrement_busy, apr_pool_cleanup_null); /* Add balancer/worker info to env. */ @@ -726,16 +734,15 @@ static void recalc_factors(proxy_balancer *balancer) /* Recalculate lbfactors */ workers = (proxy_worker **)balancer->workers->elts; - /* Special case if there is only one worker its - * load factor will always be 100 - */ if (balancer->workers->nelts == 1) { - (*workers)->s->lbstatus = (*workers)->s->lbfactor = 100; - return; + /* Special case if there is only one worker its + * load factor will always be 100 + */ + workers[0]->s->lbfactor = 100; } for (i = 0; i < balancer->workers->nelts; i++) { /* Update the status entries */ - workers[i]->s->lbstatus = workers[i]->s->lbfactor; + ap_atomic_int_set(&workers[i]->s->lbstatus, workers[i]->s->lbfactor); } } @@ -1542,7 +1549,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf, worker->s->retries); ap_rprintf(r, " %d\n", - worker->s->lbstatus); + ap_atomic_int_get(&worker->s->lbstatus)); ap_rprintf(r, " %.2f\n", (float)(worker->s->lbfactor)/100.0); @@ -1563,7 +1570,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf, "\n", NULL); ap_rprintf(r, " %" APR_SIZE_T_FMT "\n", - ap_proxy_get_busy_count(worker)); + ap_atomic_size_get(&worker->s->busy)); ap_rprintf(r, " %d\n", worker->s->lbset); /* End proxy_worker_stat */ @@ -1736,8 +1743,8 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf, ap_rvputs(r, ap_proxy_parse_wstatus(r->pool, worker), NULL); ap_rputs("", r); ap_rprintf(r, "%" APR_SIZE_T_FMT "", worker->s->elected); - ap_rprintf(r, "%" APR_SIZE_T_FMT "", ap_proxy_get_busy_count(worker)); - ap_rprintf(r, "%d", worker->s->lbstatus); + ap_rprintf(r, "%" APR_SIZE_T_FMT "", ap_atomic_size_get(&worker->s->busy)); + ap_rprintf(r, "%d", ap_atomic_int_get(&worker->s->lbstatus)); ap_rputs(apr_strfsize(worker->s->transferred, fbuf), r); ap_rputs("", r); ap_rputs(apr_strfsize(worker->s->read, fbuf), r); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 155d82be36f..748daf7adef 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -22,6 +22,7 @@ #include "apr_strings.h" #include "apr_hash.h" #include "apr_atomic.h" +#include "ap_atomics.h" #include "http_core.h" #include "proxy_util.h" #include "ajp.h" @@ -1488,7 +1489,9 @@ static proxy_worker *proxy_balancer_get_best_worker(proxy_balancer *balancer, if (best_worker) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, APLOGNO(10123) "proxy: %s selected worker \"%s\" : busy %" APR_SIZE_T_FMT " : lbstatus %d", - balancer->lbmethod->name, best_worker->s->name, best_worker->s->busy, best_worker->s->lbstatus); + balancer->lbmethod->name, best_worker->s->name, + ap_atomic_size_get(&best_worker->s->busy), + ap_atomic_int_get(&best_worker->s->lbstatus)); } return best_worker; @@ -5392,124 +5395,6 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tunnel_create(proxy_tunnel_rec **ptunnel, return APR_SUCCESS; } -PROXY_DECLARE(apr_status_t) ap_proxy_decrement_busy_count(void *worker_) -{ - apr_size_t val; - proxy_worker *worker = worker_; - -#if APR_SIZEOF_VOIDP == 4 - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); - val = apr_atomic_read32(&worker->s->busy); - while (val > 0) { - apr_size_t old = val; - val = apr_atomic_cas32(&worker->s->busy, val - 1, old); - if (val == old) { - break; - } - } -#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); - val = apr_atomic_read64(&worker->s->busy); - while (val > 0) { - apr_size_t old = val; - val = apr_atomic_cas64(&worker->s->busy, val - 1, old); - if (val == old) { - break; - } - } -#else /* Use atomics for (64bit) pointers */ - void *volatile *busy_p = (void *)&worker->s->busy; - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); - AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); - val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); - while (val > 0) { - apr_size_t old = val; - val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, - (void *)(apr_uintptr_t)(val - 1), - (void *)(apr_uintptr_t)old); - if (val == old) { - break; - } - } -#endif - return APR_SUCCESS; -} - -PROXY_DECLARE(void) ap_proxy_increment_busy_count(proxy_worker *worker) -{ - apr_size_t val; -#if APR_SIZEOF_VOIDP == 4 - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); - val = apr_atomic_read32(&worker->s->busy); - while (val < APR_INT32_MAX) { - apr_size_t old = val; - val = apr_atomic_cas32(&worker->s->busy, val + 1, old); - if (val == old) { - break; - } - } -#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); - val = apr_atomic_read64(&worker->s->busy); - while (val < APR_INT64_MAX) { - apr_size_t old = val; - val = apr_atomic_cas64(&worker->s->busy, val + 1, old); - if (val == old) { - break; - } - } -#else /* Use atomics for (64bit) pointers */ - void *volatile *busy_p = (void *)&worker->s->busy; - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); - AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); - val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); - while (val < APR_INT64_MAX) { - apr_size_t old = val; - val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, - (void *)(apr_uintptr_t)(val + 1), - (void *)(apr_uintptr_t)old); - if (val == old) { - break; - } - } -#endif -} - -PROXY_DECLARE(apr_size_t) ap_proxy_get_busy_count(proxy_worker *worker) -{ - apr_size_t val; -#if APR_SIZEOF_VOIDP == 4 - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); - val = apr_atomic_read32(&worker->s->busy); -#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); - val = apr_atomic_read64(&worker->s->busy); -#else /* Use atomics for (64bit) pointers */ - void *volatile *busy_p = (void *)&worker->s->busy; - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); - AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); - val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); -#endif - - return val; -} - -PROXY_DECLARE(void) ap_proxy_set_busy_count(proxy_worker *worker, apr_size_t to) -{ -#if APR_SIZEOF_VOIDP == 4 - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); - apr_atomic_set32(&worker->s->busy, to); -#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); - apr_atomic_set64(&worker->s->busy, to); -#else /* Use atomics for (64bit) pointers */ - void *volatile *busy_p = (void *)&worker->s->busy; - AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); - AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); - apr_atomic_xchgptr((void *)busy_p, (void *)(apr_uintptr_t)to); -#endif -} - static void add_pollset(apr_pollset_t *pollset, apr_pollfd_t *pfd, apr_int16_t events) { diff --git a/modules/proxy/proxy_util.h b/modules/proxy/proxy_util.h index 42d0f89811a..bc131da0f0f 100644 --- a/modules/proxy/proxy_util.h +++ b/modules/proxy/proxy_util.h @@ -40,41 +40,6 @@ extern PROXY_DECLARE_DATA const apr_strmatch_pattern *ap_proxy_strmatch_domain; */ void proxy_util_register_hooks(apr_pool_t *p); -/* - * Get the busy counter from the shared worker memory - * - * @param worker Pointer to the worker structure. - * @return apr_size_t value atomically read for the worker. - */ -PROXY_DECLARE(apr_size_t) ap_proxy_get_busy_count(proxy_worker *worker); - -/* - * Set the busy counter from the shared worker memory - * - * @param worker Pointer to the worker structure. - * @param to value to set the busy counter. - * @return void - */ -PROXY_DECLARE(void) ap_proxy_set_busy_count(proxy_worker *worker, apr_size_t to); - -/* - * decrement the busy counter from the shared worker memory - * note it is called by apr_pool_cleanup_register() - * therfore the void * and apr_status_t. - * - * @param worker_ Pointer to the worker structure. - * @return apr_status_t returns APR_SUCCESS. - */ -PROXY_DECLARE(apr_status_t) ap_proxy_decrement_busy_count(void *worker_); - -/* - * increment the busy counter from the shared worker memory - * - * @param worker Pointer to the worker structure. - * @return void - */ -PROXY_DECLARE(void) ap_proxy_increment_busy_count(proxy_worker *worker); - /** @} */ #endif /* PROXY_UTIL_H_ */