Message ID | 20220318223339.4226-5-mwilck@suse.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Delegated to: | christophe varoqui |
Headers | show |
Series | multipathd: fix __delayed_reconfig logic | expand |
On Fri, Mar 18, 2022 at 5:33 PM <mwilck@suse.com> wrote: > > From: Martin Wilck <mwilck@suse.com> > > 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@redhat.com> > Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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