On Thu, Oct 03, 2024 at 11:15:21PM +0200, Martin Wilck wrote:
> On Thu, 2024-09-12 at 17:49 -0400, Benjamin Marzinski wrote:
> > Add a new optional checker class function, libcheck_need_wait() and a
> > new function to call it, checker_need_wait(). This can be used to see
> > if
> > a path_checker is currently running. This will be used to determine
> > if
> > there are pending checkers that need to be waited for.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> > ---
> > libmultipath/checkers.c | 12 +++++++++++-
> > libmultipath/checkers.h | 4 +++-
> > libmultipath/checkers/directio.c | 10 ++++++++++
> > libmultipath/checkers/tur.c | 10 ++++++++++
> > 4 files changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> > index f3e98352..e2eda58d 100644
> > --- a/libmultipath/checkers.c
> > +++ b/libmultipath/checkers.c
> > @@ -27,6 +27,7 @@ struct checker_class {
> > void (*reset)(void); /* to reset the global
> > variables */
> > void *(*thread)(void *); /* async thread entry
> > point */
> > int (*pending)(struct checker *); /* to recheck pending
> > paths */
> > + bool (*need_wait)(struct checker *); /* checker needs
> > waiting for */
> > const char **msgtable;
> > short msgtable_size;
> > };
> > @@ -182,7 +183,8 @@ static struct checker_class
> > *add_checker_class(const char *name)
> > c->reset = (void (*)(void)) dlsym(c->handle,
> > "libcheck_reset");
> > c->thread = (void *(*)(void*)) dlsym(c->handle,
> > "libcheck_thread");
> > c->pending = (int (*)(struct checker *)) dlsym(c->handle,
> > "libcheck_pending");
> > - /* These 4 functions can be NULL. call dlerror() to clear
> > out any
> > + c->need_wait = (bool (*)(struct checker *)) dlsym(c->handle,
> > "libcheck_need_wait");
> > + /* These 5 functions can be NULL. call dlerror() to clear
> > out any
> > * error string */
> > dlerror();
> >
> > @@ -313,6 +315,14 @@ int checker_get_state(struct checker *c)
> > return c->path_state;
> > }
> >
> > +bool checker_need_wait(struct checker *c)
> > +{
> > + if (!c || !c->cls || c->path_state != PATH_PENDING ||
> > + !c->cls->need_wait)
> > + return false;
> > + return c->cls->need_wait(c);
> > +}
> > +
> > void checker_check (struct checker * c, int path_state)
> > {
> > if (!c)
> > diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
> > index b2342a1b..da91f499 100644
> > --- a/libmultipath/checkers.h
> > +++ b/libmultipath/checkers.h
> > @@ -2,6 +2,7 @@
> > #define CHECKERS_H_INCLUDED
> >
> > #include <pthread.h>
> > +#include <stdbool.h>
> > #include "list.h"
> > #include "defaults.h"
> >
> > @@ -171,6 +172,7 @@ struct checker_context {
> > int start_checker_thread (pthread_t *thread, const pthread_attr_t
> > *attr,
> > struct checker_context *ctx);
> > int checker_get_state(struct checker *c);
> > +bool checker_need_wait(struct checker *c);
> > void checker_check (struct checker *, int);
> > int checker_is_sync(const struct checker *);
> > const char *checker_name (const struct checker *);
> > @@ -191,7 +193,7 @@ void *libcheck_thread(struct checker_context
> > *ctx);
> > void libcheck_reset(void);
> > int libcheck_mp_init(struct checker *);
> > int libcheck_pending(struct checker *c);
> > -
> > +bool libcheck_need_wait(struct checker *c);
> >
> > /*
> > * msgid => message map.
> > diff --git a/libmultipath/checkers/directio.c
> > b/libmultipath/checkers/directio.c
> > index 904e3071..4beed02e 100644
> > --- a/libmultipath/checkers/directio.c
> > +++ b/libmultipath/checkers/directio.c
> > @@ -65,6 +65,7 @@ struct directio_context {
> > struct aio_group *aio_grp;
> > struct async_req *req;
> > struct timespec endtime;
> > + bool waited_for;
> > };
> >
> > static struct aio_group *
> > @@ -295,6 +296,7 @@ check_pending(struct directio_context *ct, struct
> > timespec endtime)
> > int r;
> > struct timespec currtime, timeout;
> >
> > + ct->waited_for = true;
>
> Not sure if it's important, but strictly speaking, we have only waited
> for the checker if the endtime was in the future, which is often not
> the case. Otherwise we'd just have polled.
But we always set the endtime in the future when we're starting a new
request, so the first time the we call check_pending() we don't return
till we either get an answer or endtime has passed, which is what we
want waited_for to check (that we've give the checker till endtime to
respond).
-Ben
>
> Martin
>
>
> > while(1) {
> > get_monotonic_time(&currtime);
> > timespecsub(&endtime, &currtime, &timeout);
> > @@ -346,6 +348,7 @@ check_state(int fd, struct directio_context *ct,
> > int sync, int timeout_secs)
> > get_monotonic_time(&ct->endtime);
> > ct->endtime.tv_nsec += 1000 * 1000;
> > normalize_timespec(&ct->endtime);
> > + ct->waited_for = false;
> > }
> > ct->running++;
> > if (!sync)
> > @@ -386,6 +389,13 @@ static void set_msgid(struct checker *c, int
> > state)
> > }
> > }
> >
> > +bool libcheck_need_wait(struct checker *c)
> > +{
> > + struct directio_context *ct = (struct directio_context *)c-
> > >context;
> > + return (ct && ct->running && ct->req->state == PATH_PENDING
> > &&
> > + !ct->waited_for);
> > +}
> > +
> > int libcheck_pending(struct checker *c)
> > {
> > int rc;
> > diff --git a/libmultipath/checkers/tur.c
> > b/libmultipath/checkers/tur.c
> > index 81db565b..41d6b9c3 100644
> > --- a/libmultipath/checkers/tur.c
> > +++ b/libmultipath/checkers/tur.c
> > @@ -59,6 +59,7 @@ struct tur_checker_context {
> > struct checker_context ctx;
> > unsigned int nr_timeouts;
> > struct timespec endtime;
> > + bool waited_for;
> > };
> >
> > int libcheck_init (struct checker * c)
> > @@ -351,9 +352,17 @@ int check_pending(struct checker *c)
> > ct->thread = 0;
> > }
> >
> > + ct->waited_for = true;
> > return tur_status;
> > }
> >
> > +bool libcheck_need_wait(struct checker *c)
> > +{
> > + struct tur_checker_context *ct = c->context;
> > + return (ct && ct->thread && uatomic_read(&ct->running) != 0
> > &&
> > + !ct->waited_for);
> > +}
> > +
> > int libcheck_pending(struct checker *c)
> > {
> > struct tur_checker_context *ct = c->context;
> > @@ -463,6 +472,7 @@ int libcheck_check(struct checker * c)
> > pthread_mutex_unlock(&ct->lock);
> > ct->fd = c->fd;
> > ct->timeout = c->timeout;
> > + ct->waited_for = false;
> > uatomic_add(&ct->holders, 1);
> > uatomic_set(&ct->running, 1);
> > tur_set_async_timeout(c);
