From owner-svn-src-all@FreeBSD.ORG Wed Jan 21 08:16:13 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A6BBE68A; Wed, 21 Jan 2015 08:16:13 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4741B93E; Wed, 21 Jan 2015 08:16:13 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t0L8G6tr025128 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 21 Jan 2015 10:16:07 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t0L8G6tr025128 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t0L8G6kF025127; Wed, 21 Jan 2015 10:16:06 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 21 Jan 2015 10:16:06 +0200 From: Konstantin Belousov To: "K. Macy" Subject: Re: svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys Message-ID: <20150121081606.GN42409@kib.kiev.ua> References: <20150120211137.GY15484@FreeBSD.org> <54BED6FB.8060401@selasky.org> <54BEE62D.2060703@ignoranthack.me> <54BEE8E6.3080009@ignoranthack.me> <54BEEA7F.1070301@ignoranthack.me> <54BEF154.3030606@ignoranthack.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: Hans Petter Selasky , Adrian Chadd , "src-committers@freebsd.org" , Jason Wolfe , "svn-src-all@freebsd.org" , Sean Bruno , Gleb Smirnoff , "svn-src-head@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jan 2015 08:16:13 -0000 On Tue, Jan 20, 2015 at 04:37:44PM -0800, K. Macy wrote: > I would pick stability over performance any day. However, it _seems_ > to me, and maybe I simply don't understand some key details, that the > fix consisted of largely single-threading the callout system. And as I > say I may simply not understand the specifics, but this sort of large > scale disabling does not constitute a fix but is a workaround. It's > more like disabling preemption because it fixes a panic. Yes, it might > "fix" a whole array of bugs that crop up but it could not be seen as a > fix to an otherwise working system. Well, this is not a complete truth. Let me try to explain my understanding, I spent some time actually looking at the new code. Would be nice if corrections to my understanding is posted, if needed. Now, when callout_reset() is directed to change cpu, the change only happens when the callout is associated with a lock, and that lock is owned by the caller of callout_reset(). There are two consequences. One, which is good, is significant simplification of the migration code, together with the drain. This is exactly the place where there bugs which make my head hurt, see e.g. r234952 and r243901. Note that some callouts follow this pattern already, e.g. process timers after r243869. Another consequence, which is very visible and which causes the roar, is that all lockless callers of callout_reset_on(9) now silently changed the behaviour to callout_reset(9). There is no audit performed to identify such callers, and there is no even a plan to fix them. The answer to the complains seems to be 'if you think that the fix is needed, go and fix'. My impression is that some set of vocal active developers consider the second consequence unacceptable, myself included. IMO, committing the change, however good the consequence one is, without fixing the consequence two, is inappropriate. And the onus of doing this is on the person doing the change. Yes, it is very interesting is the change actually good WRT fixing migration, since indeed there is serious reduction in the migration amount due to locked callout_reset() being not universally used. It is possible that the bugs are only covered.