kern/109152: [rp] RocketPort panic from device_unbusy()

[Doug Ambrisko]

The following reply was made to PR kern/109152; it has been noted by GNATS.

From: Doug Ambrisko <ambrisko at ambrisko.com>
To: Craig Leres <leres at ee.lbl.gov>
Cc: bug-followup at FreeBSD.org
Subject: Re: kern/109152: [rp] RocketPort panic from device_unbusy()
Date: Wed, 14 Mar 2007 19:55:48 -0800 (PST)

 Craig Leres writes:
 | I was still able to crash Doug Ambrisko's version of the rp driver.
 | I think the problem is that in some cases rp_handle_port() calls
 | rpclose() which calls device_unbusy(). Later rpclose() is called
 | again and we hit the panic.
 | 
 | I looked at the last known good version of the driver I'd used,
 | 1.45.2.2 from 4.10-RELEASE, and found it has code to avoid calling
 | rpclose() twice. I did something similar that seems to work.
 | 
 | Note: The appended diffs are against version 1.67.2.2 of rp.c.
 | 
 | 		Craig
 | 
 | ===================================================================
 | RCS file: RCS/rp.c,v
 | retrieving revision 1.2
 | retrieving revision 1.3
 | diff -c -r1.2 -r1.3
 | *** rp.c	2007/03/08 04:07:10	1.2
 | --- rp.c	2007/03/14 02:23:17	1.3
 | ***************
 | *** 573,578 ****
 | --- 573,579 ----
 |   
 |   static	void	rpbreak(struct tty *, int);
 |   static	void	rpclose(struct tty *tp);
 | + static	void	rphardclose(struct tty *tp);
 |   static	int	rpmodem(struct tty *, int, int);
 |   static	int	rpparam(struct tty *, struct termios *);
 |   static	void	rpstart(struct tty *);
 | ***************
 | *** 697,703 ****
 |   			if((tp->t_state & TS_CARR_ON)) {
 |   				(void)ttyld_modem(tp, 0);
 |   				if(ttyld_modem(tp, 0) == 0) {
 | ! 					rpclose(tp);
 |   				}
 |   			}
 |   		}
 | --- 698,704 ----
 |   			if((tp->t_state & TS_CARR_ON)) {
 |   				(void)ttyld_modem(tp, 0);
 |   				if(ttyld_modem(tp, 0) == 0) {
 | ! 					rphardclose(tp);
 |   				}
 |   			}
 |   		}
 | ***************
 | *** 936,941 ****
 | --- 937,952 ----
 |   rpclose(struct tty *tp)
 |   {
 |   	struct	rp_port	*rp;
 | + 
 | + 	rp = tp->t_sc;
 | + 	rphardclose(tp);
 | + 	device_unbusy(rp->rp_ctlp->dev);
 | + }
 | + 
 | + static void
 | + rphardclose(struct tty *tp)
 | + {
 | + 	struct	rp_port	*rp;
 |   	CHANNEL_t	*cp;
 |   
 |   	rp = tp->t_sc;
 | ***************
 | *** 959,965 ****
 |   	tp->t_actout = FALSE;
 |   	wakeup(&tp->t_actout);
 |   	wakeup(TSA_CARR_ON(tp));
 | - 	device_unbusy(rp->rp_ctlp->dev);
 |   }
 |   
 |   static void
 | --- 970,975 ----
 | 
 
 I wonder if this should be a reference count?  Good find.
 
 Doug A.