On 2018-12-10 20:02, Thomas Schmitt wrote:
Hi,
Gene Heskett wrote:
Perhaps that patch could be reverted,
It had its legitimate intentions, 10 years ago.
See
https://github.com/torvalds/linux/commit/210ba1d1724f5c4ed87a2ab1a21ca861a915f734
and a glimpse of the following woes
http://lkml.iu.edu/hypermail//linux/kernel/0807.0/0287.html
(The culprits already suffered duely. Hehehe.)
The decisive change is not easy to spot. It's the loss of the function
static int test_unit_ready(Scsi_CD *cd)
with the line
return sr_do_ioctl(cd, &cgc);
sr_do_ioctl() has a waiting loop that eats "still undecided" replies
and
retries at most 10 times. It still exists in the current kernel
https://github.com/torvalds/linux/blob/master/drivers/scsi/sr_ioctl.c
where in line 223 ff. we can see the old timeout of 20 seconds
if (!cgc->quiet)
sr_printk(KERN_INFO, cd,
"CDROM not ready yet.\n");
if (retries++ < 10) {
/* sleep 2 sec and try again */
ssleep(2);
goto retry;
} else {
/* 20 secs are enough? */
err = -ENOMEDIUM;
break;
}
The replacement for test_unit_ready() is a call to
scsi_test_unit_ready()
which does not call a function with retry loop.
For the purpose of sr_drive_status(), the loop is really inappropriate.
This function shall obtain the drive status and not wait until the
status of the medium is decided.
Regrettably the subsequent correction attempts never reached the doings
of drivers/cdrom/cdrom.c function open_for_data(). By the original
change it lost its loop and never got a new one.
My fix proposal is to create a function with such a loop and to let
open_for_data() use it instead of the call of cdo->drive_status(),
which actually is sr_drive_status().
Currently this call is in line 1068 of
https://github.com/torvalds/linux/blob/master/drivers/cdrom/cdrom.c
and the timeout made say 2 minutes,
by which time the drive should be able to make up its mind
30 seconds seems to be enough for all normal situations. My longest
experiment result with various drives and media was 18 seconds.
One would have to convince the hypothetical committer why 120 is
needed.
But before commit comes the test and before test comes the recent
kernel
on real iron. I only have code for now and feel fewly talented for the
missing steps.
That I think we can all agree is a PITA.
Oh. I know some more such dumplings.
Have a nice day :)
Thomas
completely off the topic but I have noticed that people whose first
language might not be english use
"shall" as apposed to "will" or "should". It seems a little bit old
fashioned but maybe it isn't.
Like some check list of an experiment approached logically.
Just something I was curious about.
"This function shall obtain the drive status and not wait until the
status of the medium is decided."
mick
--
Key ID 4BFEBB31
