svn commit: r302894 - head/sys/kern

[Gleb Smirnoff]

On Fri, Jul 15, 2016 at 04:29:45PM +0200, Hans Petter Selasky wrote:
H> On 07/15/16 11:28, Gleb Smirnoff wrote:
H> > Author: glebius
H> > Date: Fri Jul 15 09:28:32 2016
H> > New Revision: 302894
H> > URL: https://svnweb.freebsd.org/changeset/base/302894
H> >
H> > Log:
H> >   Fix regression introduced by r302350. The change of return value for a
H> >   callout that wasn't scheduled at all was unintentional and yielded in
H> >   several panics.
H> >
H> >   PR:		210884
H> >
H> > Modified:
H> >   head/sys/kern/kern_timeout.c
H> >
H> > Modified: head/sys/kern/kern_timeout.c
H> > ==============================================================================
H> > --- head/sys/kern/kern_timeout.c	Fri Jul 15 09:23:18 2016	(r302893)
H> > +++ head/sys/kern/kern_timeout.c	Fri Jul 15 09:28:32 2016	(r302894)
H> > @@ -1381,7 +1381,7 @@ again:
H> >  		CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p",
H> >  		    c, c->c_func, c->c_arg);
H> >  		CC_UNLOCK(cc);
H> > -		return (cancelled);
H> > +		return (0);
H> >  	}
H> >
H> >  	c->c_iflags &= ~CALLOUT_PENDING;
H> >
H> >
H> 
H> Hi,
H> 
H> I think r302894 and r302350 changes the return value of the following 
H> case, which is not described in the commit message? Is this also a 
H> regression?
H> 
H> In this revision:
H> > https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?view=markup&pathrev=296320
H> 
H> Assume we enter _callout_stop_safe() having the following assertions:
H> 
H> (c->c_iflags & CALLOUT_PENDING) == 0 (satisfied)
H> cc_exec_curr(cc, direct) != c (satisfied)
H> 
H> Then we exit returning (-1).
H> 
H> In this revision:
H> > https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?view=markup&pathrev=296320#l1253
H> 
H> After your changes, entering the same function under the same conditions:
H> 
H> cc_exec_curr(cc, direct) == c (not satisifed)
H> (c->c_iflags & CALLOUT_PENDING) == 0 (satisfied)
H> 
H> Then we exit returning (0).
H> 
H> > https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?revision=302894&view=markup#l1384
H> 
H> If we call callout_stop() on a never scheduled callout, we now get a 
H> return value of 0 instead of -1, which by manual page definition is 
H> wrong ????
H> 
H> Am I wrong? Do others see this too?

You are right, the new -1 value was introduced by you and rrs@ in the
commit:

------------------------------------------------------------------------
r290664 | rrs | 2015-11-10 06:49:32 -0800 (вт, 10 нояб. 2015) | 6 lines

Add new async_drain to the callout system. This is so-far not used but
should be used by TCP for sure in its cleanup of the IN-PCB (will be coming shortly).

Sponsored by:   Netflix Inc.
Differential Revision:  https://reviews.freebsd.org/D4076

------------------------------------------------------------------------

So commit not only extended the KPI, but also changed it. And I missed that,
using stable/10 version as reference.

-- 
Totus tuus, Glebius.