svn commit: r222537 - in head/sys: kern sys

Kenneth D. Merry ken at FreeBSD.org
Thu Jun 2 21:36:13 UTC 2011


On Wed, Jun 01, 2011 at 20:07:31 +1000, Bruce Evans wrote:
> On Wed, 1 Jun 2011, 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 see reasons why it must not exist.  The message buffer code cannot use
> any normal locking, and was carefully written to avoid doing so.
> 
> >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
> 
> Something like this might work.
> 
> PRINTF_BUFR size should not exist either, especially now that it is
> much more complicated and broken.  Here is some of my old mail about
> adding (necessarily not normal) locking to to printf.  No one replied,
> so I never finished this :-(.

That particular solution doesn't lock the normal kernel printf path, and so
won't fix what we're trying to fix.  (We've got lots of disks in each
system, and when there are various SES events and disks arriving and
departing, there are multiple kernel contexts doing printfs simultaneously,
and that results in scrambled dmesg output.)

I think the goals should be:

 - console output, syslog output, and dmesg output are not scrambled.
   (i.e. individual kernel printfs make it out atomically, or at least
   with a certain granularity, like PRINTF_BUFR_SIZE.)

 - Can be called by any kernel routine (i.e. doesn't sleep)

 - Offers a string at a time interface.

 - Does the right thing for log messages (priority, etc.)

It looks like we should add "does not use normal locking" to the list as
well.

As Justin mentioned, we started off down the path of using atomic
operations, and then figured out that wouldn't fully work.  Then we
discussed doing a per-CPU message buffer, with each message tagged with a
sequence number, and having the reader re-serialize the messages in the
right order.  The complexity started to get large enough that we decided
that using a spin lock would be a much easier approach.

cnputs() already uses a spinlock, so we're no worse off than before from a
locking standpoint.

We could perhaps make the message buffer mutex recursive and set the
MTX_NOWITNESS flag as well, that might make things a little better from a
side effect standpoint.

If we can come up with a solution that meets the above goals, Justin and I
would be happy to help implement it.

Ken
-- 
Kenneth Merry
ken at FreeBSD.ORG


More information about the svn-src-head mailing list