Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Jul 2016 16:29:45 +0200
From:      Hans Petter Selasky <hps@selasky.org>
To:        Gleb Smirnoff <glebius@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r302894 - head/sys/kern
Message-ID:  <23f60f35-db99-ea20-837b-becd9c5c1887@selasky.org>
In-Reply-To: <201607150928.u6F9SW3h065100@repo.freebsd.org>
References:  <201607150928.u6F9SW3h065100@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 07/15/16 11:28, Gleb Smirnoff wrote:
> Author: glebius
> Date: Fri Jul 15 09:28:32 2016
> New Revision: 302894
> URL: https://svnweb.freebsd.org/changeset/base/302894
>
> Log:
>   Fix regression introduced by r302350. The change of return value for a
>   callout that wasn't scheduled at all was unintentional and yielded in
>   several panics.
>
>   PR:		210884
>
> Modified:
>   head/sys/kern/kern_timeout.c
>
> Modified: head/sys/kern/kern_timeout.c
> ==============================================================================
> --- head/sys/kern/kern_timeout.c	Fri Jul 15 09:23:18 2016	(r302893)
> +++ head/sys/kern/kern_timeout.c	Fri Jul 15 09:28:32 2016	(r302894)
> @@ -1381,7 +1381,7 @@ again:
>  		CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p",
>  		    c, c->c_func, c->c_arg);
>  		CC_UNLOCK(cc);
> -		return (cancelled);
> +		return (0);
>  	}
>
>  	c->c_iflags &= ~CALLOUT_PENDING;
>
>

Hi,

I think r302894 and r302350 changes the return value of the following 
case, which is not described in the commit message? Is this also a 
regression?

In this revision:
> https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?view=markup&pathrev=296320

Assume we enter _callout_stop_safe() having the following assertions:

(c->c_iflags & CALLOUT_PENDING) == 0 (satisfied)
cc_exec_curr(cc, direct) != c (satisfied)

Then we exit returning (-1).

In this revision:
> https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?view=markup&pathrev=296320#l1253

After your changes, entering the same function under the same conditions:

cc_exec_curr(cc, direct) == c (not satisifed)
(c->c_iflags & CALLOUT_PENDING) == 0 (satisfied)

Then we exit returning (0).

> https://svnweb.freebsd.org/base/head/sys/kern/kern_timeout.c?revision=302894&view=markup#l1384

If we call callout_stop() on a never scheduled callout, we now get a 
return value of 0 instead of -1, which by manual page definition is 
wrong ????


Am I wrong? Do others see this too?


--HPS




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?23f60f35-db99-ea20-837b-becd9c5c1887>