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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux