We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
It can easily be reproduced by:
1. Inserting an audio CD
2. Running the following command as soon as the blue CD/DVD/BD drive LED
stops blinking and is lit continuously:
cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
insertion, or it will fail.
On 2.6.24 and older, it just works immediately.
It does not matter whether
http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
is applied or not.
We haven't bisected it yet.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB
Geert Uytterhoeven wrote:
> We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
>
> It can easily be reproduced by:
>
> 1. Inserting an audio CD
> 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> stops blinking and is lit continuously:
>
> cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
>
> On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> insertion, or it will fail.
> On 2.6.24 and older, it just works immediately.
>
> It does not matter whether
> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> is applied or not.
>
> We haven't bisected it yet.
>
> With kind regards,
>
> Geert Uytterhoeven
> Software Architect
>
> Sony Techsoft Centre
> The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
>
> Phone: +32 (0)2 700 8453
> Fax: +32 (0)2 700 8622
> E-mail: [email protected]
> Internet: http://www.sony-europe.com/
>
> Sony Technology and Software Centre Europe
> A division of Sony Service Centre (Europe) N.V.
> Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
> VAT BE 0413.825.160 · RPR Brussels
> Fortis 293-0376800-10 GEBA-BE-BB
James hi.
It looks like the same problem as before. Any BLOCK_PC command will
eat the UNIT_ATTENTION notification. Which is what I was afraid of.
I was thinking of something like below.
Totally untested, never even booted. It is just to demonstrate the concept.
Please tell me if it is at all desirable and I'll spend some time to see
if it runs, and test it.
Boaz
---
>From 8cd166b76100a2830f0f973df866f5509398f6cc Mon Sep 17 00:00:00 2001
From: Boaz Harrosh <[email protected]>
Date: Tue, 10 Jun 2008 20:09:43 +0300
Subject: [PATCH] scsi: Proccess UNIT_ATTENTION on any type command
Let scsi_check_sense() report UNIT_ATTENTION media
changes for any type of command. Currently only FS
commands are reported.
Signed-off-by: Boaz Harrosh <[email protected]>
---
drivers/scsi/scsi_error.c | 4 ++++
drivers/scsi/scsi_lib.c | 3 +--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 006a959..7039d11 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -370,6 +370,10 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
if (scmd->device->allow_restart &&
(sshdr.asc == 0x04) && (sshdr.ascq == 0x02))
return FAILED;
+
+ /* Detected disc change.*/
+ if (scmd->device->removable)
+ scmd->device->changed = 1;
return SUCCESS;
/* these three are not supported */
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 033c58a..efd3e09 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -903,11 +903,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
if (sense_valid && !sense_deferred) {
switch (sshdr.sense_key) {
case UNIT_ATTENTION:
- if (cmd->device->removable) {
+ if (cmd->device->changed) {
/* Detected disc change. Set a bit
* and quietly refuse further access.
*/
- cmd->device->changed = 1;
scsi_end_request(cmd, -EIO, this_count, 1);
return;
} else {
--
1.5.6.rc1.5.gadf6
On Sun, 2008-06-15 at 15:46 +0300, Boaz Harrosh wrote:
> Geert Uytterhoeven wrote:
> > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> >
> > It can easily be reproduced by:
> >
> > 1. Inserting an audio CD
> > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > stops blinking and is lit continuously:
> >
> > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> >
> > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > insertion, or it will fail.
> > On 2.6.24 and older, it just works immediately.
> >
> > It does not matter whether
> > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > is applied or not.
> >
> > We haven't bisected it yet.
> >
> > With kind regards,
> >
> > Geert Uytterhoeven
> > Software Architect
> >
> > Sony Techsoft Centre
> > The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
> >
> > Phone: +32 (0)2 700 8453
> > Fax: +32 (0)2 700 8622
> > E-mail: [email protected]
> > Internet: http://www.sony-europe.com/
> >
> > Sony Technology and Software Centre Europe
> > A division of Sony Service Centre (Europe) N.V.
> > Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
> > VAT BE 0413.825.160 · RPR Brussels
> > Fortis 293-0376800-10 GEBA-BE-BB
>
> James hi.
>
> It looks like the same problem as before. Any BLOCK_PC command will
> eat the UNIT_ATTENTION notification. Which is what I was afraid of.
> I was thinking of something like below.
It's a bit unlikely to be the cause of a regression, since the code has
behaved this way by design since at least 2.4. The problem we're
looking for appeared in 2.6.25+
Also, the taxonomy of the regression: errors returned within 10s of
inserting the CD sounds very much like an ignored NOT_READY.
> Totally untested, never even booted. It is just to demonstrate the concept.
> Please tell me if it is at all desirable and I'll spend some time to see
> if it runs, and test it.
Really, no ... not until there's a proveable problem caused by it. Even
then, not like this: the way you've coded it interferes with the cc_ua
processing code. It would have to go in scsi_decide_disposition().
James
On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
>
> It can easily be reproduced by:
>
> 1. Inserting an audio CD
> 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> stops blinking and is lit continuously:
>
> cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
>
> On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> insertion, or it will fail.
> On 2.6.24 and older, it just works immediately.
>
> It does not matter whether
> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> is applied or not.
>
> We haven't bisected it yet.
There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
so I'd bet on the previous culprits.
This time, the taxonomy looks like NOT_READY isn't being waited for
properly. I'd still tend to blame
210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
suspect this to be the problem line:
if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
return CDS_DISC_OK;
Previously the code would have returned CDS_NO_DISC for the not ready
case. The CD would have tried to close the door but (and this is the
key) it would have waited for the door to close before trying again.
James
Hi James,
On Sun, 15 Jun 2008, James Bottomley wrote:
> On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> >
> > It can easily be reproduced by:
> >
> > 1. Inserting an audio CD
> > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > stops blinking and is lit continuously:
> >
> > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> >
> > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > insertion, or it will fail.
> > On 2.6.24 and older, it just works immediately.
> >
> > It does not matter whether
> > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > is applied or not.
> >
> > We haven't bisected it yet.
>
> There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> so I'd bet on the previous culprits.
>
> This time, the taxonomy looks like NOT_READY isn't being waited for
> properly. I'd still tend to blame
> 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
Reverting that commit doesn't fix the problem.
> suspect this to be the problem line:
>
> if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> return CDS_DISC_OK;
>
> Previously the code would have returned CDS_NO_DISC for the not ready
> case. The CD would have tried to close the door but (and this is the
> key) it would have waited for the door to close before trying again.
I tried the patch below.
Now the kernel spits out messages every 2 seconds:
| G: CDS_NO_DISC
| G: CDS_NO_DISC
Insert CD-DA media
Reading from CD using cdparanioa fails
| F: CDS_DISC_OK
| F: CDS_DISC_OK
| F: CDS_DISC_OK
| F: CDS_DISC_OK
| F: CDS_DISC_OK
Reading from CD starts to work here
| B: CDS_DISC_OK
| B: CDS_DISC_OK
I tried changing the return value in the `F' case to CDS_NO_DISC, but that
doesn't make a difference.
--- a/drivers/scsi/sr_ioctl.c 2008-06-16 16:34:01.000000000 +0200
+++ b/drivers/scsi/sr_ioctl.c 2008-06-16 16:59:28.000000000 +0200
@@ -304,25 +304,34 @@ int sr_drive_status(struct cdrom_device_
if (CDSL_CURRENT != slot) {
/* we have no changer support */
+printk("A: -EINVAL\n");
return -EINVAL;
}
- if (0 == sr_test_unit_ready(cd->device, &sshdr))
+ if (0 == sr_test_unit_ready(cd->device, &sshdr)) {
+printk("B: CDS_DISC_OK\n");
return CDS_DISC_OK;
+ }
if (!cdrom_get_media_event(cdi, &med)) {
- if (med.media_present)
+ if (med.media_present) {
+printk("C: CDS_DISC_OK\n");
return CDS_DISC_OK;
- else if (med.door_open)
+ } else if (med.door_open) {
+printk("D: CDS_TRAY_OPEN\n");
return CDS_TRAY_OPEN;
- else
+ } else {
+printk("E: CDS_NO_DISC\n");
return CDS_NO_DISC;
+ }
}
/*
* 0x04 is format in progress .. but there must be a disc present!
*/
- if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
+ if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04) {
+printk("F: CDS_DISC_OK\n");
return CDS_DISC_OK;
+ }
/*
* If not using Mt Fuji extended media tray reports,
@@ -331,11 +340,15 @@ int sr_drive_status(struct cdrom_device_
*/
if (scsi_sense_valid(&sshdr) &&
/* 0x3a is medium not present */
- sshdr.asc == 0x3a)
+ sshdr.asc == 0x3a) {
+printk("G: CDS_NO_DISC\n");
return CDS_NO_DISC;
- else
+ } else {
+printk("H: CDS_TRAY_OPEN\n");
return CDS_TRAY_OPEN;
+ }
+printk("I: CDS_DRIVE_NOT_READY\n");
return CDS_DRIVE_NOT_READY;
}
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB
On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> Hi James,
>
> On Sun, 15 Jun 2008, James Bottomley wrote:
> > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > >
> > > It can easily be reproduced by:
> > >
> > > 1. Inserting an audio CD
> > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > stops blinking and is lit continuously:
> > >
> > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > >
> > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > insertion, or it will fail.
> > > On 2.6.24 and older, it just works immediately.
> > >
> > > It does not matter whether
> > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > is applied or not.
> > >
> > > We haven't bisected it yet.
> >
> > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > so I'd bet on the previous culprits.
> >
> > This time, the taxonomy looks like NOT_READY isn't being waited for
> > properly. I'd still tend to blame
> > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
>
> Reverting that commit doesn't fix the problem.
That's a bit of a problem. Particularly as:
> > suspect this to be the problem line:
> >
> > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > return CDS_DISC_OK;
> >
> > Previously the code would have returned CDS_NO_DISC for the not ready
> > case. The CD would have tried to close the door but (and this is the
> > key) it would have waited for the door to close before trying again.
>
> I tried the patch below.
>
> Now the kernel spits out messages every 2 seconds:
>
> | G: CDS_NO_DISC
> | G: CDS_NO_DISC
>
> Insert CD-DA media
>
> Reading from CD using cdparanioa fails
>
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
That's in the two lines I fingered as the source of the problem ...
however, if reversing the apparently bad commit (that introduces those
lines) doesn't work there's something else wrong.
Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
if that doesn't work, it's going to take a bit of bisection, I'm
afraid ...
Thanks,
James
> Reading from CD starts to work here
>
> | B: CDS_DISC_OK
> | B: CDS_DISC_OK
>
> I tried changing the return value in the `F' case to CDS_NO_DISC, but that
> doesn't make a difference.
>
> --- a/drivers/scsi/sr_ioctl.c 2008-06-16 16:34:01.000000000 +0200
> +++ b/drivers/scsi/sr_ioctl.c 2008-06-16 16:59:28.000000000 +0200
> @@ -304,25 +304,34 @@ int sr_drive_status(struct cdrom_device_
>
> if (CDSL_CURRENT != slot) {
> /* we have no changer support */
> +printk("A: -EINVAL\n");
> return -EINVAL;
> }
> - if (0 == sr_test_unit_ready(cd->device, &sshdr))
> + if (0 == sr_test_unit_ready(cd->device, &sshdr)) {
> +printk("B: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> + }
>
> if (!cdrom_get_media_event(cdi, &med)) {
> - if (med.media_present)
> + if (med.media_present) {
> +printk("C: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> - else if (med.door_open)
> + } else if (med.door_open) {
> +printk("D: CDS_TRAY_OPEN\n");
> return CDS_TRAY_OPEN;
> - else
> + } else {
> +printk("E: CDS_NO_DISC\n");
> return CDS_NO_DISC;
> + }
> }
>
> /*
> * 0x04 is format in progress .. but there must be a disc present!
> */
> - if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> + if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04) {
> +printk("F: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> + }
>
> /*
> * If not using Mt Fuji extended media tray reports,
> @@ -331,11 +340,15 @@ int sr_drive_status(struct cdrom_device_
> */
> if (scsi_sense_valid(&sshdr) &&
> /* 0x3a is medium not present */
> - sshdr.asc == 0x3a)
> + sshdr.asc == 0x3a) {
> +printk("G: CDS_NO_DISC\n");
> return CDS_NO_DISC;
> - else
> + } else {
> +printk("H: CDS_TRAY_OPEN\n");
> return CDS_TRAY_OPEN;
> + }
>
> +printk("I: CDS_DRIVE_NOT_READY\n");
> return CDS_DRIVE_NOT_READY;
> }
>
>
> With kind regards,
>
> Geert Uytterhoeven
> Software Architect
>
> Sony Techsoft Centre
> The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
>
> Phone: +32 (0)2 700 8453
> Fax: +32 (0)2 700 8622
> E-mail: [email protected]
> Internet: http://www.sony-europe.com/
>
> Sony Technology and Software Centre Europe
> A division of Sony Service Centre (Europe) N.V.
> Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
> VAT BE 0413.825.160 · RPR Brussels
> Fortis 293-0376800-10 GEBA-BE-BB
On Mon, 16 Jun 2008, James Bottomley wrote:
> On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> > On Sun, 15 Jun 2008, James Bottomley wrote:
> > > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > > >
> > > > It can easily be reproduced by:
> > > >
> > > > 1. Inserting an audio CD
> > > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > > stops blinking and is lit continuously:
> > > >
> > > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > > >
> > > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > > insertion, or it will fail.
> > > > On 2.6.24 and older, it just works immediately.
> > > >
> > > > It does not matter whether
> > > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > > is applied or not.
> > > >
> > > > We haven't bisected it yet.
> > >
> > > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > > so I'd bet on the previous culprits.
> > >
> > > This time, the taxonomy looks like NOT_READY isn't being waited for
> > > properly. I'd still tend to blame
> > > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> >
> > Reverting that commit doesn't fix the problem.
>
> That's a bit of a problem. Particularly as:
>
> > > suspect this to be the problem line:
> > >
> > > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > > return CDS_DISC_OK;
> > >
> > > Previously the code would have returned CDS_NO_DISC for the not ready
> > > case. The CD would have tried to close the door but (and this is the
> > > key) it would have waited for the door to close before trying again.
> >
> > I tried the patch below.
> >
> > Now the kernel spits out messages every 2 seconds:
> >
> > | G: CDS_NO_DISC
> > | G: CDS_NO_DISC
> >
> > Insert CD-DA media
> >
> > Reading from CD using cdparanioa fails
> >
> > | F: CDS_DISC_OK
> > | F: CDS_DISC_OK
> > | F: CDS_DISC_OK
> > | F: CDS_DISC_OK
> > | F: CDS_DISC_OK
>
> That's in the two lines I fingered as the source of the problem ...
> however, if reversing the apparently bad commit (that introduces those
> lines) doesn't work there's something else wrong.
>
> Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
> if that doesn't work, it's going to take a bit of bisection, I'm
> afraid ...
Doesn't make any difference.
git-bisect taught me it was introduced by
commit 38582a62ecd337de4212004c7d4844899dc57890
Author: James Bottomley <[email protected]>
Date: Wed Feb 6 13:01:58 2008 -0600
[SCSI] sr: fix test unit ready responses
Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
the scsi_test_unit_ready() function. Unfortunately, this has the
wrong characteristic of eating NOT_READY returns which sr.c relies on
for tray status.
Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB
James Bottomley wrote:
> On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
>> We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
>>
>> It can easily be reproduced by:
>>
>> 1. Inserting an audio CD
>> 2. Running the following command as soon as the blue CD/DVD/BD drive LED
>> stops blinking and is lit continuously:
>>
>> cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
>>
>> On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
>> insertion, or it will fail.
>> On 2.6.24 and older, it just works immediately.
>>
>> It does not matter whether
>> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
>> is applied or not.
>>
>> We haven't bisected it yet.
>
> There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> so I'd bet on the previous culprits.
>
> This time, the taxonomy looks like NOT_READY isn't being waited for
> properly. I'd still tend to blame
> 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> suspect this to be the problem line:
>
Looking at the last part of that commit, what code path could ever lead
to reaching that last return statement and returning
CDS_DRIVE_NOT_READY? Maybe I'm just being dense but it looks unreachable
to me:
+ /*
+ * If not using Mt Fuji extended media tray reports,
+ * just return TRAY_OPEN since ATAPI doesn't provide
+ * any other way to detect this...
+ */
+ if (scsi_sense_valid(&sshdr) &&
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a)
+ return CDS_NO_DISC;
+ else
+ return CDS_TRAY_OPEN;
+
+ return CDS_DRIVE_NOT_READY;
}
On Tue, 2008-06-17 at 18:56 -0400, Chuck Ebbert wrote:
> James Bottomley wrote:
> > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> >> We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> >>
> >> It can easily be reproduced by:
> >>
> >> 1. Inserting an audio CD
> >> 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> >> stops blinking and is lit continuously:
> >>
> >> cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> >>
> >> On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> >> insertion, or it will fail.
> >> On 2.6.24 and older, it just works immediately.
> >>
> >> It does not matter whether
> >> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> >> is applied or not.
> >>
> >> We haven't bisected it yet.
> >
> > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > so I'd bet on the previous culprits.
> >
> > This time, the taxonomy looks like NOT_READY isn't being waited for
> > properly. I'd still tend to blame
> > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> > suspect this to be the problem line:
> >
>
> Looking at the last part of that commit, what code path could ever lead
> to reaching that last return statement and returning
> CDS_DRIVE_NOT_READY? Maybe I'm just being dense but it looks unreachable
> to me:
It is ... unfortunately, it's also been verified not to be the problem
(that's what I thought at first too) ... it looks like drivers/cdrom has
no real use for CDS_DRIVE_NOT_READY.
James
On Tue, 2008-06-17 at 15:31 +0200, Geert Uytterhoeven wrote:
> On Mon, 16 Jun 2008, James Bottomley wrote:
> > On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> > > On Sun, 15 Jun 2008, James Bottomley wrote:
> > > > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > > > >
> > > > > It can easily be reproduced by:
> > > > >
> > > > > 1. Inserting an audio CD
> > > > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > > > stops blinking and is lit continuously:
> > > > >
> > > > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > > > >
> > > > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > > > insertion, or it will fail.
> > > > > On 2.6.24 and older, it just works immediately.
> > > > >
> > > > > It does not matter whether
> > > > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > > > is applied or not.
> > > > >
> > > > > We haven't bisected it yet.
> > > >
> > > > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > > > so I'd bet on the previous culprits.
> > > >
> > > > This time, the taxonomy looks like NOT_READY isn't being waited for
> > > > properly. I'd still tend to blame
> > > > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> > >
> > > Reverting that commit doesn't fix the problem.
> >
> > That's a bit of a problem. Particularly as:
> >
> > > > suspect this to be the problem line:
> > > >
> > > > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > > > return CDS_DISC_OK;
> > > >
> > > > Previously the code would have returned CDS_NO_DISC for the not ready
> > > > case. The CD would have tried to close the door but (and this is the
> > > > key) it would have waited for the door to close before trying again.
> > >
> > > I tried the patch below.
> > >
> > > Now the kernel spits out messages every 2 seconds:
> > >
> > > | G: CDS_NO_DISC
> > > | G: CDS_NO_DISC
> > >
> > > Insert CD-DA media
> > >
> > > Reading from CD using cdparanioa fails
> > >
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> >
> > That's in the two lines I fingered as the source of the problem ...
> > however, if reversing the apparently bad commit (that introduces those
> > lines) doesn't work there's something else wrong.
> >
> > Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
> > if that doesn't work, it's going to take a bit of bisection, I'm
> > afraid ...
>
> Doesn't make any difference.
>
> git-bisect taught me it was introduced by
>
> commit 38582a62ecd337de4212004c7d4844899dc57890
> Author: James Bottomley <[email protected]>
> Date: Wed Feb 6 13:01:58 2008 -0600
>
> [SCSI] sr: fix test unit ready responses
>
> Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> the scsi_test_unit_ready() function. Unfortunately, this has the
> wrong characteristic of eating NOT_READY returns which sr.c relies on
> for tray status.
>
> Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
OK, that basically just replaced scsi_test_unit_ready with
sr_test_unit_ready. The only difference I can see is that
scsi_test_unit_ready treats NOT_READY as a media change event and the
new code doesn't. This seems slightly wrong: NOT_READY can indeed mean
I have no medium, which is obviously what it's looking for. However, it
can also mean there's another command in progress. If this is some type
of write or format, you don't necessarily want the cd treating it as a
disk change.
However, before we get into serious debugging, could you try this patch
(applies over the previous fix for the capacity problem)? I think it
should restore the old not ready == medium change behaviour.
Thanks,
James
---
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c82df8b..a9acbc8 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -179,7 +179,8 @@ int sr_test_unit_ready(struct scsi_device *sdev, struct scsi_sense_hdr *sshdr)
0, sshdr, SR_TIMEOUT,
retries--);
if (scsi_sense_valid(sshdr) &&
- sshdr->sense_key == UNIT_ATTENTION)
+ (sshdr->sense_key == UNIT_ATTENTION ||
+ sshdr->sense_key == NOT_READY))
sdev->changed = 1;
} while (retries > 0 &&
On Wed, 18 Jun 2008, James Bottomley wrote:
> On Tue, 2008-06-17 at 15:31 +0200, Geert Uytterhoeven wrote:
> > On Mon, 16 Jun 2008, James Bottomley wrote:
> > > On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> > > > On Sun, 15 Jun 2008, James Bottomley wrote:
> > > > > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > > > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > > > > >
> > > > > > It can easily be reproduced by:
> > > > > >
> > > > > > 1. Inserting an audio CD
> > > > > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > > > > stops blinking and is lit continuously:
> > > > > >
> > > > > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > > > > >
> > > > > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > > > > insertion, or it will fail.
> > > > > > On 2.6.24 and older, it just works immediately.
> > > > > >
> > > > > > It does not matter whether
> > > > > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > > > > is applied or not.
> > > > > >
> > > > > > We haven't bisected it yet.
> > > > >
> > > > > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > > > > so I'd bet on the previous culprits.
> > > > >
> > > > > This time, the taxonomy looks like NOT_READY isn't being waited for
> > > > > properly. I'd still tend to blame
> > > > > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> > > >
> > > > Reverting that commit doesn't fix the problem.
> > >
> > > That's a bit of a problem. Particularly as:
> > >
> > > > > suspect this to be the problem line:
> > > > >
> > > > > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > > > > return CDS_DISC_OK;
> > > > >
> > > > > Previously the code would have returned CDS_NO_DISC for the not ready
> > > > > case. The CD would have tried to close the door but (and this is the
> > > > > key) it would have waited for the door to close before trying again.
> > > >
> > > > I tried the patch below.
> > > >
> > > > Now the kernel spits out messages every 2 seconds:
> > > >
> > > > | G: CDS_NO_DISC
> > > > | G: CDS_NO_DISC
> > > >
> > > > Insert CD-DA media
> > > >
> > > > Reading from CD using cdparanioa fails
> > > >
> > > > | F: CDS_DISC_OK
> > > > | F: CDS_DISC_OK
> > > > | F: CDS_DISC_OK
> > > > | F: CDS_DISC_OK
> > > > | F: CDS_DISC_OK
> > >
> > > That's in the two lines I fingered as the source of the problem ...
> > > however, if reversing the apparently bad commit (that introduces those
> > > lines) doesn't work there's something else wrong.
> > >
> > > Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
> > > if that doesn't work, it's going to take a bit of bisection, I'm
> > > afraid ...
> >
> > Doesn't make any difference.
> >
> > git-bisect taught me it was introduced by
> >
> > commit 38582a62ecd337de4212004c7d4844899dc57890
> > Author: James Bottomley <[email protected]>
> > Date: Wed Feb 6 13:01:58 2008 -0600
> >
> > [SCSI] sr: fix test unit ready responses
> >
> > Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> > the scsi_test_unit_ready() function. Unfortunately, this has the
> > wrong characteristic of eating NOT_READY returns which sr.c relies on
> > for tray status.
> >
> > Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
>
> OK, that basically just replaced scsi_test_unit_ready with
> sr_test_unit_ready. The only difference I can see is that
> scsi_test_unit_ready treats NOT_READY as a media change event and the
> new code doesn't. This seems slightly wrong: NOT_READY can indeed mean
> I have no medium, which is obviously what it's looking for. However, it
> can also mean there's another command in progress. If this is some type
> of write or format, you don't necessarily want the cd treating it as a
> disk change.
>
> However, before we get into serious debugging, could you try this patch
> (applies over the previous fix for the capacity problem)? I think it
> should restore the old not ready == medium change behaviour.
>
> Thanks,
>
> James
>
> ---
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index c82df8b..a9acbc8 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -179,7 +179,8 @@ int sr_test_unit_ready(struct scsi_device *sdev, struct scsi_sense_hdr *sshdr)
> 0, sshdr, SR_TIMEOUT,
> retries--);
> if (scsi_sense_valid(sshdr) &&
> - sshdr->sense_key == UNIT_ATTENTION)
> + (sshdr->sense_key == UNIT_ATTENTION ||
> + sshdr->sense_key == NOT_READY))
> sdev->changed = 1;
>
> } while (retries > 0 &&
Thanks, but unfortunately it doesn't help.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB
On Tue, 2008-06-17 at 15:31 +0200, Geert Uytterhoeven wrote:
> On Mon, 16 Jun 2008, James Bottomley wrote:
> > On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> > > On Sun, 15 Jun 2008, James Bottomley wrote:
> > > > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > > > >
> > > > > It can easily be reproduced by:
> > > > >
> > > > > 1. Inserting an audio CD
> > > > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > > > stops blinking and is lit continuously:
> > > > >
> > > > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > > > >
> > > > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > > > insertion, or it will fail.
> > > > > On 2.6.24 and older, it just works immediately.
> > > > >
> > > > > It does not matter whether
> > > > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > > > is applied or not.
> > > > >
> > > > > We haven't bisected it yet.
> > > >
> > > > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > > > so I'd bet on the previous culprits.
> > > >
> > > > This time, the taxonomy looks like NOT_READY isn't being waited for
> > > > properly. I'd still tend to blame
> > > > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
> > >
> > > Reverting that commit doesn't fix the problem.
> >
> > That's a bit of a problem. Particularly as:
> >
> > > > suspect this to be the problem line:
> > > >
> > > > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > > > return CDS_DISC_OK;
> > > >
> > > > Previously the code would have returned CDS_NO_DISC for the not ready
> > > > case. The CD would have tried to close the door but (and this is the
> > > > key) it would have waited for the door to close before trying again.
> > >
> > > I tried the patch below.
> > >
> > > Now the kernel spits out messages every 2 seconds:
> > >
> > > | G: CDS_NO_DISC
> > > | G: CDS_NO_DISC
> > >
> > > Insert CD-DA media
> > >
> > > Reading from CD using cdparanioa fails
> > >
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> > > | F: CDS_DISC_OK
> >
> > That's in the two lines I fingered as the source of the problem ...
> > however, if reversing the apparently bad commit (that introduces those
> > lines) doesn't work there's something else wrong.
> >
> > Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
> > if that doesn't work, it's going to take a bit of bisection, I'm
> > afraid ...
>
> Doesn't make any difference.
>
> git-bisect taught me it was introduced by
>
> commit 38582a62ecd337de4212004c7d4844899dc57890
> Author: James Bottomley <[email protected]>
> Date: Wed Feb 6 13:01:58 2008 -0600
>
> [SCSI] sr: fix test unit ready responses
>
> Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> the scsi_test_unit_ready() function. Unfortunately, this has the
> wrong characteristic of eating NOT_READY returns which sr.c relies on
> for tray status.
>
> Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
OK, I thought I had a test case for this, but when I revert this commit
on git head (and fix up the one reject which just leaves the sr_
function in place) I still produce the same behaviour.
What I'm trying is
sg_start -i -l <cdrom>
to close the tray followed by your cdparanoia command
Could you see if reverting this commit on git head works for you (in
which case I'm not reproducing it correctly)?
Thanks,
James
Hi James,
On Fri, 27 Jun 2008, James Bottomley wrote:
> > git-bisect taught me it was introduced by
> >
> > commit 38582a62ecd337de4212004c7d4844899dc57890
> > Author: James Bottomley <[email protected]>
> > Date: Wed Feb 6 13:01:58 2008 -0600
> >
> > [SCSI] sr: fix test unit ready responses
> >
> > Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> > the scsi_test_unit_ready() function. Unfortunately, this has the
> > wrong characteristic of eating NOT_READY returns which sr.c relies on
> > for tray status.
> >
> > Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
>
> OK, I thought I had a test case for this, but when I revert this commit
> on git head (and fix up the one reject which just leaves the sr_
> function in place) I still produce the same behaviour.
>
> What I'm trying is
>
> sg_start -i -l <cdrom>
>
> to close the tray followed by your cdparanoia command
>
> Could you see if reverting this commit on git head works for you (in
> which case I'm not reproducing it correctly)?
On 9bedbcb207ed9a571b239231d99c8fd4a34ae24d, the sequence
eject; sg_start -i -l /dev/scd0; cdparanoia \
-d /dev/scd0 -Z -q 1-1[:1] /dev/null || echo failed
fails with
004: Unable to read table of contents header
After reverting 38582a62ecd337de4212004c7d4844899dc57890, it works.
I added the eject as the PS3 has a slot-loading drive.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB
On Mon, 2008-06-30 at 11:25 +0200, Geert Uytterhoeven wrote:
> Hi James,
>
> On Fri, 27 Jun 2008, James Bottomley wrote:
> > > git-bisect taught me it was introduced by
> > >
> > > commit 38582a62ecd337de4212004c7d4844899dc57890
> > > Author: James Bottomley <[email protected]>
> > > Date: Wed Feb 6 13:01:58 2008 -0600
> > >
> > > [SCSI] sr: fix test unit ready responses
> > >
> > > Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> > > the scsi_test_unit_ready() function. Unfortunately, this has the
> > > wrong characteristic of eating NOT_READY returns which sr.c relies on
> > > for tray status.
> > >
> > > Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
> >
> > OK, I thought I had a test case for this, but when I revert this commit
> > on git head (and fix up the one reject which just leaves the sr_
> > function in place) I still produce the same behaviour.
> >
> > What I'm trying is
> >
> > sg_start -i -l <cdrom>
> >
> > to close the tray followed by your cdparanoia command
> >
> > Could you see if reverting this commit on git head works for you (in
> > which case I'm not reproducing it correctly)?
>
> On 9bedbcb207ed9a571b239231d99c8fd4a34ae24d, the sequence
>
> eject; sg_start -i -l /dev/scd0; cdparanoia \
> -d /dev/scd0 -Z -q 1-1[:1] /dev/null || echo failed
>
> fails with
>
> 004: Unable to read table of contents header
>
> After reverting 38582a62ecd337de4212004c7d4844899dc57890, it works.
>
> I added the eject as the PS3 has a slot-loading drive.
>
> With kind regards,
OK ... but this doesn't happen for me with or without this commit
reverted. This is what I see from stracing cdparanoia:
lstat64("/dev/scd0", {st_mode=S_IFBLK|0660, st_rdev=makedev(11, 0), ...}) = 0
open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
ioctl(3, SG_IO, 0xbfc0afdc) = -1 EINVAL (Invalid argument)
close(3) = 0
open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
dup(3) = 4
ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0xbfc0b240) = 0
ioctl(3, SCSI_IOCTL_GET_BUS_NUMBER, 0xbfc0b248) = 0
ioctl(3, SG_IO, 0xbfc0af8c) = 0
ioctl(4, SG_EMULATED_HOST, 0xbfc0b2b8) = 0
ioctl(3, SG_IO, 0xbfc0b07c) = 0
ioctl(3, SG_IO, 0xbfc0b04c) = 0
write(2, "004: Unable to read table of con"..., 45004: Unable to read table of contents header) = 45
The O_NONBLOCK tells the cdrom layer not to do drive ready processing.
The three SG_IOs are INQUIRY, MODE_SENSE(10) and READ TOC/PMA/ATIP
The latter one is returned with a check condition and sense data not
ready; medium not present - tray open.
There's no way the drive ready processing can have an effect on this
sequence unless cdparanoia either invokes it from the cdrom layer or
processes the not ready itself. Could you strace your cdparanoia and
see what sequence it is using?
Thanks,
James
On Mon, 2008-06-30 at 10:51 -0500, James Bottomley wrote:
> On Mon, 2008-06-30 at 11:25 +0200, Geert Uytterhoeven wrote:
> > Hi James,
> >
> > On Fri, 27 Jun 2008, James Bottomley wrote:
> > > > git-bisect taught me it was introduced by
> > > >
> > > > commit 38582a62ecd337de4212004c7d4844899dc57890
> > > > Author: James Bottomley <[email protected]>
> > > > Date: Wed Feb 6 13:01:58 2008 -0600
> > > >
> > > > [SCSI] sr: fix test unit ready responses
> > > >
> > > > Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> > > > the scsi_test_unit_ready() function. Unfortunately, this has the
> > > > wrong characteristic of eating NOT_READY returns which sr.c relies on
> > > > for tray status.
> > > >
> > > > Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
> > >
> > > OK, I thought I had a test case for this, but when I revert this commit
> > > on git head (and fix up the one reject which just leaves the sr_
> > > function in place) I still produce the same behaviour.
> > >
> > > What I'm trying is
> > >
> > > sg_start -i -l <cdrom>
> > >
> > > to close the tray followed by your cdparanoia command
> > >
> > > Could you see if reverting this commit on git head works for you (in
> > > which case I'm not reproducing it correctly)?
> >
> > On 9bedbcb207ed9a571b239231d99c8fd4a34ae24d, the sequence
> >
> > eject; sg_start -i -l /dev/scd0; cdparanoia \
> > -d /dev/scd0 -Z -q 1-1[:1] /dev/null || echo failed
> >
> > fails with
> >
> > 004: Unable to read table of contents header
> >
> > After reverting 38582a62ecd337de4212004c7d4844899dc57890, it works.
> >
> > I added the eject as the PS3 has a slot-loading drive.
> >
> > With kind regards,
>
> OK ... but this doesn't happen for me with or without this commit
> reverted. This is what I see from stracing cdparanoia:
>
> lstat64("/dev/scd0", {st_mode=S_IFBLK|0660, st_rdev=makedev(11, 0), ...}) = 0
> open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
> ioctl(3, SG_IO, 0xbfc0afdc) = -1 EINVAL (Invalid argument)
> close(3) = 0
> open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
> dup(3) = 4
> ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0xbfc0b240) = 0
> ioctl(3, SCSI_IOCTL_GET_BUS_NUMBER, 0xbfc0b248) = 0
> ioctl(3, SG_IO, 0xbfc0af8c) = 0
> ioctl(4, SG_EMULATED_HOST, 0xbfc0b2b8) = 0
> ioctl(3, SG_IO, 0xbfc0b07c) = 0
> ioctl(3, SG_IO, 0xbfc0b04c) = 0
> write(2, "004: Unable to read table of con"..., 45004: Unable to read table of contents header) = 45
>
> The O_NONBLOCK tells the cdrom layer not to do drive ready processing.
>
> The three SG_IOs are INQUIRY, MODE_SENSE(10) and READ TOC/PMA/ATIP
>
> The latter one is returned with a check condition and sense data not
> ready; medium not present - tray open.
>
> There's no way the drive ready processing can have an effect on this
> sequence unless cdparanoia either invokes it from the cdrom layer or
> processes the not ready itself. Could you strace your cdparanoia and
> see what sequence it is using?
OK ... I think I finally found the problem. Our DVD drives obviously
have a slight difference in the way they handle tray close. Mine still
returns tray open for a while after the close operation has been
initiated. To see the behaviour you need it to return not ready; in
process of becoming ready. The zero return from scsi_test_unit_ready()
actually causes sr_media_changed() incorrectly to return zero. But,
before it does, it tries to update the CD information and capacity.
This is where the delay occurs ... as long as the drive reports in
process of becoming ready, sr_cd_check() will wait 2s and retry until it
becomes ready and it can get the CD information.
The problem is I don't think the new behaviour is a regression.
cdparanoia requested O_NONBLOCK ... it's a bit counter to this if we
wait for the drive to become ready. cdparanoia doesn't care that we get
the correct CD parameters on the open since it's using SG_IO to
manipulate the device. So, I think the bug is actually in cdparanoia.
If it requests O_NONBLOCK, it's asking for full status immediately and
is supposed to be able to cope with the returns (including knowing to
retry the NOT_READY ones).
James
Hi James,
On Mon, 30 Jun 2008, James Bottomley wrote:
> On Mon, 2008-06-30 at 11:25 +0200, Geert Uytterhoeven wrote:
> > On Fri, 27 Jun 2008, James Bottomley wrote:
> > > > git-bisect taught me it was introduced by
> > > >
> > > > commit 38582a62ecd337de4212004c7d4844899dc57890
> > > > Author: James Bottomley <[email protected]>
> > > > Date: Wed Feb 6 13:01:58 2008 -0600
> > > >
> > > > [SCSI] sr: fix test unit ready responses
> > > >
> > > > Commit 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 updated sr.c to use
> > > > the scsi_test_unit_ready() function. Unfortunately, this has the
> > > > wrong characteristic of eating NOT_READY returns which sr.c relies on
> > > > for tray status.
> > > >
> > > > Fix by rolling an internal sr_test_unit_ready() that doesn't do this.
> > >
> > > OK, I thought I had a test case for this, but when I revert this commit
> > > on git head (and fix up the one reject which just leaves the sr_
> > > function in place) I still produce the same behaviour.
> > >
> > > What I'm trying is
> > >
> > > sg_start -i -l <cdrom>
> > >
> > > to close the tray followed by your cdparanoia command
> > >
> > > Could you see if reverting this commit on git head works for you (in
> > > which case I'm not reproducing it correctly)?
> >
> > On 9bedbcb207ed9a571b239231d99c8fd4a34ae24d, the sequence
> >
> > eject; sg_start -i -l /dev/scd0; cdparanoia \
> > -d /dev/scd0 -Z -q 1-1[:1] /dev/null || echo failed
> >
> > fails with
> >
> > 004: Unable to read table of contents header
> >
> > After reverting 38582a62ecd337de4212004c7d4844899dc57890, it works.
> >
> > I added the eject as the PS3 has a slot-loading drive.
> >
> > With kind regards,
>
> OK ... but this doesn't happen for me with or without this commit
> reverted. This is what I see from stracing cdparanoia:
>
> lstat64("/dev/scd0", {st_mode=S_IFBLK|0660, st_rdev=makedev(11, 0), ...}) = 0
> open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
> ioctl(3, SG_IO, 0xbfc0afdc) = -1 EINVAL (Invalid argument)
> close(3) = 0
> open("/dev/scd0", O_RDWR|O_NONBLOCK) = 3
> dup(3) = 4
> ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0xbfc0b240) = 0
> ioctl(3, SCSI_IOCTL_GET_BUS_NUMBER, 0xbfc0b248) = 0
> ioctl(3, SG_IO, 0xbfc0af8c) = 0
> ioctl(4, SG_EMULATED_HOST, 0xbfc0b2b8) = 0
> ioctl(3, SG_IO, 0xbfc0b07c) = 0
> ioctl(3, SG_IO, 0xbfc0b04c) = 0
> write(2, "004: Unable to read table of con"..., 45004: Unable to read table of contents header) = 45
>
> The O_NONBLOCK tells the cdrom layer not to do drive ready processing.
>
> The three SG_IOs are INQUIRY, MODE_SENSE(10) and READ TOC/PMA/ATIP
>
> The latter one is returned with a check condition and sense data not
> ready; medium not present - tray open.
>
> There's no way the drive ready processing can have an effect on this
> sequence unless cdparanoia either invokes it from the cdrom layer or
> processes the not ready itself. Could you strace your cdparanoia and
> see what sequence it is using?
My cdparanioa is using the exact same sequence.
Sorry it took that long to get back to you (holidays and OLS).
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB