Date: Mon, 15 Jun 2009 16:17:59 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Sam Leffler <sam@FreeBSD.org>, Ed Schouten <ed@FreeBSD.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r194204 - in head/sys: amd64/conf i386/conf Message-ID: <20090615153040.R1080@besplex.bde.org> In-Reply-To: <20090615114142.B775@besplex.bde.org> References: <200906141801.n5EI1Zti056239@svn.freebsd.org> <4A356A0F.3050800@freebsd.org> <20090615075134.K24645@delplex.bde.org> <4A359AA6.7010101@freebsd.org> <20090615114142.B775@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 15 Jun 2009, Bruce Evans wrote:
> On Sun, 14 Jun 2009, Sam Leffler wrote:
>> I'd love to see someone properly lock printf/console output instead of
>> kludges like PRINTF_BUFR_SIZE.
>
> Almost done (for FreeBSD-~5.2): non-production version with lots of debugging
> cruft:
This version for RELENG_7 seems to work as intended. jhb should look at
its atomic ops.
I only have 1 normal source of interleaved messages -- the messages about
waiting for system processes on shutdown on SMP systems -- and this seems
to be fixed.
The patch applies even without removing PRINTF_BUFR_SIZE or cnputs(), and
works, at least if PRINTF_BUFR_SIZE and the per-cpu buffer are not enabled.
Note that it serializes accesses to the message buffer too. The locks
automatically cover the message buffer if they are applied, and are
applied to all cases that set TOLOG, though this might be too dangerous
for tprintf() since printing to a user's console shouldn't block other
parts of the kernel. printf() to a redirected console is more careful
about this -- it extracts messages from the message buffer and can handle
long delays provided the message buffer doesn't wrap around. This
might be the best method for all cases. Accesses to the message buffer
need to be serialized for this to work properly, and serializing ony
accesses to the message buffer is easier since there are no hardware
delays to worry about -- strict locking with no timeouts will work
provided the halting problem is solvable for kvprintf() to the message
buffer alone.
No attempt is made to buffer messages across printf()s. Neither does
PRINTF_BUFR_SIZE. PRINTF_BUFR_SIZE uses an auto buffer so it obviously
has to flush at the end of each printf. Always extracting from the
message buffer might be the best way to handle this too. Normally you
would extract complete lines, but there must be a timeout to handle
partial lines.
The 1 second timeout here isn't best if the timeout is needed for
avoiding deadlock, as may happen when a printf() is interrupted by
panic(). panic could reasonably() blow open all locks including
printf_lock(), but this is not easy to do without letting other CPUs
proceed and/or complicating the usual case.
Very slow terminals, say 50 bps, need a timeout of say 16 seconds so
that the can print an 80-character line without being interrupted.
% Index: subr_prf.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/kern/subr_prf.c,v
% retrieving revision 1.130.2.1
% diff -u -2 -r1.130.2.1 subr_prf.c
% --- subr_prf.c 21 Jan 2009 00:26:45 -0000 1.130.2.1
% +++ subr_prf.c 15 Jun 2009 05:32:03 -0000
% @@ -112,4 +112,27 @@
% &always_console_output, 0, "Always output to console despite TIOCCONS.");
%
% +static int printf_lockcount;
% +
% +static void
% +printf_lock(void)
% +{
% + int timeout;
% +
% + timeout = 1000;
% + do {
% + if (atomic_cmpset_acq_int(&printf_lockcount, 0, 1))
% + return;
% + DELAY(1000);
% + } while (--timeout != 0);
% + atomic_add_acq_int(&printf_lockcount, 1);
% +}
% +
% +static void
% +printf_unlock(void)
% +{
% +
% + atomic_add_rel_int(&printf_lockcount, -1);
% +}
% +
% /*
% * Warn that a system table is full.
% @@ -198,5 +221,9 @@
% pca.flags = flags;
% va_start(ap, fmt);
% + if (pri != -1)
% + printf_lock(); /* To serialize msgbuf. XXX: long-held lock? */
% kvprintf(fmt, putchar, &pca, 10, ap);
% + if (pri != -1)
% + printf_unlock();
% va_end(ap);
% if (sess != NULL)
% @@ -243,5 +270,7 @@
%
% va_start(ap, fmt);
% + printf_lock(); /* Even for TOLOG, for serializing msgbuf. */
% kvprintf(fmt, putchar, &pca, 10, ap);
% + printf_unlock();
% va_end(ap);
%
% @@ -312,5 +341,7 @@
% #endif
%
% + printf_lock();
% retval = kvprintf(fmt, putchar, &pca, 10, ap);
% + printf_unlock();
% va_end(ap);
%
% @@ -350,5 +381,7 @@
% #endif
%
% + printf_lock();
% retval = kvprintf(fmt, putchar, &pca, 10, ap);
% + printf_unlock();
%
% #ifdef PRINTF_BUFR_SIZE
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090615153040.R1080>