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?
Martin
