From: Benjamin Marzinski <bmarzins@xxxxxxxxxx> delayed_reconfig was inside the config struct, but it wasn't updated during an RCU write section, so there's no synchronization on it. Instead, pull it out of the config structure, and use the config_lock to synchronize it. Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> Reviewed-by: Martin Wilck <mwilck@xxxxxxxx> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> --- libmpathpersist/libmpathpersist.version | 12 +++---- libmultipath/config.h | 1 - libmultipath/libmultipath.version | 9 ++--- multipathd/main.c | 45 ++++++++++++++++--------- 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/libmpathpersist/libmpathpersist.version b/libmpathpersist/libmpathpersist.version index e074813..b52b750 100644 --- a/libmpathpersist/libmpathpersist.version +++ b/libmpathpersist/libmpathpersist.version @@ -10,7 +10,7 @@ * * See libmultipath.version for general policy about version numbers. */ -LIBMPATHPERSIST_1.0.0 { +LIBMPATHPERSIST_2.0.0 { global: __mpath_persistent_reserve_in; @@ -28,11 +28,9 @@ global: prout_do_scsi_ioctl; update_map_pr; -local: *; -}; - -LIBMPATHPERSIST_1.1.0 { -global: + /* added int 1.1.0 */ libmpathpersist_init; libmpathpersist_exit; -} LIBMPATHPERSIST_1.0.0; + +local: *; +}; diff --git a/libmultipath/config.h b/libmultipath/config.h index 933fe0d..c73389b 100644 --- a/libmultipath/config.h +++ b/libmultipath/config.h @@ -178,7 +178,6 @@ struct config { int strict_timing; int retrigger_tries; int retrigger_delay; - int delayed_reconfig; int uev_wait_timeout; int skip_kpartx; int remove_retries; diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index 2fb2547..4f6fa1d 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -31,7 +31,7 @@ * The new version inherits the previous ones. */ -LIBMULTIPATH_10.0.0 { +LIBMULTIPATH_11.0.0 { global: /* symbols referenced by multipath and multipathd */ add_foreign; @@ -290,11 +290,8 @@ global: /* added in 9.2.0 */ set_wakeup_fn; + /* added in 10.1.0 */ + __snprint_config; local: *; }; - -LIBMULTIPATH_10.1.0 { -global: - __snprint_config; -} LIBMULTIPATH_10.0.0; diff --git a/multipathd/main.c b/multipathd/main.c index 7c3e9b2..57c572d 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -127,6 +127,8 @@ static int poll_dmevents = 1; #endif /* Don't access this variable without holding config_lock */ static volatile enum daemon_status running_state = DAEMON_INIT; +/* Don't access this variable without holding config_lock */ +static bool __delayed_reconfig; pid_t daemon_pid; static pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_cond_t config_cond; @@ -150,6 +152,23 @@ int should_exit(void) return get_running_state() == DAEMON_SHUTDOWN; } +static bool get_delayed_reconfig(void) +{ + bool val; + + pthread_mutex_lock(&config_lock); + val = __delayed_reconfig; + pthread_mutex_unlock(&config_lock); + return val; +} + +static void set_delayed_reconfig(bool val) +{ + pthread_mutex_lock(&config_lock); + __delayed_reconfig = val; + pthread_mutex_unlock(&config_lock); +} + /* * global copy of vecs for use in sig handlers */ @@ -297,8 +316,10 @@ static void __post_config_state(enum daemon_status state) old_state = DAEMON_IDLE; state = DAEMON_CONFIGURE; } - if (reconfigure_pending && state == DAEMON_CONFIGURE) + if (state == DAEMON_CONFIGURE) { reconfigure_pending = false; + __delayed_reconfig = false; + } running_state = state; pthread_cond_broadcast(&config_cond); do_sd_notify(old_state, state); @@ -765,7 +786,7 @@ int ev_add_map (char * dev, const char * alias, struct vectors * vecs) { struct multipath * mpp; - int delayed_reconfig, reassign_maps; + int reassign_maps; struct config *conf; if (dm_is_mpath(alias) != 1) { @@ -784,12 +805,11 @@ ev_add_map (char * dev, const char * alias, struct vectors * vecs) return 1; } conf = get_multipath_config(); - delayed_reconfig = conf->delayed_reconfig; reassign_maps = conf->reassign_maps; put_multipath_config(conf); if (mpp->wait_for_udev) { mpp->wait_for_udev = 0; - if (delayed_reconfig && + if (get_delayed_reconfig() && !need_to_delay_reconfig(vecs)) { condlog(2, "reconfigure (delayed)"); schedule_reconfigure(); @@ -1791,8 +1811,7 @@ missing_uev_wait_tick(struct vectors *vecs) { struct multipath * mpp; unsigned int i; - int timed_out = 0, delayed_reconfig; - struct config *conf; + int timed_out = 0; vector_foreach_slot (vecs->mpvec, mpp, i) { if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) { @@ -1808,10 +1827,7 @@ missing_uev_wait_tick(struct vectors *vecs) } } - conf = get_multipath_config(); - delayed_reconfig = conf->delayed_reconfig; - put_multipath_config(conf); - if (timed_out && delayed_reconfig && + if (timed_out && get_delayed_reconfig() && !need_to_delay_reconfig(vecs)) { condlog(2, "reconfigure (delayed)"); schedule_reconfigure(); @@ -3257,13 +3273,10 @@ child (__attribute__((unused)) void *param) pthread_cleanup_push(cleanup_lock, &vecs->lock); lock(&vecs->lock); pthread_testcancel(); - if (!need_to_delay_reconfig(vecs)) { + if (!need_to_delay_reconfig(vecs)) reconfigure(vecs); - } else { - conf = get_multipath_config(); - conf->delayed_reconfig = 1; - put_multipath_config(conf); - } + else + set_delayed_reconfig(true); lock_cleanup_pop(vecs->lock); post_config_state(DAEMON_IDLE); } -- 2.33.1 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel