Date: Wed, 2 May 2018 20:00:39 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Mark Johnston <markj@FreeBSD.org> Cc: Ian Lepore <ian@freebsd.org>, freebsd-arch@FreeBSD.org Subject: Re: callout_stop() return value Message-ID: <20180502170038.GB6887@kib.kiev.ua> 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, May 02, 2018 at 12:40:02PM -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. > > > 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. The fix is really social, to make people who change the callout_stop() KPI to care about non-tcp stack uses. I tried to add a flag to _callout_stop_safe() to make return values useful for sleepqueues, see r296320. But as I said, it was broken again and I gave up.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180502170038.GB6887>