svn commit: r302998 - head/sys/kern

[Randall Stewart]

Gleb

Ok

I have now updated

https://reviews.freebsd.org/D7135

You can take this or not… I really don’t care either way… (you are welcome to
own the kern_timeout.c code I hate it) :-)

Basically when you went off and re-factored kern_timeout.c I had worked in parallel on fixing
the bugs you were seeing.. There were three distinct problems that I fixed… but then
you had refactored the stop() routine.. and I thought ok.. thats fine. I had actually thought about
doing something similar to what you did and was too chicken to poke that much at it.. it has
always had a nasty habit of biting back when you make a lot of changes :-D

I know my version has worked for quite some time in my testing so I brought it back.
Complete with its 3 return codes (I only recently switched to your version and thus
started having difficulties with leaks and crashes)….

You are welcome not to use this..  I know it works (it ran
on a number of machines at NF last night.. and we will of course continue testing
it as we finish our dev testing for the upcoming OCA software release).. For now
this is what will be going out into the OCA’s at least :-)

R

> On Jul 18, 2016, at 6:19 PM, Randall Stewart <rrs at netflix.com> wrote:
> 
> I have worked out a fix of this in Netflix code base (I have the same code running there). I
> will get that tested tonight I will get the fixes in to restore the behavior.
> 
> I will setup a phabricator shortly.. most likely I will update the one I already
> have on the one problem your earlier patch did not fix.
> 
> R
>> On Jul 18, 2016, at 5:44 PM, Randall Stewart <rrs at netflix.com> wrote:
>> 
>> Gleb:
>> 
>> This now leaks TCP-PCB’s since you have broken the return codes with all your
>> fixes that used to be in here.
>> 
>> It was
>> 
>> return 1 — You stopped the callout
>> return 0 — The callout could not be stopped
>> return -1 — The callout was not running.
>> 
>> The LLRef code that was crashing in in.c depended on this to know to free
>> the memory.. i.e. if was > 0 then they needed to free the memory.
>> 
>> TCP depends on a return 0 to indicate the async-drain function will be called back and
>> thus increments a refcnt and waits for the callback.
>> 
>> You now return 0 when no timer was active.. which makes the stack then wait
>> for the not forth coming async-drain call.
>> 
>> R
>>> On Jul 18, 2016, at 11:29 AM, Gleb Smirnoff <glebius at freebsd.org> wrote:
>>> 
>>> Author: glebius
>>> Date: Mon Jul 18 09:29:08 2016
>>> New Revision: 302998
>>> URL: https://svnweb.freebsd.org/changeset/base/302998
>>> 
>>> Log:
>>> Revert the last commit. It must get more review and testing first.
>>> 
>>> Modified:
>>> head/sys/kern/kern_timeout.c
>>> 
>>> Modified: head/sys/kern/kern_timeout.c
>>> ==============================================================================
>>> --- head/sys/kern/kern_timeout.c	Mon Jul 18 09:26:06 2016	(r302997)
>>> +++ head/sys/kern/kern_timeout.c	Mon Jul 18 09:29:08 2016	(r302998)
>>> @@ -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 (-1);
>>> +		return (0);
>>> 	}
>>> 
>>> 	c->c_iflags &= ~CALLOUT_PENDING;
>>> 
>> 
>> --------
>> Randall Stewart
>> rrs at netflix.com
>> 803-317-4952
>> 
>> 
>> 
>> 
>> 
> 
> --------
> Randall Stewart
> rrs at netflix.com
> 803-317-4952
> 
> 
> 
> 
> 

--------
Randall Stewart
rrs at netflix.com
803-317-4952