Subject: Re: bug in RX50 device driver?
To: None <port-vax@netbsd.org>
From: Kirk Russell <kirk@ba23.org>
List: port-vax
Date: 01/03/2005 22:39:11
On Mon, 3 Jan 2005, Kirk Russell wrote:

> On Mon, 3 Jan 2005, Johnny Billquist wrote:
>
> > On Mon, 3 Jan 2005, Kirk Russell wrote:
> > > Here are the diffs:
> > > 	http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/mscp/mscp_disk.c.diff?r1=1.30&r2=1.30.10.1&f=h
> > >
> > > So, using version 1.30 is my work-a-round.  Does anybody know what the fix
> > > is?  It appears that these extra calls to disk_busy() and disk_unbusy()
> > > are causing problems.  TIA.
> >
> > disk_busy() and disk_unbusy() would be very weird if they caused any
> > problems...
>
> I noticed that disk_busy() uses ra->ra_disk or rx->ra_disk.  But
> disk_unbusy() only uses ra->ra_disk.  Is that an issue?  I know nothing
> about this, but just noticed that lack of symmetry :-)

In other words, it appears that rriodone() is shared by the ra and rx
devices.  But rriodone() only calls disk_unbusy() with the ra context
but never the rx context -- that cannot be good thing.  But looking
at the rrfillin() routine -- it has code to distinguish between
the ra and rx devices.  I cut and pasted this code, from rrfillin(),
into rriodone().  This added code appears to stop the kernel from
crashing.  Does that help narrow down where the regression is?  TIA.

Here are the diffs to mscp_disk.c verision 1.30.10.1, that I tried:

***************
*** 856,862 ****
  	/* We assume that this is a reasonable drive. ra_strategy should
  	   already have verified it. Thus, no checks here... /bqt */
  	unit = DISKUNIT(bp->b_dev);
! 	ra = ra_cd.cd_devs[unit];
  	disk_unbusy(&ra->ra_disk, bp->b_bcount);

  	biodone(bp);
--- 856,869 ----
  	/* We assume that this is a reasonable drive. ra_strategy should
  	   already have verified it. Thus, no checks here... /bqt */
  	unit = DISKUNIT(bp->b_dev);
! #if NRA
! 	if (major(bp->b_dev) == RAMAJOR)
! 		ra = ra_cd.cd_devs[unit];
! #endif
! #if NRX
! 	if (major(bp->b_dev) != RAMAJOR)
! 		ra = rx_cd.cd_devs[unit];
! #endif
  	disk_unbusy(&ra->ra_disk, bp->b_bcount);

  	biodone(bp);

-- 
Kirk Russell            <kirk@ba23.org>            http://www.ba23.org/
Bridlewood Software Testers Guild                  Ottawa Ontario Canada