Jens Axboe wrote:
>
> 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;
>
Yes. test13-pre4 + the above patch is back to normal speed. I had
spotted that too as a likely candidate, especially because when the
accesses were slow, the light on the cdrom was blinking at a much higher
rate than before (I suppose it was processing 8x the number of commands,
right?).
Anyway, do you think a 'try to allocate 8, if that fails, try to
allocate 1' solution would be a simple compromise? That should be easy
to do, based on the above code (if kmalloc returns NULL && frames > 1,
frames = 1, retry...).
David
-- David Mansfield (718) 963-2020 david@ultramaster.com Ultramaster Group, LLC www.ultramaster.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Dec 31 2000 - 21:00:09 EST