Date: Wed, 2 May 2018 17:43:03 -0400 From: Mark Johnston <markj@FreeBSD.org> To: Ian Lepore <ian@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, freebsd-arch@FreeBSD.org Subject: Re: callout_stop() return value Message-ID: <20180502214303.GE24397@raichu> In-Reply-To: <1525280529.57768.230.camel@freebsd.org> References: <20180502152024.GA24397@raichu> <1525275297.57768.202.camel@freebsd.org> <20180502154237.GB24397@raichu> <20180502162412.GA6887@kib.kiev.ua> <20180502164002.GC24397@raichu> <1525280529.57768.230.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 02, 2018 at 11:02:09AM -0600, Ian Lepore wrote: > On Wed, 2018-05-02 at 12:40 -0400, Mark Johnston wrote: > > On Wed, May 02, 2018 at 07:24:12PM +0300, Konstantin Belousov wrote: > > > > > > On Wed, May 02, 2018 at 11:42:37AM -0400, Mark Johnston wrote: > > > > > > > > On Wed, May 02, 2018 at 09:34:57AM -0600, Ian Lepore wrote: > > > > > > > > > > On Wed, 2018-05-02 at 11:20 -0400, Mark Johnston wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > We have a few pieces of code that follow a pattern: a thread > > > > > > allocates a > > > > > > resource and schedules a callout. The callout handler releases the > > > > > > resource and never reschedules itself. In the common case, a thread > > > > > > will cancel the callout before it executes. To know whether it must > > > > > > release the resource associated with the callout, this thread must > > > > > > know > > > > > > whether the pending callout was cancelled. > > > > > > > > > > > It seems to me a better solution would be to track the state / lifetime > > > > > of the resource separately rather than trying to infer the state of the > > > > > resource from the state of the callout as viewed through a semi-opaque > > > > > interface. > > > > > > > > > > If the original thread that schedules the callout keeps enough > > > > > information about the resource to free it after cancelling, then it is > > > > > already maintaining some kind of sideband info about the resource, even > > > > > if it's just a pointer to be freed. Couldn't the callout routine > > > > > manipulate this resource tracking info (null out the pointer or set a > > > > > bool or whatever), then after cancelling you don't really care whether > > > > > the callout ran or not, you just examine the pointer/bool/whatever and > > > > > free or not based on that. > > > > I'd considered that. It's not quite as elegant a solution as you > > > > suggest, since in my case the resource is embedded in an opaque > > > > structure, so I need to add an extra field beside the callout to track > > > > state that's already tracked by the callout subsystem. That plus the > > > > fact that we have multiple instances of this bug make me want to fix it > > > > in a general way, though I recognize that the callout API is already > > > > overly complicated. > > I forgot to add that in some cases, extra synchronization would be > > needed to maintain this hypothetical flag. Specifically, some of these > > callouts do not have an associated lock, so the callout handler doesn't > > have an easy way to mark the resource as consumed. > > > > Wouldn't there be implied temporal synchronization? The callout will > free the resource and manipulate the sideband info to say so, knowing > that nothing else will modify it or even examine it at the same time > because the callout would be cancelled and drained before the resource > is manipulated by someone else. Conversely, the someone-else doesn't > examine or modify the resource or sideband info until the callout is > drained (whether cancelled or it just runs to normal completion). In one case, the callout isn't drained but simply stopped. Only if callout_stop() returns 1 (i.e., we cancelled a future invocation AND the callout wasn't running at the time of the call) do we release the resource. However, if the call returns 0 we don't know whether to release the resource. In this case though, I think it's sufficient to check whether callout_pending() is true before calling callout_stop(). I will spend some time trying to fix these issues locally before trying to push this topic further. > Only if it were impossible to reliably cancel the callout and reach a > point where you know it's not running would there be any chance of a > race that needs explicit locking.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180502214303.GE24397>