Re: [PATCH v2 19/22] libmultipath: add libcheck_need_wait checker function

[Benjamin Marzinski <bmarzins@redhat.com>]

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