On Thu, Oct 03, 2024 at 11:00:20PM +0200, Martin Wilck wrote: > On Thu, 2024-09-12 at 17:49 -0400, Benjamin Marzinski wrote: > > Instead of updating the path priorities and possibly reloading the > > multipath device when each path is updated, wait till all paths > > have been updated, and then go through the multipath devices updating > > the priorities once, reloading if necessary. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > --- > > libmultipath/structs.h | 9 +++ > > multipathd/main.c | 169 ++++++++++++++++++++++++++------------- > > -- > > 2 files changed, 118 insertions(+), 60 deletions(-) > > > > diff --git a/libmultipath/structs.h b/libmultipath/structs.h > > index af8e31e9..1f531d30 100644 > > --- a/libmultipath/structs.h > > +++ b/libmultipath/structs.h > > @@ -318,6 +318,7 @@ enum check_path_states { > > CHECK_PATH_UNCHECKED, > > CHECK_PATH_STARTED, > > CHECK_PATH_CHECKED, > > + CHECK_PATH_NEW_UP, > > CHECK_PATH_SKIPPED, > > CHECK_PATH_REMOVED, > > }; > > @@ -421,6 +422,13 @@ enum prflag_value { > > PRFLAG_SET, > > }; > > > > +enum prio_update_type { > > + PRIO_UPDATE_NONE, > > + PRIO_UPDATE_NORMAL, > > + PRIO_UPDATE_NEW_PATH, > > + PRIO_UPDATE_MARGINAL, > > +}; > > + > > struct multipath { > > char wwid[WWID_SIZE]; > > char alias_old[WWID_SIZE]; > > @@ -464,6 +472,7 @@ struct multipath { > > int queue_mode; > > unsigned int sync_tick; > > int synced_count; > > + enum prio_update_type prio_update; > > uid_t uid; > > gid_t gid; > > mode_t mode; > > diff --git a/multipathd/main.c b/multipathd/main.c > > index 9519b6c5..3cda3c18 100644 > > --- a/multipathd/main.c > > +++ b/multipathd/main.c > > @@ -1996,15 +1996,13 @@ mpvec_garbage_collector (struct vectors * > > vecs) > > * best pathgroup, and this is the first path in the pathgroup to > > come back > > * up, then switch to this pathgroup */ > > static int > > -followover_should_failback(struct path * pp) > > +do_followover_should_failback(struct path * pp) > > { > > struct pathgroup * pgp; > > struct path *pp1; > > int i; > > > > - if (pp->mpp->pgfailback != -FAILBACK_FOLLOWOVER || > > - !pp->mpp->pg || !pp->pgindex || > > - pp->pgindex != pp->mpp->bestpg) > > + if (!pp->pgindex || pp->pgindex != pp->mpp->bestpg) > > return 0; > > > > pgp = VECTOR_SLOT(pp->mpp->pg, pp->pgindex - 1); > > @@ -2017,6 +2015,26 @@ followover_should_failback(struct path * pp) > > return 1; > > } > > > > +static int > > +followover_should_failback(struct multipath *mpp) > > +{ > > + struct path *pp; > > + struct pathgroup * pgp; > > + int i, j; > > + > > + if (mpp->pgfailback != -FAILBACK_FOLLOWOVER || !mpp->pg) > > + return 0; > > + > > + vector_foreach_slot (mpp->pg, pgp, i) { > > + vector_foreach_slot (pgp->paths, pp, j) { > > + if (pp->is_checked == CHECK_PATH_NEW_UP && > > + do_followover_should_failback(pp)) > > + return 1; > > + } > > + } > > + return 0; > > +} > > + > > static void > > missing_uev_wait_tick(struct vectors *vecs) > > { > > @@ -2132,41 +2150,53 @@ partial_retrigger_tick(vector pathvec) > > } > > } > > > > -static int update_prio(struct path *pp, int force_refresh_all) > > +static bool update_prio(struct multipath *mpp, bool refresh_all) > > { > > int oldpriority; > > - struct path *pp1; > > + struct path *pp; > > struct pathgroup * pgp; > > - int i, j, changed = 0; > > + int i, j; > > + bool changed = false; > > + bool skipped_path = false; > > struct config *conf; > > > > - oldpriority = pp->priority; > > - 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); > > + vector_foreach_slot (mpp->pg, pgp, i) { > > + vector_foreach_slot (pgp->paths, pp, j) { > > + if (pp->state == PATH_DOWN) > > + continue; > > + if (!refresh_all && > > + pp->is_checked != CHECK_PATH_CHECKED) { > > + skipped_path = true; > > + continue; > > + } > > Nit: My first thought here was that this would skip paths for which > pp->is_checked == CHECK_PATH_NEW_UP. Then I realized that if there was > a new path up, refresh_all would be set, which is not immediately > obvious. Can you add a comment? Sure. > > Martin