On Thu, Oct 03, 2024 at 10:41:20PM +0200, Martin Wilck wrote:
> On Thu, 2024-09-12 at 17:49 -0400, Benjamin Marzinski wrote:
> > Instead of checking and updating each path, the checkerloop now
> > starts
> > the checkers on all the paths in check_paths(), and then goes back
> > and
> > updates all the paths in update_paths(). Since the async checkers use
> > an
> > absolute time to wait for before returning PATH_PENDING, only one
> > checker actually needs to be waited for in update_paths(). The rest
> > will
> > already have reached their timeout when update_path() is called for
> > them.
> >
> > The check_paths() and update_paths() loop over the pathvec instead of
> > looping through the multipath device paths to avoid having to restart
> > checking of the device paths when the multipath device needs to be
> > resynced while updating the paths.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> > ---
> > libmultipath/structs.h | 10 +++-
> > multipathd/main.c | 105 +++++++++++++++++++--------------------
> > --
> > 2 files changed, 57 insertions(+), 58 deletions(-)
> >
> > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> > index d8231e95..af8e31e9 100644
> > --- a/libmultipath/structs.h
> > +++ b/libmultipath/structs.h
> > @@ -314,6 +314,14 @@ enum recheck_wwid_states {
> > RECHECK_WWID_ON = YNU_YES,
> > };
> >
> > +enum check_path_states {
> > + CHECK_PATH_UNCHECKED,
> > + CHECK_PATH_STARTED,
> > + CHECK_PATH_CHECKED,
> > + CHECK_PATH_SKIPPED,
> > + CHECK_PATH_REMOVED,
> > +};
> > +
> > struct vpd_vendor_page {
> > int pg;
> > const char *name;
> > @@ -395,7 +403,7 @@ struct path {
> > int fast_io_fail;
> > unsigned int dev_loss;
> > int eh_deadline;
> > - bool is_checked;
> > + enum check_path_states is_checked;
> > bool can_use_env_uid;
> > unsigned int checker_timeout;
> > /* configlet pointers */
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index 45d40559..9519b6c5 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -2389,13 +2389,6 @@ sync_mpp(struct vectors * vecs, struct
> > multipath *mpp, unsigned int ticks)
> > do_sync_mpp(vecs, mpp);
> > }
> >
> > -enum check_path_return {
> > - CHECK_PATH_STARTED,
> > - CHECK_PATH_CHECKED,
> > - CHECK_PATH_SKIPPED,
> > - CHECK_PATH_REMOVED,
> > -};
> > -
> > static int
> > update_path_state (struct vectors * vecs, struct path * pp)
> > {
> > @@ -2735,6 +2728,7 @@ check_uninitialized_path(struct path * pp,
> > unsigned int ticks)
> > conf = get_multipath_config();
> > retrigger_tries = conf->retrigger_tries;
> > pp->tick = conf->max_checkint;
> > + pp->checkint = conf->checkint;
> > put_multipath_config(conf);
> >
> > if (pp->initialized == INIT_MISSING_UDEV) {
> > @@ -2778,6 +2772,10 @@ update_uninitialized_path(struct vectors *
> > vecs, struct path * pp)
> > int newstate, ret;
> > struct config *conf;
> >
> > + if (pp->initialized != INIT_NEW && pp->initialized !=
> > INIT_FAILED &&
> > + pp->initialized != INIT_MISSING_UDEV)
> > + return CHECK_PATH_SKIPPED;
> > +
> > newstate = get_new_state(pp);
> >
>
> It seems to me that this hunk and the previous one belong into 11/22.
Setting pp->checkint could certainly go in the earlier patch. It's just
there to match check_path(), although to be honest, I'm still not sure
that there is any way for either check_path() or
check_uninitialized_path() to get called with pp->checkint unset.
The checking the path->initialized state in update_uninitialized_path()
makes less sense. That code is already in check_unintialized_path(). In
11/22, update_uninitialized_path() is always called immediately after
check_unintialized_path() if it's called at all, and pp->initialized
never has a chance to change.
In this patch, check_unintialized_path() is called in check_paths(), and
it's possible that path checking takes too long, and we need take a
break and yield to other vecs lock waiters before we call
update_paths(), where update_uninitialized_path() is called. In this
case, it's possible that a uevent got processed and changed
pp->initialized, so we need to recheck it in
update_uninitialized_path().
-Ben
> Martin
