Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 5 Nov 2008 10:44:08 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        David Xu <davidxu@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r184217 - head/sys/kern
Message-ID:  <200811051044.08650.jhb@freebsd.org>
In-Reply-To: <4910F8E7.4010805@freebsd.org>
References:  <200810240109.m9O19P1u071214@svn.freebsd.org> <200811041705.48043.jhb@freebsd.org> <4910F8E7.4010805@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 04 November 2008 08:37:43 pm David Xu wrote:
> John Baldwin wrote:
> > On Thursday 23 October 2008 09:09:25 pm David Xu wrote:
> >> Author: davidxu
> >> Date: Fri Oct 24 01:09:24 2008
> >> New Revision: 184217
> >> URL: http://svn.freebsd.org/changeset/base/184217
> >>
> >> Log:
> >>   Don't rearm callout if the process is exiting, it may leak a callout
> >>   because callout_drain() only waits for running callout, but not disable
> >>   it if it is rearmed.
> > 
> > Can you please revert this as per previous discussion?
> > 
> 
> I will do it, maybe I lost previous discussion ?

I thought someone else had followed up to my original e-mail, but here are my 
comments from when this was first committed:

> Log:
>   Don't rearm callout if the process is exiting, it may leak a callout
>   because callout_drain() only waits for running callout, but not disable
>   it if it is rearmed.

Actually, it does prevent rearming from within the callout routine itself 
while waiting:

int
callout_reset_on(struct callout *c, int to_ticks, void (*ftn)(void *),
    void *arg, int cpu)
{
        ...
        if (cc->cc_curr == c) {
                ...
                if (cc->cc_waiting) {
                        /*
                         * Someone has called callout_drain to kill this
                         * callout.  Don't reschedule.
                         */
                        CTR4(KTR_CALLOUT, "%s %p func %p arg %p",
                            cancelled ? "cancelled" : "failed to cancel",
                            c, c->c_func, c->c_arg);
                        CC_UNLOCK(cc);
                        return (cancelled);
                }
        }
        ...
}

Lots of callouts using callout_init_mtx() (e.g. all the callouts in the NIC 
drivers) depend on this feature.  Please revert this.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811051044.08650.jhb>