From owner-svn-src-head@FreeBSD.ORG Tue Jan 20 22:29:39 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6D4184C3; Tue, 20 Jan 2015 22:29:39 +0000 (UTC) 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 1A6DB75D; Tue, 20 Jan 2015 22:29:38 +0000 (UTC) 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 25EAB1FE023; Tue, 20 Jan 2015 23:29:30 +0100 (CET) Message-ID: <54BED6FB.8060401@selasky.org> Date: Tue, 20 Jan 2015 23:30:19 +0100 From: Hans Petter Selasky User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Gleb Smirnoff , Konstantin Belousov Subject: Re: svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys References: <201501151532.t0FFWV2Y037455@svn.freebsd.org> <54BDD9E1.6090505@selasky.org> <20150120075126.GA42409@kib.kiev.ua> <20150120211137.GY15484@FreeBSD.org> In-Reply-To: <20150120211137.GY15484@FreeBSD.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: "svn-src-head@freebsd.org" , Adrian Chadd , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , Jason Wolfe X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 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: Tue, 20 Jan 2015 22:29:39 -0000 On 01/20/15 22:11, Gleb Smirnoff wrote: > On Tue, Jan 20, 2015 at 09:51:26AM +0200, Konstantin Belousov wrote: > K> > Like stated in the manual page, callout_reset_curcpu/on() does not work > K> > with MPSAFE callouts any more! > K> I.e. you 'fixed' some undeterminate bugs in callout migration by not > K> doing migration at all anymore. > K> > K> > > K> > You need to use callout_init_{mtx,rm,rw} and remove the custom locking > K> > inside the callback in the TCP stack to get it working like before! > K> > K> No, you need to do this, if you think that whole callout KPI must be > K> rototiled. It is up to the person who modifies the KPI, to ensure that > K> existing code is not broken. > K> > K> As I understand, currently we are back to the one-cpu callouts. > K> Do other people consider this situation acceptable ? > > I think this isn't acceptable. The commit to a complex subsystem > lacked a review from persons involved in the system before. The > commit to subsystem broke consumers of the subsystem and this > was even done not accidentially, but due to Hans not caring about > it. > > As for me this is enough to request a backout, and let the change > back in only after proper review. > Hi Gleb, Backing out my callout API patch means we will for sure re-introduce an unknown callout spinlock hang, as noted to me by several people. What do you think about that? Maybe "Jason Wolfe" CC'ed can add to 10-stable w/o my patches: int callout_reset_sbt_on(struct callout *c, sbintime_t sbt, sbintime_t precision, void (*ftn)(void *), void *arg, int cpu, int flags) { sbintime_t to_sbt, pr; struct callout_cpu *cc; int cancelled, direct; + cpu = timeout_cpu; /* XXX test code XXX */ cancelled = 0; And see if he observes a callout spinlock hang or not on his test setup. The patch above should force all callouts to the same thread basically. Then we could maybe see if single threading the callouts has anything to do with solving the spinlock hang. The "rewritten" callout API still has all the features and capabilities the old one had, when used as described in "man 9 callout". At the present moment I'm not technically convinced a backout is correct. Gleb: I think we would see far better results with high speed internet links using TCP if we could extend the LRO (large receive offload) code to accumulate more than 64KBytes worth of data per call to the TCP stack instead of complaining about some callouts ending up on the same thread! Actually I have a patch for that. --HPS