| Message ID | 20240912214947.783819-20-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Benjamin Marzinski |
| Headers | show |
| Series | Yet Another path checker refactor | expand |
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@redhat.com> > --- > 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. 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);
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@redhat.com> > > --- > > 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);
On Tue, 2024-10-08 at 15:33 -0400, Benjamin Marzinski wrote: > 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@redhat.com> > > > --- > > > 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). That holds for this patch. But right in the next patch, the code is changed such that libcheck_pending doesn't actually wait at all. So "ct->waited_for" is a really confusing name. It just means that we have polled for the state once. In theory that can happen very quickly after starting the checker. Regards Martin
On Wed, Oct 09, 2024 at 05:49:33PM +0200, Martin Wilck wrote: > On Tue, 2024-10-08 at 15:33 -0400, Benjamin Marzinski wrote: > > 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: <snip> > > > > 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). > > That holds for this patch. But right in the next patch, the code is > changed such that libcheck_pending doesn't actually wait at all. So > "ct->waited_for" is a really confusing name. It just means that we have > polled for the state once. In theory that can happen very quickly after > starting the checker. Oh. Yeah. I can change the of the variable to something like is_unpolled, and the calling function to checker_is_unpolled(), which matches what we're actually asking for. -Ben > Regards > Martin
On Mon, Oct 14, 2024 at 01:48:14PM -0400, Benjamin Marzinski wrote: > On Wed, Oct 09, 2024 at 05:49:33PM +0200, Martin Wilck wrote: > > On Tue, 2024-10-08 at 15:33 -0400, Benjamin Marzinski wrote: > > > 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: > > <snip> > > > > > > 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). > > > > That holds for this patch. But right in the next patch, the code is > > changed such that libcheck_pending doesn't actually wait at all. So > > "ct->waited_for" is a really confusing name. It just means that we have > > polled for the state once. In theory that can happen very quickly after > > starting the checker. > > Oh. Yeah. I can change the of the variable to something like > is_unpolled, and the calling function to checker_is_unpolled(), which > matches what we're actually asking for. Actually, I'm going to leave the calling functon as checker_need_wait(), since the question we're asking the checker is whether or not we need to wait. Even if the checker is unpolled, if it's not pending, there's no point in waiting. Inside the checkers, I'll change the variable name to checked_state, which makes more sense than is_unpolled in the initial patch, where you're still waiting instead of just polling. -Ben > > -Ben > > > Regards > > Martin
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; 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);
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@redhat.com> --- 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(-)