Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 23 Sep 2004 16:38:06 +0900
From:      Pyun YongHyeon <yongari@kt-is.co.kr>
To:        dhaigh@gatorzone.com
Cc:        sparc64@FreeBSD.org
Subject:   Re: PR sparc64/71729
Message-ID:  <20040923073806.GA12622@kt-is.co.kr>
In-Reply-To: <20040922083625.GA8910@kt-is.co.kr>
References:  <!~!UENERkVCMDkAAQACAAAAAAAAAAAAAAAAABgAAAAAAAAACQvSeqVn1BGUQgBglOsMr8KAAAAQAAAAcPmXkBll7EW4XKEqja5UUQEAAAAA@nc.rr.com> <20040920104006.GA1905@kt-is.co.kr> <20040922083625.GA8910@kt-is.co.kr>

Next in thread | Previous in thread | Raw E-Mail | Index | Archive | Help

--liOOAslEiF7prFVr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Sep 22, 2004 at 05:36:25PM +0900, To dhaigh@gatorzone.com wrote:
 > On Mon, Sep 20, 2004 at 07:40:06PM +0900, To dhaigh@gatorzone.com wrote:
 >  > On Fri, Sep 17, 2004 at 09:29:41AM -0400, Doug Haigh wrote:
 >  >  > 
 >  >  > I am not sure if the PR got updated, but I wanted to let you know that the
 >  >  > printf in a kernel thread only fails if you are on a SMP machine. It appears
 >  >  > that if you printf on a CPU other than CPU #0, it will panic the kernel.
 >  >  >  
 >  > 
 >  > After reading your mail, I made a small test program which creates
 >  > a kernel thread and ran it on AXe(UP) and U2(SMP).
 >  > Under AXe it worked as expected but it paniced on U2.
 >  > So it seems that there is some issues in OF console.
 >  > Backtrace for the panic is somthing like:
 >  > 
 > 
 > While looking into the ofw_console(4) code I noticed there is
 > possible races there. So I patched the code and ran it. However
 > it didn't fix the problem. Since we can't simply use a mutex in
 > ofw_cons_putc(), there should be other way to workaround this.
 > Have no idea yet.
 > 

Ok, here is patch for ofw_console(4). This patch should fix kernel
panic when a kernel thread tries to write chars via ofw_console(4)
interface.

It seems that the patch works well on my U2/AXe. I'd like to know
whether it works on other Ultrasprcs. Also it would be great
if it doesn't produce negative impacts on PPC too.


Cheers,
Pyun YongHyeon
-- 
Pyun YongHyeon <http://www.kr.freebsd.org/~yongari>;

--liOOAslEiF7prFVr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ofw_console.patch"

--- sys/dev/ofw/ofw_console.c.orig	Wed Aug  4 09:21:19 2004
+++ sys/dev/ofw/ofw_console.c	Thu Sep 23 16:19:29 2004
@@ -47,6 +47,25 @@
 #define	OFWCONS_POLL_HZ	4	/* 50-100 works best on Ultra2 */
 #endif
 #define OFBURSTLEN	128	/* max number of bytes to write in one chunk */
+#define OFWCONS_POLL	1
+#define PCHAR_MAX	128
+
+struct ofw_softc {
+	struct cdev	*ofw_dev;
+	struct tty	*ofw_tty;
+	struct callout	ofw_co;
+	int		polltime;
+	u_int8_t        pchar[PCHAR_MAX];
+	int             phead;
+	int             ptail;
+	int             ccnt;
+	int		ofw_flags;
+	struct mtx	lock;
+};
+static struct ofw_softc ofw_sc;
+
+#define OFWCONS_LOCK(sc)	mtx_lock_spin(&sc->lock)
+#define OFWCONS_UNLOCK(sc)	mtx_unlock_spin(&sc->lock)
 
 static d_open_t		ofw_dev_open;
 static d_close_t	ofw_dev_close;
@@ -59,11 +78,6 @@
 	.d_flags =	D_TTY | D_NEEDGIANT,
 };
 
-static struct tty		*ofw_tp = NULL;
-static int			polltime;
-static struct callout_handle	ofw_timeouthandle
-    = CALLOUT_HANDLE_INITIALIZER(&ofw_timeouthandle);
-
 #if defined(KDB) && defined(ALT_BREAK_TO_DEBUGGER)
 static int			alt_break_state;
 #endif
@@ -71,7 +85,7 @@
 static void	ofw_tty_start(struct tty *);
 static int	ofw_tty_param(struct tty *, struct termios *);
 static void	ofw_tty_stop(struct tty *, int);
-static void	ofw_timeout(void *);
+static void	ofw_pollc(void *);
 
 static cn_probe_t	ofw_cons_probe;
 static cn_init_t	ofw_cons_init;
