Date: Sun, 4 May 2014 20:06:29 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: Andrew Turner <andrew@freebsd.org>, svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r265268 - projects/arm64/sys/arm64/arm64 Message-ID: <20140504193923.Y1622@besplex.bde.org> In-Reply-To: <20140504182755.K1388@besplex.bde.org> References: <201405031527.s43FRUTg019278@svn.freebsd.org> <20140504182755.K1388@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 4 May 2014, Bruce Evans wrote:
> Here is an old version of my printf serialization patches. See the
> accompanying mail in 2009 for more details
>
> % 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
Here is a more up to date versiom, hopefully also with less mailer
mangling. It has more debugging code and a more sophisticated timeout.
@ diff -c2 ./kern/subr_prf.c~ ./kern/subr_prf.c
@ *** ./kern/subr_prf.c~ Thu Mar 8 09:23:44 2007
@ --- ./kern/subr_prf.c Tue Feb 28 01:30:04 2012
@ ***************
@ *** 1,2 ****
@ --- 1,4 ----
@ + int bde_printf_lock_verbose = 0;
@ +
@ /*-
@ * Copyright (c) 1986, 1988, 1991, 1993
@ ***************
@ *** 70,73 ****
@ --- 72,77 ----
@ #include <machine/stdarg.h>
@
@ + void bde_printf_emerg(const char *fmt, ...);
@ +
@ #define TOCONS 0x01
@ #define TOTTY 0x02
Special output routine for when normal output might deadlock.
Implementation not included in this patch, but for the one call
in this patch it can be just cnputs() with the locking in that
removed.
@ ***************
@ *** 112,115 ****
@ --- 116,201 ----
@ &always_console_output, 0, "Always output to console despite TIOCCONS.");
@
@ + static int printf_lock_cpu;
@ +
@ + static void
@ + printf_lock(void)
@ + {
@ + int cpu, timeout;
@ +
@ + /*
@ + * We now have fragility from using locking based on the cpuid
@ + * without pinning the CPU. The timeout handling will recover
@ + * provided some CPU is allowed to run here at all; hopefully
@ + * CPU switches rarely occur. We never had any defense against
@ + * the thread being preempted. Console drivers must still defend
@ + * against deadlocking for their locks -- this layer only
@ + * prevides serialization, only in most cases, and the
@ + * serialization reduces the chance of deadlocks in console
@ + * drivers as a side effect.
@ + */
@ + timeout = 1000;
@ + cpu = PCPU_GET(cpuid);
@ + do {
@ + if (atomic_cmpset_acq_int(&printf_lock_cpu, NOCPU, cpu) != 0)
@ + return; /* Not locked -- allow us in. */
@ + if (atomic_load_acq_int(&printf_lock_cpu) == cpu)
@ + return; /* Deadlock -- allow us in. */
@ + DELAY(1000);
@ + } while (--timeout != 0);
@ + /*
@ + * Blow away current lock owner after the timeout. This case
@ + * is supposed to be very rare now. Except for slow serial
@ + * consoles, the owner of the lock should complete the output
@ + * and release the lock before the timeout expires. Don't
@ + * worry about there being multiple previous lock owners with
@ + * the current one only having been active a small fraction of
@ + * the timeout. It seems difficult to reset the timeout for
@ + * all CPUs, and we would still want to limit the full timeout.
@ + */
@ + atomic_store_rel_int(&printf_lock_cpu, cpu);
@ + }
@ +
@ + static void
@ + printf_unlock(void)
@ + {
@ + /* Release lock iff we hold it. */
@ + atomic_cmpset_int(&printf_lock_cpu, PCPU_GET(cpuid), NOCPU);
@ + }
@ +
@ + #if 0
@ + static int printf_lockcount;
@ + static int printf_locks_blown;
@ +
@ + static void
@ + printf_lock(void)
@ + {
@ + int timeout;
@ +
@ + timeout = 10;
@ + do {
@ + if (atomic_cmpset_acq_int(&printf_lockcount, 0, 1))
@ + return;
@ + DELAY(1000);
@ + } while (--timeout != 0);
@ + if (bde_printf_lock_verbose)
@ + bde_printf_emerg("printf_lock: giving up");
@ + atomic_add_int(&printf_locks_blown, 1);
@ + atomic_add_acq_int(&printf_lockcount, 1);
@ + }
@ +
@ + static void
@ + printf_unlock(void)
@ + {
@ + if (bde_printf_lock_verbose) {
@ + int lc;
@ +
@ + lc = atomic_load_acq_int(&printf_lockcount);
@ + if (lc != 1)
@ + bde_printf_emerg("printf_unlock: %d -> %d", lc, lc - 1);
@ + }
@ + atomic_add_rel_int(&printf_lockcount, -1);
@ + }
@ + #endif
@ +
@ /*
@ * Warn that a system table is full.
Previous version. It has more debugging code and a shorter timeout.
@ ***************
@ *** 198,202 ****
@ --- 284,292 ----
@ 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)
@ ***************
@ *** 239,247 ****
@ --- 329,340 ----
@ pca.tty = NULL;
@ pca.pri = level;
@ + /* XXX why not log always? */
@ pca.flags = log_open ? TOLOG : TOCONS;
@ pca.p_bufr = NULL;
@
@ va_start(ap, fmt);
@ + printf_lock(); /* Even for TOLOG, for serializing msgbuf. */
@ kvprintf(fmt, putchar, &pca, 10, ap);
@ + printf_unlock();
@ va_end(ap);
@
@ ***************
@ *** 312,316 ****
@ --- 405,411 ----
@ #endif
@
@ + printf_lock();
@ retval = kvprintf(fmt, putchar, &pca, 10, ap);
@ + printf_unlock();
@ va_end(ap);
@
@ ***************
@ *** 350,354 ****
@ --- 445,451 ----
@ #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?20140504193923.Y1622>