Web lists-archives.com

Re: About /dev/sr impatience with automatic tray loading




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