On 09/18/15 22:33, Davide Italiano wrote:
On Thu, Sep 17, 2015 at 12:20 AM, Hans Petter Selasky <h...@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@. 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
_______________________________________________
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"
