From owner-freebsd-arch@freebsd.org Wed May 2 21:43:08 2018 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4963FFB5513 for ; Wed, 2 May 2018 21:43:08 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-it0-x229.google.com (mail-it0-x229.google.com [IPv6:2607:f8b0:4001:c0b::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id CA17B6B74C; Wed, 2 May 2018 21:43:07 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-it0-x229.google.com with SMTP id n202-v6so18754239ita.1; Wed, 02 May 2018 14:43:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ZkluGu76sp/XCUV/0Y64jqCmvAdK79q5q1o59u8Ul2c=; b=dWVECj1tUWtuhB4Z2WiH2y7azeFCS/L3xWP6Ec0UUAFzHlwqmNqoYmg8bwdADXrV0O TDLhc/gYKzyXV5iwiPYuA36/1bZL8XfgBp5qntyvk/6OeJ9fW5qxFGrv3WB3FhUuSiSo 1W6nFP71fnV1l7co6dUWNSZSNHwK4FtdVvB1RW+ARMdrkGpSU0stXV7c8L8DGk+9Wk4a o4C+QPDzq9QxIeuJ293HJIWEjx61j0fS1aphcdkzFwqTxViYuF+hkectLPa9Gj8eUXYa rYNe1oZJEoGQunvMvysyKevK0Tk3bOHtyalKPjrmMO7uVQdjyAjYYm1D/JlWdOP7NBvG d0Aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=ZkluGu76sp/XCUV/0Y64jqCmvAdK79q5q1o59u8Ul2c=; b=DsTjHWUkATNFGzjxayxl4y6QUuNMyA86SW2b9YmZS6SfreIxfPqmYfBvS7m29MTRMx Wy+PnKjzNEYWMbz/aUnF0IqkfzPqZc3vPI5LsPFgp6asdNaPirFRCZ34vg2ZbcAdYA4L dV3dcmf7PINH6iHYBBjsDgKPx1UalyHY8gFakqBwFah84eIGRhM5LkrfLKfuBKU8xy6Z Od1xsQHOjehv93B+0MTr/8bFME1C9UR+xk3GJMH9ls9ZK606T1sIKbpx7V4QH+Nt0A2H +NnoJOPmlm60CDsAg82d4xWvzNlfBOL8gfbdZCVhfBva4YKKI4jI52/Ebgu2gMFMpIRV renw== X-Gm-Message-State: ALQs6tAc20tjR0A6u53dDAYpatPPGJz93tqEZ0LgF20xlNZoKGbxAZSP EPch7640isVy0NKhfH0rnbtF8A== X-Google-Smtp-Source: AB8JxZqcHFdnvOtyqdNB5w2BF8Gxnztlz/fK2pzM1xmiPXbXm2J0sZ+Q0ZkHmSbKrWaPuQD5PECniw== X-Received: by 2002:a24:97c4:: with SMTP id k187-v6mr8023011ite.115.1525297386916; Wed, 02 May 2018 14:43:06 -0700 (PDT) Received: from raichu (toroon0560w-lp130-04-184-145-252-74.dsl.bell.ca. [184.145.252.74]) by smtp.gmail.com with ESMTPSA id o1-v6sm4562821ite.37.2018.05.02.14.43.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 May 2018 14:43:06 -0700 (PDT) Sender: Mark Johnston Date: Wed, 2 May 2018 17:43:03 -0400 From: Mark Johnston To: Ian Lepore Cc: Konstantin Belousov , freebsd-arch@FreeBSD.org Subject: Re: callout_stop() return value Message-ID: <20180502214303.GE24397@raichu> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1525280529.57768.230.camel@freebsd.org> User-Agent: Mutt/1.9.5 (2018-04-13) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 May 2018 21:43:08 -0000 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.