@@ -87,7 +101,8 @@
 {
 	phandle_t options;
 	char output[32];
-	struct cdev *dev;
+	struct ofw_softc *sc;
+	struct tty *tp;
 
 	if (ofw_consdev.cn_pri != CN_DEAD &&
 	    ofw_consdev.cn_name[0] != '\0') {
@@ -95,9 +110,22 @@
 		    OF_getprop(options, "output-device", output,
 		    sizeof(output)) == -1)
 			return;
-		dev = make_dev(&ofw_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600, "%s",
-		    output);
-		make_dev_alias(dev, "ofwcons");
+		sc = &ofw_sc;
+		bzero(sc, sizeof(*sc));
+		mtx_init(&sc->lock, "ofw_cons mtx", NULL, MTX_SPIN);
+		sc->ofw_dev = make_dev(&ofw_cdevsw, 0, UID_ROOT, GID_WHEEL,
+		    0600, "%s", output);
+		make_dev_alias(sc->ofw_dev, "ofwcons");
+		tp = ttymalloc(NULL);
+		tp->t_sc = sc;
+		sc->ofw_dev->si_drv1 = sc;
+		sc->ofw_dev->si_tty = tp;
+		sc->ofw_tty = tp;
+		tp->t_dev = sc->ofw_dev;
+		tp->t_oproc = ofw_tty_start;
+		tp->t_param = ofw_tty_param;
+		tp->t_stop = ofw_tty_stop;
+		callout_init(&sc->ofw_co, 0);
 	}
 }
 
