On Fri, Mar 18, 2022 at 5:33 PM <mwilck@xxxxxxxx> wrote:
>
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> With the previous patch, one race condition between child() and
> the uevent handler (ev_add_map()) remains: 1. child() sets
> __delayed_reconfig, 2. ev_add_map() calls schedule_reconfigure() and
> thus DAEMON_CONFIGURE, 3. child() sets DAEMON_IDLE. This would cause
> the pending reconfigure request to be missed.
>
> To fix this, reset __delayed_reconfig immediately from ev_add_map()
> (and likewise, missing_uev_wait_tick()). This way the wait loop in
> child() terminates on the reconfigure_pending condition.
>
> Also, these schedule_reconfigure() callers don't really request a
> reconfigure() call; they just unblock processing previously requested
> reconfigure() calls. Add a new helper, unblock_reconfigure(), that
> does just that.
>
> Suggested-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
> multipathd/main.c | 45 +++++++++++++++++++++++++--------------------
> 1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index d033a9a..1454a18 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -155,16 +155,6 @@ 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;
> -}
> -
> /*
> * global copy of vecs for use in sig handlers
> */
> @@ -308,6 +298,27 @@ void post_config_state(enum daemon_status state)
> pthread_cleanup_pop(1);
> }
>
> +static bool unblock_reconfigure(void)
> +{
> + bool was_delayed;
> +
> + pthread_mutex_lock(&config_lock);
> + was_delayed = __delayed_reconfig;
> + if (was_delayed) {
> + __delayed_reconfig = false;
> + /*
> + * In IDLE state, make sure child() is woken up
> + * Otherwise it will wake up when state switches to IDLE
> + */
> + if (running_state == DAEMON_IDLE)
> + __post_config_state(DAEMON_CONFIGURE);
> + }
> + pthread_mutex_unlock(&config_lock);
> + if (was_delayed)
> + condlog(3, "unblocked delayed reconfigure");
> + return was_delayed;
> +}
> +
> void schedule_reconfigure(enum force_reload_types requested_type)
> {
> pthread_mutex_lock(&config_lock);
> @@ -790,12 +801,9 @@ ev_add_map (char * dev, const char * alias, struct vectors * vecs)
> dm_get_info(mpp->alias, &mpp->dmi);
> if (mpp->wait_for_udev) {
> mpp->wait_for_udev = 0;
> - if (get_delayed_reconfig() &&
> - !need_to_delay_reconfig(vecs)) {
> - condlog(2, "reconfigure (delayed)");
> - schedule_reconfigure(FORCE_RELOAD_WEAK);
> + if (!need_to_delay_reconfig(vecs) &&
> + unblock_reconfigure())
> return 0;
> - }
> }
> /*
> * Not really an error -- we generate our own uevent
> @@ -1899,11 +1907,8 @@ missing_uev_wait_tick(struct vectors *vecs)
> }
> }
>
> - if (timed_out && get_delayed_reconfig() &&
> - !need_to_delay_reconfig(vecs)) {
> - condlog(2, "reconfigure (delayed)");
> - schedule_reconfigure(FORCE_RELOAD_WEAK);
> - }
> + if (timed_out && !need_to_delay_reconfig(vecs))
> + unblock_reconfigure();
> }
>
> static void
> --
> 2.35.1
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
