Date: Sat, 19 Sep 2015 10:55:05 +0200 From: Hans Petter Selasky <hps@selasky.org> To: Davide Italiano <davide@freebsd.org> Cc: 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>, Julien Charbon <jch@FreeBSD.org>, rss@freebsd.org Subject: Re: svn commit: r287780 - in head: share/man/man9 sys/kern sys/sys Message-ID: <55FD22E9.2040508@selasky.org> In-Reply-To: <CACYV=-EaGiUkjnoAH%2BJLgyqBWYx7Mw=r-ADEwfr0MQ8VsT7iJw@mail.gmail.com> References: <201509141052.t8EAqRWf008293@repo.freebsd.org> <20150916220559.GS1023@FreeBSD.org> <55FA69BD.10507@selasky.org> <CACYV=-EaGiUkjnoAH%2BJLgyqBWYx7Mw=r-ADEwfr0MQ8VsT7iJw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 09/18/15 22:33, Davide Italiano wrote: > On Thu, Sep 17, 2015 at 12:20 AM, Hans Petter Selasky <hps@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55FD22E9.2040508>