@@ -109,20 +137,13 @@
 static int
 ofw_dev_open(struct cdev *dev, int flag, int mode, struct thread *td)
 {
+	struct ofw_softc *sc;
 	struct	tty *tp;
-	int	unit;
-	int	error, setuptimeout;
+	int	error;
 
+	sc = dev->si_drv1;
+	tp = dev->si_tty;
 	error = 0;
-	setuptimeout = 0;
-	unit = minor(dev);
-
-	tp = ofw_tp = dev->si_tty = ttymalloc(ofw_tp);
-
-	tp->t_oproc = ofw_tty_start;
-	tp->t_param = ofw_tty_param;
-	tp->t_stop = ofw_tty_stop;
-	tp->t_dev = dev;
 
 	if ((tp->t_state & TS_ISOPEN) == 0) {
 		tp->t_state |= TS_CARR_ON;
@@ -133,21 +154,20 @@
 		tp->t_lflag = TTYDEF_LFLAG;
 		tp->t_ispeed = tp->t_ospeed = TTYDEF_SPEED;
 		ttsetwater(tp);
-
-		setuptimeout = 1;
 	} else if ((tp->t_state & TS_XCLUDE) && suser(td)) {
 		return (EBUSY);
 	}
 
+	if ((error = tty_open(dev, tp)))
+		return (error);
 	error = ttyld_open(tp, dev);
-
-	if (error == 0 && setuptimeout) {
-		polltime = hz / OFWCONS_POLL_HZ;
-		if (polltime < 1) {
-			polltime = 1;
-		}
-
-		ofw_timeouthandle = timeout(ofw_timeout, tp, polltime);
+	if (error == 0 && (sc->ofw_flags & OFWCONS_POLL) == 0) {
+		sc->polltime = hz / OFWCONS_POLL_HZ;
+		if (sc->polltime < 1)
+			sc->polltime = 1;
+		sc->ofw_flags |= OFWCONS_POLL;
+		callout_reset(&sc->ofw_co, sc->polltime,
+		    ofw_pollc, tp);
 	}
 
 	return (error);
@@ -156,18 +176,17 @@
 static int
 ofw_dev_close(struct cdev *dev, int flag, int mode, struct thread *td)
 {
-	int	unit;
+	struct ofw_softc *sc;
 	struct	tty *tp;
 
-	unit = minor(dev);
-	tp = ofw_tp;
+	sc = dev->si_drv1;
+	tp = dev->si_tty;
 
-	if (unit != 0) {
-		return (ENXIO);
-	}
+	if ((tp->t_state & TS_ISOPEN) == 0)
+		return (0);
 
-	/* XXX Should be replaced with callout_stop(9) */
-	untimeout(ofw_timeout, tp, ofw_timeouthandle);
+	callout_stop(&sc->ofw_co);
+	sc->ofw_flags &= ~OFWCONS_POLL;
 	ttyld_close(tp, flag);
 	tty_close(tp);
 
@@ -190,15 +209,18 @@
 	u_char buf[OFBURSTLEN];
 
 
-	if (tp->t_state & (TS_TIMEOUT | TS_BUSY | TS_TTSTOP))
+	if (tp->t_state & (TS_TIMEOUT | TS_BUSY | TS_TTSTOP)) {
+		ttwwakeup(tp);
 		return;
+	}
 
 	tp->t_state |= TS_BUSY;
 	cl = &tp->t_outq;
-	len = q_to_b(cl, buf, OFBURSTLEN);
-	OF_write(stdout, buf, len);
+	while (cl->c_cc) {
+		len = q_to_b(cl, buf, OFBURSTLEN);
+		OF_write(stdout, buf, len);
+	}
 	tp->t_state &= ~TS_BUSY;
-
 	ttwwakeup(tp);
 }
 
@@ -214,20 +236,58 @@
 }
 
 static void
-ofw_timeout(void *v)
+ofw_pollc(void *arg)
 {
-	struct	tty *tp;
-	int 	c;
+	struct ofw_softc *sc;
+	struct tty *tp;
+	int l;
+	char ch;
 
-	tp = (struct tty *)v;
+	tp = (struct tty *)arg;
+	sc = tp->t_sc;
 
-	while ((c = ofw_cons_checkc(NULL)) != -1) {
+	OFWCONS_LOCK(sc);
+#if 0
+	for (; sc->ccnt > 0; sc->ccnt--) {
+		ch = sc->pchar[sc->phead];
+		OF_write(stdout, &ch, 1);
+		if (++sc->phead >= PCHAR_MAX)
+			sc->phead = 0;
+	}
+#else
+	/*
+	 * Since interrupts are disabled during acqusition of MTX_SPIN lock,
+	 * the following code fragments should be optmized and short enough
+	 * in order not to monopolize other system resouces.
+	 */
+	if (sc->ccnt > 0) {
+		if (sc->phead + sc->ccnt <= PCHAR_MAX) {
+			OF_write(stdout, &sc->pchar[sc->phead], sc->ccnt);
+			sc->phead += sc->ccnt;
+			if (sc->phead >= PCHAR_MAX)
+				sc->phead = 0;
+			sc->ccnt = 0;
+		} else {
+			l = PCHAR_MAX - sc->phead;
+			OF_write(stdout, &sc->pchar[sc->phead], l);
+			sc->phead = 0;
+			sc->ccnt -= l;
+			if (sc->ccnt) {
+				OF_write(stdout, &sc->pchar[sc->phead],
+				    sc->ccnt);
+				sc->phead += sc->ccnt;
+				sc->ccnt = 0;
+			}
+		}			
+	}
+#endif
+	OFWCONS_UNLOCK(sc);
+	while(OF_read(stdin, &ch, 1) > 0) {
 		if (tp->t_state & TS_ISOPEN) {
-			ttyld_rint(tp, c);
+			ttyld_rint(tp, ch);
 		}
 	}
-
-	ofw_timeouthandle = timeout(ofw_timeout, tp, polltime);
+	callout_reset(&sc->ofw_co, sc->polltime, ofw_pollc, tp);
 }
 
 static void
@@ -256,10 +316,12 @@
 static void
 ofw_cons_init(struct consdev *cp)
 {
+	struct ofw_softc *sc;
 
 	/* XXX: This is the alias, but that should be good enough */
 	sprintf(cp->cn_name, "ofwcons");
-	cp->cn_tp = ofw_tp;
+	sc = &ofw_sc;
+	cp->cn_tp = sc->ofw_tty;
 }
 
 static int
@@ -269,7 +331,6 @@
 	int l;
 
 	ch = '\0';
-
 	while ((l = OF_read(stdin, &ch, 1)) != 1) {
 		if (l != -2 && l != 0) {
 			return (-1);
@@ -303,13 +364,20 @@
 static void
 ofw_cons_putc(struct consdev *cp, int c)
 {
+	struct ofw_softc *sc;
 	char cbuf;
 
-	if (c == '\n') {
-		cbuf = '\r';
-		OF_write(stdout, &cbuf, 1);
-	}
-
+	sc = &ofw_sc;
 	cbuf = c;
-	OF_write(stdout, &cbuf, 1);
+	if (sc->ofw_tty != NULL) {
+		/* not safe to write yet, queue it */
+		OFWCONS_LOCK(sc);
+		sc->pchar[sc->ptail] = cbuf;
+		if (++sc->ptail >= PCHAR_MAX)
+			sc->ptail = 0;
+		if (++sc->ccnt >= PCHAR_MAX)
+			sc->ccnt = 0;
+		OFWCONS_UNLOCK(sc);
+	} else
+		OF_write(stdout, &cbuf, 1);
 }

--liOOAslEiF7prFVr--



Want to link to this message? Use this URL: <http://docs.FreeBSD.org/cgi/mid.cgi?20040923073806.GA12622>