Date: Fri, 21 Mar 2008 13:14:03 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-smp@freebsd.org Cc: Alfred Perlstein <alfred@freebsd.org> Subject: Re: request for review, callout fix. Message-ID: <200803211314.04002.jhb@freebsd.org> In-Reply-To: <20080321043225.GZ67856@elvis.mu.org> References: <20080321043225.GZ67856@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200803211314.04002.jhb>