From owner-svn-src-all@FreeBSD.ORG Wed Jun 1 16:12:49 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8DF99106566B; Wed, 1 Jun 2011 16:12:49 +0000 (UTC) (envelope-from gibbs@FreeBSD.org) Received: from aslan.scsiguy.com (ns1.scsiguy.com [70.89.174.89]) by mx1.freebsd.org (Postfix) with ESMTP id 3E4738FC13; Wed, 1 Jun 2011 16:12:49 +0000 (UTC) Received: from Justins-MacBook-Pro.local (207-225-98-3.dia.static.qwest.net [207.225.98.3]) (authenticated bits=0) by aslan.scsiguy.com (8.14.4/8.14.4) with ESMTP id p51G1IsB097228 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 1 Jun 2011 10:01:18 -0600 (MDT) (envelope-from gibbs@FreeBSD.org) Message-ID: <4DE66249.5070503@FreeBSD.org> Date: Wed, 01 Jun 2011 10:01:13 -0600 From: "Justin T. Gibbs" Organization: The FreeBSD Project User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10 MIME-Version: 1.0 To: Andriy Gapon References: <201105311729.p4VHTwrZ033296@svn.freebsd.org> <4DE5D72A.1020405@FreeBSD.org> In-Reply-To: <4DE5D72A.1020405@FreeBSD.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (aslan.scsiguy.com [70.89.174.89]); Wed, 01 Jun 2011 10:01:19 -0600 (MDT) Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, "Kenneth D. Merry" Subject: Re: svn commit: r222537 - in head/sys: kern sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: gibbs@FreeBSD.org 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, 01 Jun 2011 16:12:49 -0000 On 6/1/11 12:07 AM, Andriy Gapon wrote: > on 31/05/2011 20:29 Kenneth D. Merry said the following: > > + mtx_init(&mbp->msg_lock, "msgbuf", NULL, MTX_SPIN); > > > Sorry that I didn't gather myself together for a review before this change got > actually committed. > Do you see any reason not to make this spinlock recursive? > > I am a little bit worried about "exotic" situations like receiving an NMI in the > middle of printing and wanting to print in the NMI context, or similar things > that penetrate contexts with disabled interrupts - e.g. Machine Check Exception. > Also it's not clear to me if there won't any bigger damage in the situations > like those described above. > > P.S. I have been thinking about fixing the problem in a different fashion, via > reserving portions of dmesg buffer for a whole message using CAS: > http://lists.freebsd.org/pipermail/freebsd-hackers/2010-April/031535.html This is actually where we started (although unfortunately independently since neither Ken nor I saw your previous email). As you say in your message form last April, integrating this type of strategy into the message buffer code is problematic. You need to track the in-progress writes (i.e. holes in the message buffer) so that only fully committed data is printed. I'm sure this can be done with per-cpu data structures and a heavy dose of atomic ops, but after an hour of discussion, we wound up with a ton of complexity and little confidence our design was sound. Even using a spinlock is a lot more complex than it would at first appear. Ken spent almost 2 weeks completing the work and ferreting out all of the corner cases. This bug has been in FreeBSD since SMP was added. While our solution may not be ideal, it is better than what was there before. Committing it seems to have also reignited the discussion on how to implement an even better solution. If there are concrete ideas on how to improve this code further, Ken and I will participate in the work to get them into -current. -- Justin