[dm-devel] [PATCH V2 09/11] multipathd: only refresh priorities in update_prio()
Benjamin Marzinski
bmarzins at redhat.com
Tue Jun 6 20:13:09 UTC 2023
Multipathd previously had various rules to update priorities in
update_prio(), need_switch_pathgroup(), and reload_map(). Instead, only
update the priority in update_prio(). To cover the cases where the
priorities were getting updated in other functions, update_prio() now
always updates all paths' priorities, if the priority on the path that
it was called with changes.
Also, do not try to update a path's priority if it is down. Previously,
when refreshing all the paths' priorities, a path could have its
priority updated when it was in the PATH_DOWN state, as long as its
priority was PRIO_UNDEF. But if a path is down, and the last time
multipath tried to get its priority, it failed, it seems likely that the
prioritizer will just fail again.
Finally, skip updating all paths' priorities in
deferred_failback_tick(). Now that all the paths' priorities will get
updated when one changes before starting the deferred failback count,
it's no longer necessary to update them all again when the failback
timeout expires. The checker loop will continue to update them
correctly while the timeout is going on.
Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
---
multipathd/cli_handlers.c | 8 +--
multipathd/fpin_handlers.c | 4 +-
multipathd/main.c | 109 +++++++++++++------------------------
multipathd/main.h | 3 +-
4 files changed, 45 insertions(+), 79 deletions(-)
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 44bf43df..c9addfbb 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -810,7 +810,7 @@ cli_reload(void *v, struct strbuf *reply, void *data)
return 1;
}
- return reload_and_sync_map(mpp, vecs, 0);
+ return reload_and_sync_map(mpp, vecs);
}
static int resize_map(struct multipath *mpp, unsigned long long size,
@@ -1449,7 +1449,7 @@ static int cli_set_marginal(void * v, struct strbuf *reply, void * data)
}
pp->marginal = 1;
- return reload_and_sync_map(pp->mpp, vecs, 0);
+ return reload_and_sync_map(pp->mpp, vecs);
}
static int cli_unset_marginal(void * v, struct strbuf *reply, void * data)
@@ -1476,7 +1476,7 @@ static int cli_unset_marginal(void * v, struct strbuf *reply, void * data)
}
pp->marginal = 0;
- return reload_and_sync_map(pp->mpp, vecs, 0);
+ return reload_and_sync_map(pp->mpp, vecs);
}
static int cli_unset_all_marginal(void * v, struct strbuf *reply, void * data)
@@ -1513,7 +1513,7 @@ static int cli_unset_all_marginal(void * v, struct strbuf *reply, void * data)
vector_foreach_slot (pgp->paths, pp, j)
pp->marginal = 0;
- return reload_and_sync_map(mpp, vecs, 0);
+ return reload_and_sync_map(mpp, vecs);
}
#define HANDLER(x) x
diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c
index 8f464f04..aa0f63c9 100644
--- a/multipathd/fpin_handlers.c
+++ b/multipathd/fpin_handlers.c
@@ -127,7 +127,7 @@ empty:
/* walk backwards because reload_and_sync_map() can remove mpp */
vector_foreach_slot_backwards(vecs->mpvec, mpp, i) {
if (mpp->fpin_must_reload) {
- ret = reload_and_sync_map(mpp, vecs, 0);
+ ret = reload_and_sync_map(mpp, vecs);
if (ret == 2)
condlog(2, "map removed during reload");
else
@@ -262,7 +262,7 @@ unref:
/* walk backwards because reload_and_sync_map() can remove mpp */
vector_foreach_slot_backwards(vecs->mpvec, mpp, i) {
if (mpp->fpin_must_reload) {
- ret = reload_and_sync_map(mpp, vecs, 0);
+ ret = reload_and_sync_map(mpp, vecs);
if (ret == 2)
condlog(2, "map removed during reload");
else
diff --git a/multipathd/main.c b/multipathd/main.c
index bdeffe76..f603d143 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -396,32 +396,13 @@ void put_multipath_config(__attribute__((unused)) void *arg)
}
static int
-need_switch_pathgroup (struct multipath * mpp, int refresh)
+need_switch_pathgroup (struct multipath * mpp)
{
- struct pathgroup * pgp;
- struct path * pp;
- unsigned int i, j;
- struct config *conf;
int bestpg;
if (!mpp)
return 0;
- /*
- * Refresh path priority values
- */
- if (refresh) {
- vector_foreach_slot (mpp->pg, pgp, i) {
- vector_foreach_slot (pgp->paths, pp, j) {
- conf = get_multipath_config();
- pthread_cleanup_push(put_multipath_config,
- conf);
- pathinfo(pp, conf, DI_PRIO);
- pthread_cleanup_pop(1);
- }
- }
- }
-
if (!mpp->pg || VECTOR_SIZE(mpp->paths) == 0)
return 0;
@@ -1594,7 +1575,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
else {
if (ro == 1)
pp->mpp->force_readonly = 1;
- retval = reload_and_sync_map(mpp, vecs, 0);
+ retval = reload_and_sync_map(mpp, vecs);
if (retval == 2)
condlog(2, "%s: map removed during reload", pp->dev);
else {
@@ -1994,7 +1975,7 @@ deferred_failback_tick (vector mpvec)
if (mpp->pgfailback > 0 && mpp->failback_tick > 0) {
mpp->failback_tick--;
- if (!mpp->failback_tick && need_switch_pathgroup(mpp, 1))
+ if (!mpp->failback_tick && need_switch_pathgroup(mpp))
switch_pathgroup(mpp);
}
}
@@ -2051,54 +2032,40 @@ int update_prio(struct path *pp, int refresh_all)
int i, j, changed = 0;
struct config *conf;
- if (refresh_all) {
- vector_foreach_slot (pp->mpp->pg, pgp, i) {
- vector_foreach_slot (pgp->paths, pp1, j) {
- oldpriority = pp1->priority;
- conf = get_multipath_config();
- pthread_cleanup_push(put_multipath_config,
- conf);
- pathinfo(pp1, conf, DI_PRIO);
- pthread_cleanup_pop(1);
- if (pp1->priority != oldpriority)
- changed = 1;
- }
- }
- return changed;
- }
oldpriority = pp->priority;
- conf = get_multipath_config();
- pthread_cleanup_push(put_multipath_config, conf);
- if (pp->state != PATH_DOWN)
+ if (pp->state != PATH_DOWN) {
+ conf = get_multipath_config();
+ pthread_cleanup_push(put_multipath_config, conf);
pathinfo(pp, conf, DI_PRIO);
- pthread_cleanup_pop(1);
+ pthread_cleanup_pop(1);
+ }
- if (pp->priority == oldpriority)
+ if (pp->priority == oldpriority && !refresh_all)
return 0;
- return 1;
+
+ vector_foreach_slot (pp->mpp->pg, pgp, i) {
+ vector_foreach_slot (pgp->paths, pp1, j) {
+ if (pp1 == pp || pp1->state == PATH_DOWN)
+ continue;
+ oldpriority = pp1->priority;
+ conf = get_multipath_config();
+ pthread_cleanup_push(put_multipath_config, conf);
+ pathinfo(pp1, conf, DI_PRIO);
+ pthread_cleanup_pop(1);
+ if (pp1->priority != oldpriority)
+ changed = 1;
+ }
+ }
+ return changed;
}
-static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh,
+static int reload_map(struct vectors *vecs, struct multipath *mpp,
int is_daemon)
{
char *params __attribute__((cleanup(cleanup_charp))) = NULL;
- struct path *pp;
- int i, r;
+ int r;
update_mpp_paths(mpp, vecs->pathvec);
- if (refresh) {
- vector_foreach_slot (mpp->paths, pp, i) {
- struct config *conf = get_multipath_config();
- pthread_cleanup_push(put_multipath_config, conf);
- r = pathinfo(pp, conf, DI_PRIO);
- pthread_cleanup_pop(1);
- if (r) {
- condlog(2, "%s: failed to refresh pathinfo",
- mpp->alias);
- return 1;
- }
- }
- }
if (setup_map(mpp, ¶ms, vecs)) {
condlog(0, "%s: failed to setup map", mpp->alias);
return 1;
@@ -2115,10 +2082,9 @@ static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh,
return 0;
}
-int reload_and_sync_map(struct multipath *mpp,
- struct vectors *vecs, int refresh)
+int reload_and_sync_map(struct multipath *mpp, struct vectors *vecs)
{
- if (reload_map(vecs, mpp, refresh, 1))
+ if (reload_map(vecs, mpp, 1))
return 1;
if (setup_multipath(vecs, mpp) != 0)
return 2;
@@ -2573,25 +2539,26 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
*/
condlog(4, "path prio refresh");
- if (marginal_changed)
- reload_and_sync_map(pp->mpp, vecs, 1);
- else if (update_prio(pp, new_path_up) &&
- (pp->mpp->pgpolicyfn == (pgpolicyfn *)group_by_prio) &&
- pp->mpp->pgfailback == -FAILBACK_IMMEDIATE) {
+ if (marginal_changed) {
+ update_prio(pp, 1);
+ reload_and_sync_map(pp->mpp, vecs);
+ } else if (update_prio(pp, new_path_up) &&
+ pp->mpp->pgpolicyfn == (pgpolicyfn *)group_by_prio &&
+ pp->mpp->pgfailback == -FAILBACK_IMMEDIATE) {
condlog(2, "%s: path priorities changed. reloading",
pp->mpp->alias);
- reload_and_sync_map(pp->mpp, vecs, !new_path_up);
- } else if (need_switch_pathgroup(pp->mpp, 0)) {
+ reload_and_sync_map(pp->mpp, vecs);
+ } else if (need_switch_pathgroup(pp->mpp)) {
if (pp->mpp->pgfailback > 0 &&
(new_path_up || pp->mpp->failback_tick <= 0))
- pp->mpp->failback_tick =
- pp->mpp->pgfailback + 1;
+ pp->mpp->failback_tick = pp->mpp->pgfailback + 1;
else if (pp->mpp->pgfailback == -FAILBACK_IMMEDIATE ||
(chkr_new_path_up && followover_should_failback(pp)))
switch_pathgroup(pp->mpp);
}
return 1;
}
+
enum checker_state {
CHECKER_STARTING,
CHECKER_RUNNING,
diff --git a/multipathd/main.h b/multipathd/main.h
index e8bee8e6..a253d186 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -47,8 +47,7 @@ int __setup_multipath (struct vectors * vecs, struct multipath * mpp,
int reset);
#define setup_multipath(vecs, mpp) __setup_multipath(vecs, mpp, 1)
int update_multipath (struct vectors *vecs, char *mapname, int reset);
-int reload_and_sync_map(struct multipath *mpp, struct vectors *vecs,
- int refresh);
+int reload_and_sync_map(struct multipath *mpp, struct vectors *vecs);
void handle_path_wwid_change(struct path *pp, struct vectors *vecs);
bool check_path_wwid_change(struct path *pp);
--
2.17.2
More information about the dm-devel
mailing list