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