Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 19 Sep 2015 18:38:46 -0700
From:      Adrian Chadd <adrian.chadd@gmail.com>
To:        Bruce Simpson <bms@fastmail.net>
Cc:        Ian Lepore <ian@freebsd.org>, George Neville-Neil <gnn@neville-neil.com>,  Hans Petter Selasky <hps@selasky.org>, David Chisnall <theraven@freebsd.org>,  Gleb Smirnoff <glebius@freebsd.org>,  "src-committers@freebsd.org" <src-committers@freebsd.org>,  "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>,  "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r287780 - in head: share/man/man9 sys/kern sys/sys
Message-ID:  <CAJ-VmoncfCo3ZaP1MY1iLMDZN3piTM_hd0XBJ1xFc=VpySEJsg@mail.gmail.com>
In-Reply-To: <55FDA1E7.8050007@fastmail.net>
References:  <201509141052.t8EAqRWf008293@repo.freebsd.org> <20150916220559.GS1023@FreeBSD.org> <55FA69BD.10507@selasky.org> <0952027A-5276-487D-99B8-74747B0EEF5D@FreeBSD.org> <55FD23C5.5010008@selasky.org> <64D8263B-1F5D-40E5-994C-479C39B69DC9@neville-neil.com> <1442684369.1224.179.camel@freebsd.org> <55FDA1E7.8050007@fastmail.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On 19 September 2015 at 10:56, Bruce Simpson <bms@fastmail.net> wrote:
> Ian,
>
> To paraphrase what I said privately to the various dramatis personae in
> January:
>
> Changes like this need to be reviewed before they go in. As timing is
> central to the entire OS, change review has to be meticulous, on par with
> the virtual memory management. We have a VM tsar; we do not have a timing
> tsar.
>
> On 19/09/15 18:39, Ian Lepore wrote:
>>
>> I'm afraid this message can be interpetted as "reviews are now mandatory
>> for a 'core component of the system' (whatever that means)".  If so,
>> this would be a Big Change from the last thing I heard about code
>> reviews, which was basically: as much as some people would like it to be
>> so, they are not mandatory.
>
> ...
>>
>> a definition as meaningless and vague as[.]
>
>
> I don't believe for one moment that George is advocating for mandatory
> reviews. But common sense should apply, _as timing is central to the entire
> OS_. It touches absolutely everything.
>
> Hell, I wouldn't feel comfortable checking anything in to e.g. timecounters
> without at least running it by phk first.
>
> So, +1 from me for backout until the change can be reviewed.
>
> But to be fair to all involved, perhaps Hans should set out a timeline for
> how he wants to proceed, subject to the jitter inherent to an all-volunteer
> project.

What isn't necessarily public knowledge is the sheer volume of emails
that went out a few months ago whilst chasing down callout and tcp
bugs. There were (and maybe still are) very subtle bugs in the callout
system and after a few attempts at fixing them there were some very
careful bug fixes made. Some attempts failed, I think a couple of
successful ones made it into the tree.

Yes, this whole callout system is very delicate at the moment. hps@
has some very specific ideas of how the API should behave in order to
be predictable/reasoning-able (and I agree with him about almost all
of it, even though it makes RSS painful, but that's because of our TCP
stack and how we use callouts, not because its his fault!) but it's a
pretty big fundamental change to how things currently work and he was
shot down. I think people are just very weary of new changes.

On the flip side, he did actively solicit reviews - rrs, kib, hiren,
jhb, wblock and jch were included in the review request, which dates
back to August 28. He gave people a little short of three weeks for
review before he committed the code. So as much as I'm cautious about
things (and it gets me in trouble at work, hi alfred!) I think he did
the right thing here - he added a new thing, documented it, solicited
a review, and it timed out. If people would like more time to review
it then fine, but please give him either a firm "no, not ever" right
now and be honest about your intentions, or give him a timeframe that
you'll review it before it times out.

Hans - personally, I think you should've emailed out a review request
on freebsd-arch@ and put out a request for testers and give a firm
date that you'll commit it. That makes it all very explicit.

People channel phrases involving silence and agreement and all that;
this is one of those times it happened.


-adrian



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmoncfCo3ZaP1MY1iLMDZN3piTM_hd0XBJ1xFc=VpySEJsg>