Date: Wed, 02 May 2018 11:02:09 -0600 From: Ian Lepore <ian@freebsd.org> To: Mark Johnston <markj@FreeBSD.org>, Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-arch@FreeBSD.org Subject: Re: callout_stop() return value Message-ID: <1525280529.57768.230.camel@freebsd.org> In-Reply-To: <20180502164002.GC24397@raichu> References: <20180502152024.GA24397@raichu> <1525275297.57768.202.camel@freebsd.org> <20180502154237.GB24397@raichu> <20180502162412.GA6887@kib.kiev.ua> <20180502164002.GC24397@raichu>
next in thread | previous in thread | raw e-mail | index | archive | help
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). 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. -- Ian > > > > I gave up on trying to get this fixed. r302350 was not the first time > > the return value was broken. > > > > I had to do r303426 exactly for this reason. > What kind of fix did you envision? The problem seems to be that > callout_stop()'s return value simply does not contain enough information > for certain use cases.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1525280529.57768.230.camel>