svn commit: r287780 - in head: share/man/man9 sys/kern sys/sys

[Hans Petter Selasky]

On 09/18/15 22:33, Davide Italiano wrote:
> On Thu, Sep 17, 2015 at 12:20 AM, Hans Petter Selasky <hps at selasky.org> wrote:
>> On 09/17/15 00:05, Gleb Smirnoff wrote:
>>>
>>> Weren't you explicitly asked not to touch this system without a proper
>>> review and discussion?
>>
>>
>> Adding a new function is not touching code.
>>
>> --HPS
>>
>
> I tried to stay away from this conversation as much as I could, partly
> because I haven't touched this code in a couple of years, partly
> because others have already expressed concerns. Sorry if I sound a
> little bit harsh here but this is not your playground.

Hi Davide,

If sys/kern/kern_timeout.c is not open for all, please state this in 
MAINTAINERS then. I will respect that.

> As somebody who spent a lot of time working on callout in the past I'm
> completely opposed to introducing new KPI without proper review.
> This has several problems:
> 1) It's a dead KPI, i.e. it has no consumers, which is a fairly bad
> engineering practice.
 > 2) Your commit message didn't explain what (if any) is the use case 
for this.

It currently has one critical client, and that is destruction of TCP 
connections.

The last 6 months there has been terribly much discussion, bugs and 
panics in the callout area, and there seems no end with recent panics 
posted to -current. Even the updates which rss & more did slipped in new 
bugs.

I'm going to let Julien finish his work first. If he doesn't need the 
KPI it will be removed. Else I want that it stays in.

> 3) You didn't discuss it with anybody else. Review timeout is not an
> excuse. If you want somebody to review this code ping -current or in
> the worst case developers at . Writing interfaces is hard, most of the
> times when you introduce a new one you may miss something useful,
> that's why we have reviews. It happened in the past when me and mav@
> introduced the new callout precision API and we had to change all the
> functions in the middle of development because of external feedback.

There has been plenty discussion about callout_drain_async() at this 
very list. Refer to discussions earlier this year, where several people 
stated the need for a callout drain async function. Even rss promised to 
make an implementation.

> Not even talking about the possibility of us changing our mind and
> removing this KPI after 11 is shipped, while third-party vendors
> started using it and very unhappily have to #ifdef their
> drivers, or even worse drop FreeBSD support.

> I personally don't think that your past/records have some influence on
> this. You introduced a new KPI, you took this decision lightly, and
> completely ignored complaints from others. This is a very bad attitude
> problem, and that's so much worse than all the technical problems this
> commit brings.

Again, please add to MAINTAINERS who owns kern/kern_timeout.c and does 
pre-commit reviews. I will respect that and not that developers claim 
sudden ownership of code.

--HPS