From owner-svn-src-head@freebsd.org Sat Sep 19 08:53:38 2015 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E2181A05E53; Sat, 19 Sep 2015 08:53:37 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (heidi.turbocat.net [88.198.202.214]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 5DB3A1C1A; Sat, 19 Sep 2015 08:53:33 +0000 (UTC) (envelope-from hps@selasky.org) Received: from laptop015.home.selasky.org (cm-176.74.213.204.customer.telag.net [176.74.213.204]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id D7BB21FE023; Sat, 19 Sep 2015 10:53:31 +0200 (CEST) Subject: Re: svn commit: r287780 - in head: share/man/man9 sys/kern sys/sys To: Davide Italiano References: <201509141052.t8EAqRWf008293@repo.freebsd.org> <20150916220559.GS1023@FreeBSD.org> <55FA69BD.10507@selasky.org> Cc: Gleb Smirnoff , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , Julien Charbon , rss@freebsd.org From: Hans Petter Selasky Message-ID: <55FD22E9.2040508@selasky.org> Date: Sat, 19 Sep 2015 10:55:05 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Sep 2015 08:53:38 -0000 On 09/18/15 22:33, Davide Italiano wrote: > On Thu, Sep 17, 2015 at 12:20 AM, Hans Petter Selasky 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