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 <r...@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 <r...@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 <gleb...@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 >> r...@netflix.com >> 803-317-4952 >> >> >> >> >> > > -------- > Randall Stewart > r...@netflix.com > 803-317-4952 > > > > > -------- Randall Stewart r...@netflix.com 803-317-4952 _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
