Date: Sat, 20 Dec 2008 11:50:05 GMT From: Jaakko Heinonen <jh@saunalahti.fi> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/95979: burncd(8) fails to fixate CDs Message-ID: <200812201150.mBKBo55n044624@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/95979; it has been noted by GNATS.
From: Jaakko Heinonen <jh@saunalahti.fi>
To: artboreb@yahoo.com, m.apitz@oclcpica.org, bug-followup@FreeBSD.org
Cc: Alexander Best <alexbestms@math.uni-muenster.de>
Subject: Re: bin/95979: burncd(8) fails to fixate CDs
Date: Sat, 20 Dec 2008 13:45:11 +0200
On 2008-04-24, Jaakko Heinonen wrote:
> The current default is 300 seconds for single speed write and the
> patch increases it to 420 seconds. (The time is divided by burning
> speed.) AFAIK cdrecord and wodim use also 420 seconds.
Alexander Best reported that event 420 seconds is enough for his drive.
I investigated the issue more and noticed that acd_test_ready() can
really never return EBUSY which is expected in acd_fixate(). This causes
acd_fixate() always to use method which just sleeps the expected
fixation time. As we have seen this method is not very accurate.
In sys/dev/ata/ata-queue.c (r186322) there's this piece of code:
397 if ((request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
398 request->u.atapi.sense.key & ATA_SENSE_KEY_MASK :
399 request->error))
400 request->result = EIO;
The problem is that this code overwrites the old request->result value
which might have been EBUSY. This is the reason for acd_test_ready() not
reporting correctly the drive state. Here's a patch which changes it to
set request->result only if it's not already set.
--- patch begins here ---
Index: sys/dev/ata/ata-queue.c
===================================================================
--- sys/dev/ata/ata-queue.c (revision 186213)
+++ sys/dev/ata/ata-queue.c (working copy)
@@ -434,7 +434,8 @@ ata_completed(void *context, int dummy)
printf("\n");
}
- if ((request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
+ if (!request->result &&
+ (request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
request->u.atapi.sense.key & ATA_SENSE_KEY_MASK :
request->error))
request->result = EIO;
--- patch ends here ---
This alone has been confirmed to fix the error with several drives:
TSSTcorpCD/DVDW SH-S182D/SB02
SAMSUNG CD-R/RW SW-216B BS05 20010727/BS05
HL-DT-ST GCE-8481B/1.00 (LG)
HL-DT-ST DVDRAM GSA-H10N/JL10 (LG) (Tested by: Alexander Best)
I still think that it makes sense to also increase the timeout value.
Here is a combined patch which also increases the timeout to 480
seconds:
--- patch begins here ---
Index: sys/dev/ata/atapi-cd.c
===================================================================
--- sys/dev/ata/atapi-cd.c (revision 186354)
+++ sys/dev/ata/atapi-cd.c (working copy)
@@ -1067,7 +1067,7 @@ acd_fixate(device_t dev, int multisessio
struct acd_softc *cdp = device_get_ivars(dev);
int8_t ccb[16] = { ATAPI_CLOSE_TRACK, 0x01, 0x02, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0 };
- int timeout = 5*60*2;
+ int timeout = 8*60*2;
int error, dummy;
struct write_param param;
Index: sys/dev/ata/ata-queue.c
===================================================================
--- sys/dev/ata/ata-queue.c (revision 186354)
+++ sys/dev/ata/ata-queue.c (working copy)
@@ -434,7 +434,8 @@ ata_completed(void *context, int dummy)
printf("\n");
}
- if ((request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
+ if (!request->result &&
+ (request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
request->u.atapi.sense.key & ATA_SENSE_KEY_MASK :
request->error))
request->result = EIO;
--- patch ends here ---
--
Jaakko
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200812201150.mBKBo55n044624>