Re: cdrom changes in test13-pre2 slow down cdrom access by 70%

Jens Axboe
2000-12-23T13:07:46Z

On Fri, Dec 22 2000, David Mansfield wrote:
> Jens,
>
> The cdrom changes that went into test13-pre2 really kill the performance
> of my cdrom. I'm using cdparanoia to read audio data, and it normally
> reads at 2-3x. Since test13-pre2 it's down to .6 - .7x. I've reverted
> the following files to the ones from test13-pre1 and it's back to
> normal:

Humm, interesting.

> This is a huge patch, is there some way I could break it apart to see
> what the relevant changes are?

The change affecting you is most likely the CDROMREADAUDIO change,
where we now just read a single cdda frame at the time. This gives
us less data per interrupt, and apparently this is more than a
theoretical slowdown for you. Please try with attached patch. If
this solves it (as it should), then we should probably try and do
persistent allocation of a bigger buffer for things like this.
Grabbing > 1 frame was disabled because multi-page allocation is not
reliable.

--- drivers/cdrom/cdrom.c~ Sat Dec 23 13:27:33 2000
+++ drivers/cdrom/cdrom.c Sat Dec 23 13:30:39 2000
@@ -1985,7 +1985,7 @@
}
case CDROMREADAUDIO: {
struct cdrom_read_audio ra;
- int lba;
+ int lba, frames;

IOCTL_IN(arg, struct cdrom_read_audio, ra);

@@ -2002,7 +2002,9 @@
if (lba < 0 || ra.nframes <= 0)
return -EINVAL;

- if ((cgc.buffer = (char *) kmalloc(CD_FRAMESIZE_RAW, GFP_KERNEL)) == NULL)
+ frames = ra.nframes > 8 ? 8 : ra.nframes;
+
+ if ((cgc.buffer = (char *) kmalloc(CD_FRAMESIZE_RAW * frames, GFP_KERNEL)) == NULL)
return -ENOMEM;

if (!access_ok(VERIFY_WRITE, ra.buf, ra.nframes*CD_FRAMESIZE_RAW)) {
@@ -2011,12 +2013,14 @@
}
cgc.data_direction = CGC_DATA_READ;
while (ra.nframes > 0) {
- ret = cdrom_read_block(cdi, &cgc, lba, 1, 1, CD_FRAMESIZE_RAW);
- if (ret) break;
- __copy_to_user(ra.buf, cgc.buffer, CD_FRAMESIZE_RAW);
- ra.buf += CD_FRAMESIZE_RAW;
- ra.nframes--;
- lba++;
+ ret = cdrom_read_block(cdi, &cgc, lba, frames, 1, CD_FRAMESIZE_RAW);
+ if (ret)
+ break;
+ __copy_to_user(ra.buf, cgc.buffer,
+ CD_FRAMESIZE_RAW * frames);
+ ra.buf += (CD_FRAMESIZE_RAW * frames);
+ ra.nframes -= frames;
+ lba += frames;
}
kfree(cgc.buffer);
return ret;

--
* Jens Axboe <axboe@suse.de>
* SuSE Labs