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

_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to