From owner-freebsd-smp@FreeBSD.ORG  Fri Mar 21 19:08:03 2008
Return-Path: <owner-freebsd-smp@FreeBSD.ORG>
Delivered-To: freebsd-smp@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 6C2581065679;
	Fri, 21 Mar 2008 19:08:03 +0000 (UTC) (envelope-from jhb@freebsd.org)
Received: from speedfactory.net (mail.speedfactory.net [66.23.216.219])
	by mx1.freebsd.org (Postfix) with ESMTP id B58408FC1E;
	Fri, 21 Mar 2008 19:08:02 +0000 (UTC) (envelope-from jhb@freebsd.org)
Received: from server.baldwin.cx (unverified [66.23.211.162]) 
	by speedfactory.net (SurgeMail 3.8s) with ESMTP id 236285243-1834499 
	for multiple; Fri, 21 Mar 2008 15:09:00 -0400
Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1])
	(authenticated bits=0)
	by server.baldwin.cx (8.14.2/8.14.2) with ESMTP id m2LJ7d5t029644;
	Fri, 21 Mar 2008 15:07:39 -0400 (EDT) (envelope-from jhb@freebsd.org)
From: John Baldwin <jhb@freebsd.org>
To: freebsd-smp@freebsd.org
Date: Fri, 21 Mar 2008 13:14:03 -0400
User-Agent: KMail/1.9.7
References: <20080321043225.GZ67856@elvis.mu.org>
In-Reply-To: <20080321043225.GZ67856@elvis.mu.org>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200803211314.04002.jhb@freebsd.org>
X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by
	milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]);
	Fri, 21 Mar 2008 15:07:40 -0400 (EDT)
X-Virus-Scanned: ClamAV 0.91.2/6316/Fri Mar 21 10:29:54 2008 on
	server.baldwin.cx
X-Virus-Status: Clean
X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 
	autolearn=ham version=3.1.3
X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx
Cc: Alfred Perlstein <alfred@freebsd.org>
Subject: Re: request for review, callout fix.
X-BeenThere: freebsd-smp@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: FreeBSD SMP implementation group <freebsd-smp.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-smp>,
	<mailto:freebsd-smp-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-smp>
List-Post: <mailto:freebsd-smp@freebsd.org>
List-Help: <mailto:freebsd-smp-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-smp>,
	<mailto:freebsd-smp-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 21 Mar 2008 19:08:03 -0000

On Friday 21 March 2008 12:32:25 am Alfred Perlstein wrote:
> Hi guys, attached is a fix for the timeout/untimeout race with
> Giant locked code.
> 
> Basically the old code would make the callout inaccessable
> right before calling it inside of softclock.
> 
> However only the callout lock is held, so when switching to
> the callout's associated mutex (in this case Giant) there's
> a race where a "local" (timeout/untimeout) callout would be
> fired even if stopped.
> 
> This patch fixes that.  We've run several hours of regression
> testing on a version of this for 6.x.
> 
> People internal to Juniper and iedowse@ helped with this.
> 
> Please review/comment.

Curious as to how c->c_flags could change if CALLOUT_LOCAL_ALLOC is set?  
Since it hasn't been enqueued on the callfree list, it isn't visible to any 
other code, so nothing should be able to mark it active or pending.  IOW, you 
should be able to do this in your second hunk:

	if (c_flags & CALLOUT_LOCAL_ALLOC) {
		KASSERT(c->c_flags == CALLOUT_LOCAL_ALLOC, ("corrupted callout"));
		c->c_func = NULL;
		SLIST_INSERT_HEAD(...);
	}

-- 
John Baldwin