Messages in this thread |  | | Date | Tue, 26 Dec 2000 15:51:22 -0500 | From | David Mansfield <> | Subject | Re: cdrom changes in test13-pre2 slow down cdrom access by 70% |
| |
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/
|  |