Subject: Re: bug in RX50 device driver?
To: Kirk Russell <kirk@ba23.org>
From: Johnny Billquist <bqt@Update.UU.SE>
List: port-vax
Date: 01/04/2005 10:44:36
On Mon, 3 Jan 2005, Kirk Russell wrote:

> 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.

Yes.

>  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.

That's actually the correct fix. I suddenly realized what the problem was. 
Argh! Thanks! I just realize that I botched that piece. 
Now we just need someone with the right to check it in there.
Is Ragge listening?

 	Johnny

---
> 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
>

Johnny Billquist                  || "I'm on a bus
                                   ||  on a psychedelic trip
email: bqt@update.uu.se           ||  Reading murder books
pdp is alive!                     ||  tryin' to stay hip" - B. Idol