Date: Wed, 21 Jan 2015 15:38:47 -0800 From: "K. Macy" <kmacy@freebsd.org> To: Adrian Chadd <adrian@freebsd.org> Cc: Hans Petter Selasky <hps@selasky.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Jason Wolfe <nitroboost@gmail.com>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Sean Bruno <sbruno@freebsd.org>, Gleb Smirnoff <glebius@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com> Subject: Re: svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys Message-ID: <CAHM0Q_NJDq41MtDAne0a-fnp%2BOg38=ypNKRidq5cAeL=nj-YDQ@mail.gmail.com> In-Reply-To: <CAJ-VmokyKX0uCA1%2B3KjziJkokDOVBCpUdwHQcVaY=buwa%2BqfhQ@mail.gmail.com> References: <20150120075126.GA42409@kib.kiev.ua> <20150120211137.GY15484@FreeBSD.org> <54BED6FB.8060401@selasky.org> <54BEE62D.2060703@ignoranthack.me> <CAHM0Q_MDJN_8sTvTDXfqA7UtJVO3Y8S8%2BNRCs_=6Nj4dkTzjOA@mail.gmail.com> <54BEE8E6.3080009@ignoranthack.me> <CAHM0Q_N_53BM-6RvXu8UpjfDzQHEn5oXZo1Nn8RO0cuOUhe8tg@mail.gmail.com> <54BEEA7F.1070301@ignoranthack.me> <CAHM0Q_PtJ7JHFTiu9_dmi_Ce=rmu1j72z2OYQ2CD3%2BEbcoEGsA@mail.gmail.com> <54BEF154.3030606@ignoranthack.me> <20150121181512.GE15484@FreeBSD.org> <CAJ-VmokyKX0uCA1%2B3KjziJkokDOVBCpUdwHQcVaY=buwa%2BqfhQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> They originally found that things were spinning for way too long. > > Hans found something similar and determined/concluded that the > migration code in callouts was racy-by-design and dramatically > simplified it and also put very hard constraints on what is a valid > situation to support migrating from one callwheel to another. Now we > have fallout which we can either address or back out until the callout > stuff is again reviewed/fixed. > > I don't think it's as alchemic as is being promoted. Let's not get drawn into semantic debates. To me alchemy implies voodoo debugging, whereas this is, in part, disabling functionality that exposes races - which is more like disabling preemption or fine grained locking. Normally I would advocate backing this out on the basis of precedence. That is to say, back it out so that developers will get it right the first time. However, it appears that hps@ and some others don't understand what was done wrong. So I'm going to state some basic premises and then the implied basic guidelines required of a commit that were not met. If these guidelines are violated by a change in the future I will agitate for a rapid back out. Premises: A) A performance regression is a bug every bit as much as a locking race. Stability OR performance (where we are talking about _current_ performance) is a false dichotomy. - This means that a change that fixes a bug by introducing a substantial performance regression does not constitute a fix. B) Existing behavior and performance characteristics should not be adversely changed unless there is widespread consensus. - Sometimes it is necessary to "break" a KPI but that should not be discovered by svn or scanning back through the mailing lists. Guidelines: A) Performance should not measurably regress. B) If a KPI changes behavior e.g. callout_reset_on being crippled, all clients need to be updated so that they maintain their current behavior by the committer changing the KPI. The client code maintainers aren't responsible for reacting to these types of changes. That is OK for marginal architectures like sun4v or pc98. C) If there is a non-zero possibility that these changes break the client code, committers who have touched that code in the last few years should be notified. HPS: Your change failed to meet these guidelines. Some of us are upset because these guidelines are fairly fundamental for the on-going viability of FreeBSD. Due to linguistic / time zone / cultural differences these expectations have not been adequately communicated to you. You are not in the USB sandbox where others need for your support outweighs the inconvenience of random breakage. It sounds like you are making progress towards updating the concerns that have been voiced. If kib's observations are in fact comprehensive then adding a callout_init_cpu function and updating all clients so that their callouts continue to be scheduled on a CPU other than the BSP will suffice and we can all move on. Thanks in advance. -K
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHM0Q_NJDq41MtDAne0a-fnp%2BOg38=ypNKRidq5cAeL=nj-YDQ>