Skip site navigation (1)Skip section navigation (2)
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>