These patches are identical to the ones I sent before. Please ignore the
resend.
My apologies
-Ben
On Mon, Jul 22, 2024 at 07:23:06PM -0400, Benjamin Marzinski wrote:
> Move the code to sync the mpp device state into a helper function and
> add a counter to to make sure that the device is synced at least once
> every max_checkint secs. This makes sure that multipath devices with no
> paths will still get synced with the kernel. Also, if multiple paths
> are checked in the same loop, the multipath device will only be synced
> with the kernel once, since every time the mpp is synced in any code
> path, mpp->sync_tick is reset.
>
> The code still syncs the mpp before updating the path state for two
> main reasons.
>
> 1. Sometimes multipathd leaves the mpp with a garbage state. Future
> patches will fix most of these cases, but the code intentially
> does not remove the mpp is resyncing fails while checking paths.
> But this does leave the mpp with a garbage state.
>
> 2. The kernel chages the multipath state independently of multipathd. If
> the kernel fails a path, a uevent will arrive shortly. But the kernel
> doesn't provide any notification when it switches the active
> path group or if it ends up picking a different one than multipathd
> selected. Multipathd needs to know the actual current pathgroup to
> know when it should be switching them.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
> libmultipath/configure.c | 1 +
> libmultipath/structs.h | 2 ++
> libmultipath/structs_vec.c | 5 +++
> multipathd/main.c | 64 +++++++++++++++++++++++++-------------
> 4 files changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index b4de863c..34158e31 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -358,6 +358,7 @@ int setup_map(struct multipath *mpp, char **params, struct vectors *vecs)
>
> sysfs_set_scsi_tmo(conf, mpp);
> marginal_pathgroups = conf->marginal_pathgroups;
> + mpp->sync_tick = conf->max_checkint;
> pthread_cleanup_pop(1);
>
> if (!mpp->features || !mpp->hwhandler || !mpp->selector) {
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 3b91e39c..002eeae1 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -453,6 +453,8 @@ struct multipath {
> int ghost_delay;
> int ghost_delay_tick;
> int queue_mode;
> + unsigned int sync_tick;
> + bool is_checked;
> uid_t uid;
> gid_t gid;
> mode_t mode;
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index d58ef5a7..7f267ba0 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -505,11 +505,16 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
> char __attribute__((cleanup(cleanup_charp))) *params = NULL;
> char __attribute__((cleanup(cleanup_charp))) *status = NULL;
> unsigned long long size;
> + struct config *conf;
>
> if (!mpp)
> return r;
>
> size = mpp->size;
> + conf = get_multipath_config();
> + mpp->sync_tick = conf->max_checkint;
> + put_multipath_config(conf);
> +
> r = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY,
> (mapid_t) { .str = mpp->alias },
> (mapinfo_t) {
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 86f7ab1f..ae40f599 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2342,6 +2342,37 @@ check_path_state(struct path *pp)
> return newstate;
> }
>
> +static void
> +do_sync_mpp(struct vectors * vecs, struct multipath *mpp)
> +{
> + int i, ret;
> + struct path *pp;
> +
> + mpp->is_checked = true;
> + ret = update_multipath_strings(mpp, vecs->pathvec);
> + if (ret != DMP_OK) {
> + condlog(1, "%s: %s", mpp->alias, ret == DMP_NOT_FOUND ?
> + "device not found" :
> + "couldn't synchronize with kernel state");
> + vector_foreach_slot (mpp->paths, pp, i)
> + pp->dmstate = PSTATE_UNDEF;
> + return;
> + }
> + set_no_path_retry(mpp);
> +}
> +
> +static void
> +sync_mpp(struct vectors * vecs, struct multipath *mpp, unsigned int ticks)
> +{
> + if (mpp->sync_tick)
> + mpp->sync_tick -= (mpp->sync_tick > ticks) ? ticks :
> + mpp->sync_tick;
> + if (mpp->sync_tick)
> + return;
> +
> + do_sync_mpp(vecs, mpp);
> +}
> +
> /*
> * Returns '1' if the path has been checked and '0' otherwise
> */
> @@ -2356,7 +2387,6 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
> unsigned int checkint, max_checkint;
> struct config *conf;
> int marginal_pathgroups, marginal_changed = 0;
> - int ret;
> bool need_reload;
>
> if (pp->initialized == INIT_REMOVED)
> @@ -2395,26 +2425,6 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
> pp->tick = 1;
> return 0;
> }
> - /*
> - * Synchronize with kernel state
> - */
> - ret = update_multipath_strings(pp->mpp, vecs->pathvec);
> - if (ret != DMP_OK) {
> - if (ret == DMP_NOT_FOUND) {
> - /* multipath device missing. Likely removed */
> - condlog(1, "%s: multipath device '%s' not found",
> - pp->dev, pp->mpp ? pp->mpp->alias : "");
> - return 0;
> - } else
> - condlog(1, "%s: Couldn't synchronize with kernel state",
> - pp->dev);
> - pp->dmstate = PSTATE_UNDEF;
> - }
> - /* if update_multipath_strings orphaned the path, quit early */
> - if (!pp->mpp)
> - return 0;
> - set_no_path_retry(pp->mpp);
> -
> if (pp->recheck_wwid == RECHECK_WWID_ON &&
> (newstate == PATH_UP || newstate == PATH_GHOST) &&
> ((pp->state != PATH_UP && pp->state != PATH_GHOST) ||
> @@ -2424,7 +2434,12 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
> handle_path_wwid_change(pp, vecs);
> return 0;
> }
> -
> + if (!pp->mpp->is_checked) {
> + do_sync_mpp(vecs, pp->mpp);
> + /* if update_multipath_strings orphaned the path, quit early */
> + if (!pp->mpp)
> + return 0;
> + }
> if ((newstate != PATH_UP && newstate != PATH_GHOST &&
> newstate != PATH_PENDING) && (pp->state == PATH_DELAYED)) {
> /* If path state become failed again cancel path delay state */
> @@ -2752,12 +2767,17 @@ checkerloop (void *ap)
> while (checker_state != CHECKER_FINISHED) {
> unsigned int paths_checked = 0, i;
> struct timespec chk_start_time;
> + struct multipath *mpp;
>
> pthread_cleanup_push(cleanup_lock, &vecs->lock);
> lock(&vecs->lock);
> pthread_testcancel();
> + vector_foreach_slot(vecs->mpvec, mpp, i)
> + mpp->is_checked = false;
> get_monotonic_time(&chk_start_time);
> if (checker_state == CHECKER_STARTING) {
> + vector_foreach_slot(vecs->mpvec, mpp, i)
> + sync_mpp(vecs, mpp, ticks);
> vector_foreach_slot(vecs->pathvec, pp, i)
> pp->is_checked = false;
> checker_state = CHECKER_RUNNING;
> --
> 2.45.0
>
