On Fri, Nov 15, 2019 at 02:41:52PM +0000, Martin Wilck wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> checkint should never be larger than max_checkint. The WATCHDOG_USEC
> setting should be honored on "reconfigure", too, not only on startup.
> Pathological values for checkint and integer overflows should be avoided.
> The logic should work reasonably well if both polling_interval and
> max_polling_interval, just one of them, or neither is set.
>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> libmultipath/config.c | 40 +++++++++++++++++++++++++++++++++++++---
> libmultipath/config.h | 1 +
> libmultipath/defaults.h | 3 +--
> multipathd/main.c | 26 ++++++--------------------
> 4 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 49723add..0bf7bb40 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -683,6 +683,27 @@ process_config_dir(struct config *conf, char *dir)
> pthread_cleanup_pop(1);
> }
>
> +static void set_max_checkint_from_watchdog(struct config *conf)
> +{
> +#ifdef USE_SYSTEMD
> + char *envp = getenv("WATCHDOG_USEC");
> + unsigned long checkint;
> +
> + if (envp && sscanf(envp, "%lu", &checkint) == 1) {
> + /* Value is in microseconds */
> + checkint /= 1000000;
> + if (checkint < 1 || checkint > UINT_MAX) {
> + condlog(1, "invalid value for WatchdogSec: \"%s\"", envp);
> + return;
> + }
> + if (conf->max_checkint == 0 || conf->max_checkint > checkint)
> + conf->max_checkint = checkint;
> + condlog(3, "enabling watchdog, interval %ld", checkint);
> + conf->use_watchdog = true;
> + }
> +#endif
> +}
> +
> struct config *
> load_config (char * file)
> {
> @@ -703,7 +724,8 @@ load_config (char * file)
> conf->multipath_dir = set_default(DEFAULT_MULTIPATHDIR);
> conf->attribute_flags = 0;
> conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
> - conf->checkint = DEFAULT_CHECKINT;
> + conf->checkint = CHECKINT_UNDEF;
> + conf->use_watchdog = false;
> conf->max_checkint = 0;
> conf->force_sync = DEFAULT_FORCE_SYNC;
> conf->partition_delim = (default_partition_delim != NULL ?
> @@ -754,8 +776,20 @@ load_config (char * file)
> /*
> * fill the voids left in the config file
> */
> - if (conf->max_checkint == 0)
> - conf->max_checkint = MAX_CHECKINT(conf->checkint);
> + set_max_checkint_from_watchdog(conf);
> + if (conf->max_checkint == 0) {
> + if (conf->checkint == CHECKINT_UNDEF)
> + conf->checkint = DEFAULT_CHECKINT;
> + conf->max_checkint = (conf->checkint < UINT_MAX / 4 ?
> + conf->checkint * 4 : UINT_MAX);
> + } else if (conf->checkint == CHECKINT_UNDEF)
> + conf->checkint = (conf->max_checkint >= 4 ?
> + conf->max_checkint / 4 : 1);
> + else if (conf->checkint > conf->max_checkint)
> + conf->checkint = conf->max_checkint;
> + condlog(3, "polling interval: %d, max: %d",
> + conf->checkint, conf->max_checkint);
> +
> if (conf->blist_devnode == NULL) {
> conf->blist_devnode = vector_alloc();
>
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index 2ab7b42c..a078947c 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -139,6 +139,7 @@ struct config {
> int minio_rq;
> unsigned int checkint;
> unsigned int max_checkint;
> + bool use_watchdog;
> int pgfailback;
> int remove;
> int rr_weight;
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index d7034655..e5ee6afe 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -53,9 +53,8 @@
> /* Enable all foreign libraries by default */
> #define DEFAULT_ENABLE_FOREIGN ""
>
> -#define CHECKINT_UNDEF (~0U)
> +#define CHECKINT_UNDEF UINT_MAX
> #define DEFAULT_CHECKINT 5
> -#define MAX_CHECKINT(a) (a << 2)
>
> #define MAX_DEV_LOSS_TMO UINT_MAX
> #define DEFAULT_PIDFILE "/" RUN_DIR "/multipathd.pid"
> diff --git a/multipathd/main.c b/multipathd/main.c
> index c0423602..95426d3d 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -33,10 +33,6 @@
> */
> #include "checkers.h"
>
> -#ifdef USE_SYSTEMD
> -static int use_watchdog;
> -#endif
> -
> /*
> * libmultipath
> */
> @@ -2284,6 +2280,7 @@ checkerloop (void *ap)
> struct timespec last_time;
> struct config *conf;
> int foreign_tick = 0;
> + bool use_watchdog;
>
> pthread_cleanup_push(rcu_unregister, NULL);
> rcu_register_thread();
> @@ -2295,6 +2292,11 @@ checkerloop (void *ap)
> get_monotonic_time(&last_time);
> last_time.tv_sec -= 1;
>
> + /* use_watchdog is set from process environment and never changes */
> + conf = get_multipath_config();
> + use_watchdog = conf->use_watchdog;
> + put_multipath_config(conf);
> +
> while (1) {
> struct timespec diff_time, start_time, end_time;
> int num_paths = 0, strict_timing, rc = 0;
> @@ -2766,7 +2768,6 @@ child (__attribute__((unused)) void *param)
> struct multipath * mpp;
> int i;
> #ifdef USE_SYSTEMD
> - unsigned long checkint;
> int startup_done = 0;
> #endif
> int rc;
> @@ -2843,21 +2844,6 @@ child (__attribute__((unused)) void *param)
> setscheduler();
> set_oom_adj();
>
> -#ifdef USE_SYSTEMD
> - envp = getenv("WATCHDOG_USEC");
> - if (envp && sscanf(envp, "%lu", &checkint) == 1) {
> - /* Value is in microseconds */
> - conf->max_checkint = checkint / 1000000;
> - /* Rescale checkint */
> - if (conf->checkint > conf->max_checkint)
> - conf->checkint = conf->max_checkint;
> - else
> - conf->checkint = conf->max_checkint / 4;
> - condlog(3, "enabling watchdog, interval %d max %d",
> - conf->checkint, conf->max_checkint);
> - use_watchdog = conf->checkint;
> - }
> -#endif
> /*
> * Startup done, invalidate configuration
> */
> --
> 2.24.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
