Date: Wed, 01 Jun 2011 10:01:13 -0600 From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: Andriy Gapon <avg@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, "Kenneth D. Merry" <ken@FreeBSD.org> Subject: Re: svn commit: r222537 - in head/sys: kern sys Message-ID: <4DE66249.5070503@FreeBSD.org> In-Reply-To: <4DE5D72A.1020405@FreeBSD.org> References: <201105311729.p4VHTwrZ033296@svn.freebsd.org> <4DE5D72A.1020405@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DE66249.5070503>