[PATCH] 2.6 ide-cd DMA ripping

[PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

Hi,

2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
optimal of course... This patch uses the block layer infrastructure to
enable zero copy DMA ripping through CDROMREADAUDIO.

I'd appreciate people giving this a test spin. Patch is against
2.6.4-rc1 (well current BK, actually).

===== drivers/block/ll_rw_blk.c 1.228 vs edited =====
--- 1.228/drivers/block/ll_rw_blk.c	Sun Feb  1 19:09:12 2004
+++ edited/drivers/block/ll_rw_blk.c	Wed Mar  3 11:34:38 2004
@@ -28,6 +28,11 @@
 #include <linux/slab.h>
 #include <linux/swap.h>
 
+/*
+ * for max sense size
+ */
+#include <scsi/scsi_cmnd.h>
+
 static void blk_unplug_work(void *data);
 static void blk_unplug_timeout(unsigned long data);
 
@@ -1756,6 +1761,138 @@
 }
 
 EXPORT_SYMBOL(blk_insert_request);
+
+/**
+ * blk_rq_map_user - map user data to a request, for REQ_BLOCK_PC usage
+ * @q:		request queue where request should be inserted
+ * @rw:		READ or WRITE data
+ * @ubuf:	the user buffer
+ * @len:	length of user data
+ *
+ * Description:
+ *    Data will be mapped directly for zero copy io, if possible. Otherwise
+ *    a kernel bounce buffer is used.
+ *
+ *    A matching blk_rq_unmap_user() must be issued at the end of io, while
+ *    still in process context.
+ */
+struct request *blk_rq_map_user(request_queue_t *q, int rw, void __user *ubuf,
+				unsigned int len)
+{
+	struct request *rq = NULL;
+	char *buf = NULL;
+	struct bio *bio;
+
+	rq = blk_get_request(q, rw, __GFP_WAIT);
+	if (!rq)
+		return NULL;
+
+	bio = bio_map_user(q, NULL, (unsigned long) ubuf, len, rw == READ);
+	if (!bio) {
+		buf = kmalloc(len, q->bounce_gfp | GFP_USER);
+		if (!buf)
+			goto fault;
+
+		if (rw == WRITE) {
+			if (copy_from_user(buf, ubuf, len))
+				goto fault;
+		} else
+			memset(buf, 0, len);
+	}
+
+	rq->bio = rq->biotail = bio;
+	if (rq->bio)
+		blk_rq_bio_prep(q, rq, bio);
+
+	rq->buffer = rq->data = buf;
+	rq->data_len = len;
+	return rq;
+fault:
+	if (buf)
+		kfree(buf);
+	if (bio)
+		bio_unmap_user(bio, 1);
+	if (rq)
+		blk_put_request(rq);
+
+	return NULL;
+}
+
+EXPORT_SYMBOL(blk_rq_map_user);
+
+/**
+ * blk_rq_unmap_user - unmap a request with user data
+ * @rq:		request to be unmapped
+ * @ubuf:	user buffer
+ * @ulen:	length of user buffer
+ * @rw:		operation was a READ or WRITE
+ *
+ * Description:
+ *    Unmap a request previously mapped by blk_rq_map_user().
+ */
+int blk_rq_unmap_user(struct request *rq, void __user *ubuf, unsigned int ulen,
+		      int rw)
+{
+	int ret = 0;
+
+	if (rq->biotail)
+		bio_unmap_user(rq->biotail, rw == READ);
+	if (rq->buffer) {
+		if (rw == READ && copy_to_user(ubuf, rq->buffer, ulen))
+			ret = -EFAULT;
+		kfree(rq->buffer);
+	}
+
+	blk_put_request(rq);
+	return ret;
+}
+
+EXPORT_SYMBOL(blk_rq_unmap_user);
+
+/**
+ * blk_execute_rq - insert a request into queue for execution
+ * @q:		queue to insert the request in
+ * @bd_disk:	matching gendisk
+ * @rq:		request to insert
+ *
+ * Description:
+ *    Insert a fully prepared request at the back of the io scheduler queue
+ *    for execution.
+ */
+int blk_execute_rq(request_queue_t *q, struct gendisk *bd_disk,
+		   struct request *rq)
+{
+	DECLARE_COMPLETION(wait);
+	char sense[SCSI_SENSE_BUFFERSIZE];
+	int err = 0;
+
+	rq->rq_disk = bd_disk;
+
+	/*
+	 * we need an extra reference to the request, so we can look at
+	 * it after io completion
+	 */
+	rq->ref_count++;
+
+	if (!rq->sense) {
+		memset(sense, 0, sizeof(sense));
+		rq->sense = sense;
+		rq->sense_len = 0;
+	}
+
+	rq->flags |= REQ_NOMERGE;
+	rq->waiting = &wait;
+	elv_add_request(q, rq, ELEVATOR_INSERT_BACK, 1);
+	generic_unplug_device(q);
+	wait_for_completion(&wait);
+
+	if (rq->errors)
+		err = -EIO;
+
+	return err;
+}
+
+EXPORT_SYMBOL(blk_execute_rq);
 
 void drive_stat_acct(struct request *rq, int nr_sectors, int new_io)
 {
===== drivers/block/scsi_ioctl.c 1.40 vs edited =====
--- 1.40/drivers/block/scsi_ioctl.c	Sun Feb  1 12:53:03 2004
+++ edited/drivers/block/scsi_ioctl.c	Wed Mar  3 11:28:07 2004
@@ -30,7 +30,7 @@
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
-
+#include <scsi/scsi_cmnd.h>
 
 /* Command group 3 is reserved and should never be used.  */
 const unsigned char scsi_command_size[8] =
@@ -41,44 +41,6 @@
 
 #define BLK_DEFAULT_TIMEOUT	(60 * HZ)
 
-/* defined in ../scsi/scsi.h  ... should it be included? */
-#ifndef SCSI_SENSE_BUFFERSIZE
-#define SCSI_SENSE_BUFFERSIZE 64
-#endif
-
-static int blk_do_rq(request_queue_t *q, struct gendisk *bd_disk,
-		     struct request *rq)
-{
-	char sense[SCSI_SENSE_BUFFERSIZE];
-	DECLARE_COMPLETION(wait);
-	int err = 0;
-
-	rq->rq_disk = bd_disk;
-
-	/*
-	 * we need an extra reference to the request, so we can look at
-	 * it after io completion
-	 */
-	rq->ref_count++;
-
-	if (!rq->sense) {
-		memset(sense, 0, sizeof(sense));
-		rq->sense = sense;
-		rq->sense_len = 0;
-	}
-
-	rq->flags |= REQ_NOMERGE;
-	rq->waiting = &wait;
-	elv_add_request(q, rq, ELEVATOR_INSERT_BACK, 1);
-	generic_unplug_device(q);
-	wait_for_completion(&wait);
-
-	if (rq->errors)
-		err = -EIO;
-
-	return err;
-}
-
 #include <scsi/sg.h>
 
 static int sg_get_version(int *p)
@@ -246,7 +208,7 @@
 	 * (if he doesn't check that is his problem).
 	 * N.B. a non-zero SCSI status is _not_ necessarily an error.
 	 */
-	blk_do_rq(q, bd_disk, rq);
+	blk_execute_rq(q, bd_disk, rq);
 
 	if (bio)
 		bio_unmap_user(bio, reading);
@@ -369,7 +331,7 @@
 	rq->data_len = bytes;
 	rq->flags |= REQ_BLOCK_PC;
 
-	blk_do_rq(q, bd_disk, rq);
+	blk_execute_rq(q, bd_disk, rq);
 	err = rq->errors & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
 		if (rq->sense_len && rq->sense) {
@@ -529,7 +491,7 @@
 			rq->cmd[0] = GPCMD_START_STOP_UNIT;
 			rq->cmd[4] = 0x02 + (close != 0);
 			rq->cmd_len = 6;
-			err = blk_do_rq(q, bd_disk, rq);
+			err = blk_execute_rq(q, bd_disk, rq);
 			blk_put_request(rq);
 			break;
 		default:
===== drivers/cdrom/cdrom.c 1.48 vs edited =====
--- 1.48/drivers/cdrom/cdrom.c	Mon Feb  9 21:58:21 2004
+++ edited/drivers/cdrom/cdrom.c	Wed Mar  3 12:34:01 2004
@@ -406,6 +406,11 @@
 	if (CDROM_CAN(CDC_MRW_W))
 		cdi->exit = cdrom_mrw_exit;
 
+	if (cdi->disk)
+		cdi->cdda_method = CDDA_BPC_FULL;
+	else
+		cdi->cdda_method = CDDA_OLD;
+
 	cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
 	spin_lock(&cdrom_lock);
 	cdi->next = topCdromPtr; 	
@@ -1788,6 +1793,142 @@
 	return cdo->generic_packet(cdi, cgc);
 }
 
+static int cdrom_read_cdda_old(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			       int lba, int nframes)
+{
+	struct cdrom_generic_command cgc;
+	int nr, ret;
+
+	memset(&cgc, 0, sizeof(cgc));
+
+	/*
+	 * start with will ra.nframes size, back down if alloc fails
+	 */
+	nr = nframes;
+	do {
+		cgc.buffer = kmalloc(CD_FRAMESIZE_RAW * nr, GFP_KERNEL);
+		if (cgc.buffer)
+			break;
+
+		nr >>= 1;
+	} while (nr);
+
+	if (!nr)
+		return -ENOMEM;
+
+	if (!access_ok(VERIFY_WRITE, ubuf, nframes * CD_FRAMESIZE_RAW)) {
+		kfree(cgc.buffer);
+		return -EFAULT;
+	}
+
+	cgc.data_direction = CGC_DATA_READ;
+	while (nframes > 0) {
+		if (nr > nframes)
+			nr = nframes;
+
+		ret = cdrom_read_block(cdi, &cgc, lba, nr, 1, CD_FRAMESIZE_RAW);
+		if (ret)
+			break;
+		__copy_to_user(ubuf, cgc.buffer, CD_FRAMESIZE_RAW * nr);
+		ubuf += CD_FRAMESIZE_RAW * nr;
+		nframes -= nr;
+		lba += nr;
+	}
+	kfree(cgc.buffer);
+	return 0;
+}
+
+static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			       int lba, int nframes)
+{
+	request_queue_t *q = cdi->disk->queue;
+	struct request *rq;
+	unsigned int len;
+	int nr, ret = 0;
+
+	if (!q)
+		return -ENXIO;
+
+	while (nframes) {
+		nr = nframes;
+		if (cdi->cdda_method == CDDA_BPC_SINGLE)
+			nr = 1;
+		if (nr * CD_FRAMESIZE_RAW > (q->max_sectors << 9))
+			nr = (q->max_sectors << 9) / CD_FRAMESIZE_RAW;
+
+		len = nr * CD_FRAMESIZE_RAW;
+
+		rq = blk_rq_map_user(q, READ, ubuf, len);
+		if (!rq)
+			return -ENOMEM;
+
+		memset(rq->cmd, 0, sizeof(rq->cmd));
+		rq->cmd[0] = GPCMD_READ_CD;
+		rq->cmd[1] = 1 << 2;
+		rq->cmd[2] = (lba >> 24) & 0xff;
+		rq->cmd[3] = (lba >> 16) & 0xff;
+		rq->cmd[4] = (lba >>  8) & 0xff;
+		rq->cmd[5] = lba & 0xff;
+		rq->cmd[6] = (nr >> 16) & 0xff;
+		rq->cmd[7] = (nr >>  8) & 0xff;
+		rq->cmd[8] = nr & 0xff;
+		rq->cmd[9] = 0xf8;
+
+		rq->cmd_len = 12;
+		rq->flags |= REQ_BLOCK_PC;
+		rq->timeout = 60 * HZ;
+
+		if (blk_execute_rq(q, cdi->disk, rq)) {
+			struct request_sense *s = rq->sense;
+			ret = -EIO;
+			printk("cdrom: cdda rip sense %02x/%02x/%02x\n", s->sense_key, s->asc, s->ascq);
+		}
+
+		if (blk_rq_unmap_user(rq, ubuf, len, READ))
+			ret = -EFAULT;
+
+		if (ret)
+			break;
+
+		nframes -= nr;
+		lba += nr;
+	}
+
+	return ret;
+}
+
+static int cdrom_read_cdda(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			   int lba, int nframes)
+{
+	int ret;
+
+	if (cdi->cdda_method == CDDA_OLD)
+		return cdrom_read_cdda_old(cdi, ubuf, lba, nframes);
+
+	do {
+		ret = cdrom_read_cdda_bpc(cdi, ubuf, lba, nframes);
+
+		if (!ret)
+			break;
+
+		/*
+		 * I've seen drives get sense 4/8/3 udma crc errors, so
+		 * drop to single frame dma if we need to
+		 */
+		if (cdi->cdda_method == CDDA_BPC_FULL && nframes > 1) {
+			printk("cdrom: dropping to single frame dma\n");
+			cdi->cdda_method = CDDA_BPC_SINGLE;
+			continue;
+		}
+
+		printk("cdrom: dropping to old style cdda\n");
+		cdi->cdda_method = CDDA_OLD;
+		ret = cdrom_read_cdda_old(cdi, ubuf, lba, nframes);	
+	} while (0);
+
+	return ret;
+}
+
 /* Just about every imaginable ioctl is supported in the Uniform layer
  * these days. ATAPI / SCSI specific code now mainly resides in
  * mmc_ioct().
@@ -2280,7 +2421,7 @@
 		}
 	case CDROMREADAUDIO: {
 		struct cdrom_read_audio ra;
-		int lba, nr;
+		int lba;
 
 		IOCTL_IN(arg, struct cdrom_read_audio, ra);
 
@@ -2297,40 +2438,7 @@
 		if (lba < 0 || ra.nframes <= 0 || ra.nframes > CD_FRAMES)
 			return -EINVAL;
 
-		/*
-		 * start with will ra.nframes size, back down if alloc fails
-		 */
-		nr = ra.nframes;
-		do {
-			cgc.buffer = kmalloc(CD_FRAMESIZE_RAW * nr, GFP_KERNEL);
-			if (cgc.buffer)
-				break;
-
-			nr >>= 1;
-		} while (nr);
-
-		if (!nr)
-			return -ENOMEM;
-
-		if (!access_ok(VERIFY_WRITE, ra.buf, ra.nframes*CD_FRAMESIZE_RAW)) {
-			kfree(cgc.buffer);
-			return -EFAULT;
-		}
-		cgc.data_direction = CGC_DATA_READ;
-		while (ra.nframes > 0) {
-			if (nr > ra.nframes)
-				nr = ra.nframes;
-
-			ret = cdrom_read_block(cdi, &cgc, lba, nr, 1, CD_FRAMESIZE_RAW);
-			if (ret)
-				break;
-			__copy_to_user(ra.buf, cgc.buffer, CD_FRAMESIZE_RAW*nr);
-			ra.buf += CD_FRAMESIZE_RAW * nr;
-			ra.nframes -= nr;
-			lba += nr;
-		}
-		kfree(cgc.buffer);
-		return ret;
+		return cdrom_read_cdda(cdi, ra.buf, lba, ra.nframes);
 		}
 	case CDROMSUBCHNL: {
 		struct cdrom_subchnl q;
===== drivers/ide/ide-cd.c 1.72 vs edited =====
--- 1.72/drivers/ide/ide-cd.c	Thu Feb 19 02:09:06 2004
+++ edited/drivers/ide/ide-cd.c	Wed Mar  3 12:24:21 2004
@@ -2931,6 +2931,7 @@
 	if (!CDROM_CONFIG_FLAGS(drive)->mrw_w)
 		devinfo->mask |= CDC_MRW_W;
 
+	devinfo->disk = drive->disk;
 	return register_cdrom(devinfo);
 }
 
===== drivers/scsi/sr.c 1.99 vs edited =====
--- 1.99/drivers/scsi/sr.c	Mon Feb 23 15:20:57 2004
+++ edited/drivers/scsi/sr.c	Wed Mar  3 11:28:07 2004
@@ -566,13 +566,16 @@
 	snprintf(disk->devfs_name, sizeof(disk->devfs_name),
 			"%s/cd", sdev->devfs_name);
 	disk->driverfs_dev = &sdev->sdev_gendev;
-	register_cdrom(&cd->cdi);
 	set_capacity(disk, cd->capacity);
 	disk->private_data = &cd->driver;
 	disk->queue = sdev->request_queue;
+	cd->cdi.disk = disk;
 
 	dev_set_drvdata(dev, cd);
 	add_disk(disk);
+
+	if (register_cdrom(&cd->cdi))
+		goto fail_put;
 
 	printk(KERN_DEBUG
 	    "Attached scsi CD-ROM %s at scsi%d, channel %d, id %d, lun %d\n",
===== include/linux/blkdev.h 1.134 vs edited =====
--- 1.134/include/linux/blkdev.h	Sun Feb  1 12:53:03 2004
+++ edited/include/linux/blkdev.h	Wed Mar  3 11:28:07 2004
@@ -514,6 +514,9 @@
 extern void __blk_stop_queue(request_queue_t *q);
 extern void blk_run_queue(request_queue_t *q);
 extern void blk_queue_activity_fn(request_queue_t *, activity_fn *, void *);
+extern struct request *blk_rq_map_user(request_queue_t *, int, void __user *, unsigned int);
+extern int blk_rq_unmap_user(struct request *, void __user *, unsigned int, int);
+extern int blk_execute_rq(request_queue_t *, struct gendisk *, struct request *);
 
 static inline request_queue_t *bdev_get_queue(struct block_device *bdev)
 {
===== include/linux/cdrom.h 1.16 vs edited =====
--- 1.16/include/linux/cdrom.h	Wed Feb  4 06:37:42 2004
+++ edited/include/linux/cdrom.h	Wed Mar  3 12:00:29 2004
@@ -877,10 +877,15 @@
 #include <linux/fs.h>		/* not really needed, later.. */
 #include <linux/device.h>
 
+#define CDDA_OLD		0	/* old style */
+#define CDDA_BPC_SINGLE		1	/* block_pc, but single frame  */
+#define CDDA_BPC_FULL		2	/* full speed block pc */
+
 /* Uniform cdrom data structures for cdrom.c */
 struct cdrom_device_info {
 	struct cdrom_device_ops  *ops;  /* link to device_ops */
 	struct cdrom_device_info *next; /* next device_info for this major */
+	struct gendisk *disk;		/* matching block layer disk */
 	void *handle;		        /* driver-dependent data */
 /* specifications */
 	int mask;                       /* mask of capability: disables them */
@@ -894,6 +899,7 @@
 /* per-device flags */
         __u8 sanyo_slot		: 2;	/* Sanyo 3 CD changer support */
         __u8 reserved		: 6;	/* not used yet */
+	int cdda_method;		/* see flags */
 	int for_data;
 	int (*exit)(struct cdrom_device_info *);
 	int mrw_mode_page;

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Wed, Mar 03 2004, Jens Axboe wrote:
> +static int cdrom_read_cdda(struct cdrom_device_info *cdi, __u8 __user *ubuf,
> +			   int lba, int nframes)
> +{
> +	int ret;
> +
> +	if (cdi->cdda_method == CDDA_OLD)
> +		return cdrom_read_cdda_old(cdi, ubuf, lba, nframes);
> +
> +	do {
> +		ret = cdrom_read_cdda_bpc(cdi, ubuf, lba, nframes);
> +
> +		if (!ret)
> +			break;
> +
> +		/*
> +		 * I've seen drives get sense 4/8/3 udma crc errors, so
> +		 * drop to single frame dma if we need to
> +		 */
> +		if (cdi->cdda_method == CDDA_BPC_FULL && nframes > 1) {
> +			printk("cdrom: dropping to single frame dma\n");
> +			cdi->cdda_method = CDDA_BPC_SINGLE;
> +			continue;
> +		}
> +
> +		printk("cdrom: dropping to old style cdda\n");
> +		cdi->cdda_method = CDDA_OLD;
> +		ret = cdrom_read_cdda_old(cdi, ubuf, lba, nframes);	
> +	} while (0);

Irk, that should have been a while (1) of course, and a break after the
last cdrom_read_cdda_old();

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Wed, Mar 03 2004, Alistair John Strachan wrote:
> On Wednesday 03 March 2004 11:37, you wrote:
> > Hi,
> >
> > 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
> > optimal of course... This patch uses the block layer infrastructure to
> > enable zero copy DMA ripping through CDROMREADAUDIO.
> >
> > I'd appreciate people giving this a test spin. Patch is against
> > 2.6.4-rc1 (well current BK, actually).
> >
> [snip] 
> 
> Is this a general optimisation, i.e. will the rip methods used by
> cdda2wav and cdparanoia, etc. be optimised, or do you need some
> specific userspace tools to utilise it?

The patch only affects CDROMREADAUDIO ioctl. cdda2wav (with recent
libscg) will use SG_IO, which works equally well already. cdparanoia
uses CDROMREADAUDIO as well iirc, if it can use /dev/sg* sg v2
interface. I'm not completely sure, if you send me an strace of the
process in question I can tell you for sure :)

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Alistair John Strachan]

On Wednesday 03 March 2004 11:37, you wrote:
> Hi,
>
> 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
> optimal of course... This patch uses the block layer infrastructure to
> enable zero copy DMA ripping through CDROMREADAUDIO.
>
> I'd appreciate people giving this a test spin. Patch is against
> 2.6.4-rc1 (well current BK, actually).
>
[snip] 

Is this a general optimisation, i.e. will the rip methods used by cdda2wav and 
cdparanoia, etc. be optimised, or do you need some specific userspace tools 
to utilise it?

-- 
Cheers,
Alistair.

personal:   alistair()devzero!co!uk
university: s0348365()sms!ed!ac!uk
student:    CS/AI Undergraduate
contact:    7/10 Darroch Court,
            University of Edinburgh.

Re:[PATCH] 2.6 ide-cd DMA ripping

[Vince]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Hi,

I've tried your patch (with the modifications you hinted, included in 
attachment) with cdparanoia and:

1) it works (I get an identical wav)
2) it makes indeed a huge difference on my system:

- before:
cdparanoia 1  4.20s user 1.98s system 3% cpu 3:12.89 total
- after:
cdparanoia 1  3.87s user 1.41s system 15% cpu 33.793 total

Total time went from 3 min to 30 sec... really great :-)

Vince

[-- Attachment #2: ide-cd-dma.diff --]
[-- Type: text/x-patch, Size: 13456 bytes --]

Hi,

2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
optimal of course... This patch uses the block layer infrastructure to
enable zero copy DMA ripping through CDROMREADAUDIO.

I'd appreciate people giving this a test spin. Patch is against
2.6.4-rc1 (well current BK, actually).

===== drivers/block/ll_rw_blk.c 1.228 vs edited =====
--- 1.228/drivers/block/ll_rw_blk.c	Sun Feb  1 19:09:12 2004
+++ edited/drivers/block/ll_rw_blk.c	Wed Mar  3 11:34:38 2004
@@ -28,6 +28,11 @@
 #include <linux/slab.h>
 #include <linux/swap.h>
 
+/*
+ * for max sense size
+ */
+#include <scsi/scsi_cmnd.h>
+
 static void blk_unplug_work(void *data);
 static void blk_unplug_timeout(unsigned long data);
 
@@ -1756,6 +1761,138 @@
 }
 
 EXPORT_SYMBOL(blk_insert_request);
+
+/**
+ * blk_rq_map_user - map user data to a request, for REQ_BLOCK_PC usage
+ * @q:		request queue where request should be inserted
+ * @rw:		READ or WRITE data
+ * @ubuf:	the user buffer
+ * @len:	length of user data
+ *
+ * Description:
+ *    Data will be mapped directly for zero copy io, if possible. Otherwise
+ *    a kernel bounce buffer is used.
+ *
+ *    A matching blk_rq_unmap_user() must be issued at the end of io, while
+ *    still in process context.
+ */
+struct request *blk_rq_map_user(request_queue_t *q, int rw, void __user *ubuf,
+				unsigned int len)
+{
+	struct request *rq = NULL;
+	char *buf = NULL;
+	struct bio *bio;
+
+	rq = blk_get_request(q, rw, __GFP_WAIT);
+	if (!rq)
+		return NULL;
+
+	bio = bio_map_user(q, NULL, (unsigned long) ubuf, len, rw == READ);
+	if (!bio) {
+		buf = kmalloc(len, q->bounce_gfp | GFP_USER);
+		if (!buf)
+			goto fault;
+
+		if (rw == WRITE) {
+			if (copy_from_user(buf, ubuf, len))
+				goto fault;
+		} else
+			memset(buf, 0, len);
+	}
+
+	rq->bio = rq->biotail = bio;
+	if (rq->bio)
+		blk_rq_bio_prep(q, rq, bio);
+
+	rq->buffer = rq->data = buf;
+	rq->data_len = len;
+	return rq;
+fault:
+	if (buf)
+		kfree(buf);
+	if (bio)
+		bio_unmap_user(bio, 1);
+	if (rq)
+		blk_put_request(rq);
+
+	return NULL;
+}
+
+EXPORT_SYMBOL(blk_rq_map_user);
+
+/**
+ * blk_rq_unmap_user - unmap a request with user data
+ * @rq:		request to be unmapped
+ * @ubuf:	user buffer
+ * @ulen:	length of user buffer
+ * @rw:		operation was a READ or WRITE
+ *
+ * Description:
+ *    Unmap a request previously mapped by blk_rq_map_user().
+ */
+int blk_rq_unmap_user(struct request *rq, void __user *ubuf, unsigned int ulen,
+		      int rw)
+{
+	int ret = 0;
+
+	if (rq->biotail)
+		bio_unmap_user(rq->biotail, rw == READ);
+	if (rq->buffer) {
+		if (rw == READ && copy_to_user(ubuf, rq->buffer, ulen))
+			ret = -EFAULT;
+		kfree(rq->buffer);
+	}
+
+	blk_put_request(rq);
+	return ret;
+}
+
+EXPORT_SYMBOL(blk_rq_unmap_user);
+
+/**
+ * blk_execute_rq - insert a request into queue for execution
+ * @q:		queue to insert the request in
+ * @bd_disk:	matching gendisk
+ * @rq:		request to insert
+ *
+ * Description:
+ *    Insert a fully prepared request at the back of the io scheduler queue
+ *    for execution.
+ */
+int blk_execute_rq(request_queue_t *q, struct gendisk *bd_disk,
+		   struct request *rq)
+{
+	DECLARE_COMPLETION(wait);
+	char sense[SCSI_SENSE_BUFFERSIZE];
+	int err = 0;
+
+	rq->rq_disk = bd_disk;
+
+	/*
+	 * we need an extra reference to the request, so we can look at
+	 * it after io completion
+	 */
+	rq->ref_count++;
+
+	if (!rq->sense) {
+		memset(sense, 0, sizeof(sense));
+		rq->sense = sense;
+		rq->sense_len = 0;
+	}
+
+	rq->flags |= REQ_NOMERGE;
+	rq->waiting = &wait;
+	elv_add_request(q, rq, ELEVATOR_INSERT_BACK, 1);
+	generic_unplug_device(q);
+	wait_for_completion(&wait);
+
+	if (rq->errors)
+		err = -EIO;
+
+	return err;
+}
+
+EXPORT_SYMBOL(blk_execute_rq);
 
 void drive_stat_acct(struct request *rq, int nr_sectors, int new_io)
 {
--- 1.40/drivers/block/scsi_ioctl.c	Sun Feb  1 12:53:03 2004
+++ edited/drivers/block/scsi_ioctl.c	Wed Mar  3 11:28:07 2004
@@ -30,7 +30,7 @@
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
-
+#include <scsi/scsi_cmnd.h>
 
 /* Command group 3 is reserved and should never be used.  */
 const unsigned char scsi_command_size[8] =
@@ -41,44 +41,6 @@
 
 #define BLK_DEFAULT_TIMEOUT	(60 * HZ)
 
-/* defined in ../scsi/scsi.h  ... should it be included? */
-#ifndef SCSI_SENSE_BUFFERSIZE
-#define SCSI_SENSE_BUFFERSIZE 64
-#endif
-
-static int blk_do_rq(request_queue_t *q, struct gendisk *bd_disk,
-		     struct request *rq)
-{
-	char sense[SCSI_SENSE_BUFFERSIZE];
-	DECLARE_COMPLETION(wait);
-	int err = 0;
-
-	rq->rq_disk = bd_disk;
-
-	/*
-	 * we need an extra reference to the request, so we can look at
-	 * it after io completion
-	 */
-	rq->ref_count++;
-
-	if (!rq->sense) {
-		memset(sense, 0, sizeof(sense));
-		rq->sense = sense;
-		rq->sense_len = 0;
-	}
-
-	rq->flags |= REQ_NOMERGE;
-	rq->waiting = &wait;
-	elv_add_request(q, rq, ELEVATOR_INSERT_BACK, 1);
-	generic_unplug_device(q);
-	wait_for_completion(&wait);
-
-	if (rq->errors)
-		err = -EIO;
-
-	return err;
-}
-
 #include <scsi/sg.h>
 
 static int sg_get_version(int *p)
@@ -246,7 +208,7 @@
 	 * (if he doesn't check that is his problem).
 	 * N.B. a non-zero SCSI status is _not_ necessarily an error.
 	 */
-	blk_do_rq(q, bd_disk, rq);
+	blk_execute_rq(q, bd_disk, rq);
 
 	if (bio)
 		bio_unmap_user(bio, reading);
@@ -369,7 +331,7 @@
 	rq->data_len = bytes;
 	rq->flags |= REQ_BLOCK_PC;
 
-	blk_do_rq(q, bd_disk, rq);
+	blk_execute_rq(q, bd_disk, rq);
 	err = rq->errors & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
 		if (rq->sense_len && rq->sense) {
@@ -529,7 +491,7 @@
 			rq->cmd[0] = GPCMD_START_STOP_UNIT;
 			rq->cmd[4] = 0x02 + (close != 0);
 			rq->cmd_len = 6;
-			err = blk_do_rq(q, bd_disk, rq);
+			err = blk_execute_rq(q, bd_disk, rq);
 			blk_put_request(rq);
 			break;
 		default:
--- 1.48/drivers/cdrom/cdrom.c	Mon Feb  9 21:58:21 2004
+++ edited/drivers/cdrom/cdrom.c	Wed Mar  3 12:34:01 2004
@@ -406,6 +406,11 @@
 	if (CDROM_CAN(CDC_MRW_W))
 		cdi->exit = cdrom_mrw_exit;
 
+	if (cdi->disk)
+		cdi->cdda_method = CDDA_BPC_FULL;
+	else
+		cdi->cdda_method = CDDA_OLD;
+
 	cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
 	spin_lock(&cdrom_lock);
 	cdi->next = topCdromPtr; 	
@@ -1788,6 +1793,143 @@
 	return cdo->generic_packet(cdi, cgc);
 }
 
+static int cdrom_read_cdda_old(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			       int lba, int nframes)
+{
+	struct cdrom_generic_command cgc;
+	int nr, ret;
+
+	memset(&cgc, 0, sizeof(cgc));
+
+	/*
+	 * start with will ra.nframes size, back down if alloc fails
+	 */
+	nr = nframes;
+	do {
+		cgc.buffer = kmalloc(CD_FRAMESIZE_RAW * nr, GFP_KERNEL);
+		if (cgc.buffer)
+			break;
+
+		nr >>= 1;
+	} while (nr);
+
+	if (!nr)
+		return -ENOMEM;
+
+	if (!access_ok(VERIFY_WRITE, ubuf, nframes * CD_FRAMESIZE_RAW)) {
+		kfree(cgc.buffer);
+		return -EFAULT;
+	}
+
+	cgc.data_direction = CGC_DATA_READ;
+	while (nframes > 0) {
+		if (nr > nframes)
+			nr = nframes;
+
+		ret = cdrom_read_block(cdi, &cgc, lba, nr, 1, CD_FRAMESIZE_RAW);
+		if (ret)
+			break;
+		__copy_to_user(ubuf, cgc.buffer, CD_FRAMESIZE_RAW * nr);
+		ubuf += CD_FRAMESIZE_RAW * nr;
+		nframes -= nr;
+		lba += nr;
+	}
+	kfree(cgc.buffer);
+	return 0;
+}
+
+static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			       int lba, int nframes)
+{
+	request_queue_t *q = cdi->disk->queue;
+	struct request *rq;
+	unsigned int len;
+	int nr, ret = 0;
+
+	if (!q)
+		return -ENXIO;
+
+	while (nframes) {
+		nr = nframes;
+		if (cdi->cdda_method == CDDA_BPC_SINGLE)
+			nr = 1;
+		if (nr * CD_FRAMESIZE_RAW > (q->max_sectors << 9))
+			nr = (q->max_sectors << 9) / CD_FRAMESIZE_RAW;
+
+		len = nr * CD_FRAMESIZE_RAW;
+
+		rq = blk_rq_map_user(q, READ, ubuf, len);
+		if (!rq)
+			return -ENOMEM;
+
+		memset(rq->cmd, 0, sizeof(rq->cmd));
+		rq->cmd[0] = GPCMD_READ_CD;
+		rq->cmd[1] = 1 << 2;
+		rq->cmd[2] = (lba >> 24) & 0xff;
+		rq->cmd[3] = (lba >> 16) & 0xff;
+		rq->cmd[4] = (lba >>  8) & 0xff;
+		rq->cmd[5] = lba & 0xff;
+		rq->cmd[6] = (nr >> 16) & 0xff;
+		rq->cmd[7] = (nr >>  8) & 0xff;
+		rq->cmd[8] = nr & 0xff;
+		rq->cmd[9] = 0xf8;
+
+		rq->cmd_len = 12;
+		rq->flags |= REQ_BLOCK_PC;
+		rq->timeout = 60 * HZ;
+
+		if (blk_execute_rq(q, cdi->disk, rq)) {
+			struct request_sense *s = rq->sense;
+			ret = -EIO;
+			printk("cdrom: cdda rip sense %02x/%02x/%02x\n", s->sense_key, s->asc, s->ascq);
+		}
+
+		if (blk_rq_unmap_user(rq, ubuf, len, READ))
+			ret = -EFAULT;
+
+		if (ret)
+			break;
+
+		nframes -= nr;
+		lba += nr;
+	}
+
+	return ret;
+}
+
+static int cdrom_read_cdda(struct cdrom_device_info *cdi, __u8 __user *ubuf,
+			   int lba, int nframes)
+{
+	int ret;
+
+	if (cdi->cdda_method == CDDA_OLD)
+		return cdrom_read_cdda_old(cdi, ubuf, lba, nframes);
+
+	do {
+		ret = cdrom_read_cdda_bpc(cdi, ubuf, lba, nframes);
+
+		if (!ret)
+			break;
+
+		/*
+		 * I've seen drives get sense 4/8/3 udma crc errors, so
+		 * drop to single frame dma if we need to
+		 */
+		if (cdi->cdda_method == CDDA_BPC_FULL && nframes > 1) {
+			printk("cdrom: dropping to single frame dma\n");
+			cdi->cdda_method = CDDA_BPC_SINGLE;
+			continue;
+		}
+
+		printk("cdrom: dropping to old style cdda\n");
+		cdi->cdda_method = CDDA_OLD;
+		ret = cdrom_read_cdda_old(cdi, ubuf, lba, nframes);
+		break;
+	} while (1);
+
+	return ret;
+}
+
 /* Just about every imaginable ioctl is supported in the Uniform layer
  * these days. ATAPI / SCSI specific code now mainly resides in
  * mmc_ioct().
@@ -2280,7 +2422,7 @@
 		}
 	case CDROMREADAUDIO: {
 		struct cdrom_read_audio ra;
-		int lba, nr;
+		int lba;
 
 		IOCTL_IN(arg, struct cdrom_read_audio, ra);
 
@@ -2297,40 +2439,7 @@
 		if (lba < 0 || ra.nframes <= 0 || ra.nframes > CD_FRAMES)
 			return -EINVAL;
 
-		/*
-		 * start with will ra.nframes size, back down if alloc fails
-		 */
-		nr = ra.nframes;
-		do {
-			cgc.buffer = kmalloc(CD_FRAMESIZE_RAW * nr, GFP_KERNEL);
-			if (cgc.buffer)
-				break;
-
-			nr >>= 1;
-		} while (nr);
-
-		if (!nr)
-			return -ENOMEM;
-
-		if (!access_ok(VERIFY_WRITE, ra.buf, ra.nframes*CD_FRAMESIZE_RAW)) {
-			kfree(cgc.buffer);
-			return -EFAULT;
-		}
-		cgc.data_direction = CGC_DATA_READ;
-		while (ra.nframes > 0) {
-			if (nr > ra.nframes)
-				nr = ra.nframes;
-
-			ret = cdrom_read_block(cdi, &cgc, lba, nr, 1, CD_FRAMESIZE_RAW);
-			if (ret)
-				break;
-			__copy_to_user(ra.buf, cgc.buffer, CD_FRAMESIZE_RAW*nr);
-			ra.buf += CD_FRAMESIZE_RAW * nr;
-			ra.nframes -= nr;
-			lba += nr;
-		}
-		kfree(cgc.buffer);
-		return ret;
+		return cdrom_read_cdda(cdi, ra.buf, lba, ra.nframes);
 		}
 	case CDROMSUBCHNL: {
 		struct cdrom_subchnl q;
--- 1.72/drivers/ide/ide-cd.c	Thu Feb 19 02:09:06 2004
+++ edited/drivers/ide/ide-cd.c	Wed Mar  3 12:24:21 2004
@@ -2931,6 +2931,7 @@
 	if (!CDROM_CONFIG_FLAGS(drive)->mrw_w)
 		devinfo->mask |= CDC_MRW_W;
 
+	devinfo->disk = drive->disk;
 	return register_cdrom(devinfo);
 }
 
--- 1.99/drivers/scsi/sr.c	Mon Feb 23 15:20:57 2004
+++ edited/drivers/scsi/sr.c	Wed Mar  3 11:28:07 2004
@@ -566,13 +566,16 @@
 	snprintf(disk->devfs_name, sizeof(disk->devfs_name),
 			"%s/cd", sdev->devfs_name);
 	disk->driverfs_dev = &sdev->sdev_gendev;
-	register_cdrom(&cd->cdi);
 	set_capacity(disk, cd->capacity);
 	disk->private_data = &cd->driver;
 	disk->queue = sdev->request_queue;
+	cd->cdi.disk = disk;
 
 	dev_set_drvdata(dev, cd);
 	add_disk(disk);
+
+	if (register_cdrom(&cd->cdi))
+		goto fail_put;
 
 	printk(KERN_DEBUG
 	    "Attached scsi CD-ROM %s at scsi%d, channel %d, id %d, lun %d\n",
--- 1.134/include/linux/blkdev.h	Sun Feb  1 12:53:03 2004
+++ edited/include/linux/blkdev.h	Wed Mar  3 11:28:07 2004
@@ -514,6 +514,9 @@
 extern void __blk_stop_queue(request_queue_t *q);
 extern void blk_run_queue(request_queue_t *q);
 extern void blk_queue_activity_fn(request_queue_t *, activity_fn *, void *);
+extern struct request *blk_rq_map_user(request_queue_t *, int, void __user *, unsigned int);
+extern int blk_rq_unmap_user(struct request *, void __user *, unsigned int, int);
+extern int blk_execute_rq(request_queue_t *, struct gendisk *, struct request *);
 
 static inline request_queue_t *bdev_get_queue(struct block_device *bdev)
 {
--- 1.16/include/linux/cdrom.h	Wed Feb  4 06:37:42 2004
+++ edited/include/linux/cdrom.h	Wed Mar  3 12:00:29 2004
@@ -877,10 +877,15 @@
 #include <linux/fs.h>		/* not really needed, later.. */
 #include <linux/device.h>
 
+#define CDDA_OLD		0	/* old style */
+#define CDDA_BPC_SINGLE		1	/* block_pc, but single frame  */
+#define CDDA_BPC_FULL		2	/* full speed block pc */
+
 /* Uniform cdrom data structures for cdrom.c */
 struct cdrom_device_info {
 	struct cdrom_device_ops  *ops;  /* link to device_ops */
 	struct cdrom_device_info *next; /* next device_info for this major */
+	struct gendisk *disk;		/* matching block layer disk */
 	void *handle;		        /* driver-dependent data */
 /* specifications */
 	int mask;                       /* mask of capability: disables them */
@@ -894,6 +899,7 @@
 /* per-device flags */
         __u8 sanyo_slot		: 2;	/* Sanyo 3 CD changer support */
         __u8 reserved		: 6;	/* not used yet */
+	int cdda_method;		/* see flags */
 	int for_data;
 	int (*exit)(struct cdrom_device_info *);
 	int mrw_mode_page;

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Thu, Mar 04 2004, Vince wrote:
> Hi,
> 
> I've tried your patch (with the modifications you hinted, included in 
> attachment) with cdparanoia and:
> 
> 1) it works (I get an identical wav)
> 2) it makes indeed a huge difference on my system:
> 
> - before:
> cdparanoia 1  4.20s user 1.98s system 3% cpu 3:12.89 total
> - after:
> cdparanoia 1  3.87s user 1.41s system 15% cpu 33.793 total
> 
> Total time went from 3 min to 30 sec... really great :-)

Thanks for testing (and reporting back).

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Colin Leroy]

Hi,

>I'd appreciate people giving this a test spin. Patch is against
>2.6.4-rc1 (well current BK, actually).

Works (on ppc, ibook G4 here). It's indeed faster. But, it breaks direct 
output to dsp (as in `cdparanoia 1 /dev/dsp`). 

HTH,
-- 
Colin

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Fri, Mar 05 2004, Colin Leroy wrote:
> Hi,
> 
> >I'd appreciate people giving this a test spin. Patch is against
> >2.6.4-rc1 (well current BK, actually).
> 
> Works (on ppc, ibook G4 here). It's indeed faster. But, it breaks direct 
> output to dsp (as in `cdparanoia 1 /dev/dsp`). 

How do you know it works, then? cdparanoia should receive identical
data, otherwise it sounds like it doesn't work.

Dump a track without the patch, repeat with the patch, and compare the
images.

(BTW, please cc recipients on lkml. At least to me, otherwise I may not
see your message for days).

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Colin Leroy]

Hi,

> > Works (on ppc, ibook G4 here). It's indeed faster. But, it breaks
direct
> > output to dsp (as in `cdparanoia 1 /dev/dsp`).
>
> How do you know it works, then? cdparanoia should receive identical
> data, otherwise it sounds like it doesn't work.

Well, using 'mplayer cdda.wav' works.

> Dump a track without the patch, repeat with the patch, and compare the
> images.

With or without the patch, that's the exact same file resulting. Maybe the
fact that it doesn't work anymore with /dev/dsp isn't due to your patch
but to something else (I hadn't tried that since a while). I'll report
again as soon as i'll be in front of my laptop.

> (BTW, please cc recipients on lkml. At least to me, otherwise I may not
> see your message for days).

Yup, sorry, I'm not subscribed so created a new mail, and forgot this.

Thanks,
-- 
Colin
  This message represents the official view of the voices
  in my head.

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Fri, Mar 05 2004, Colin Leroy wrote:
> Hi,
> 
> > > Works (on ppc, ibook G4 here). It's indeed faster. But, it breaks
> direct
> > > output to dsp (as in `cdparanoia 1 /dev/dsp`).
> >
> > How do you know it works, then? cdparanoia should receive identical
> > data, otherwise it sounds like it doesn't work.
> 
> Well, using 'mplayer cdda.wav' works.
> 
> > Dump a track without the patch, repeat with the patch, and compare the
> > images.
> 
> With or without the patch, that's the exact same file resulting. Maybe the
> fact that it doesn't work anymore with /dev/dsp isn't due to your patch
> but to something else (I hadn't tried that since a while). I'll report
> again as soon as i'll be in front of my laptop.

Ok great, then it must be something else. Thanks for the report.

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Voluspa]

On 2004-03-03 12:25:06 Jens Axboe wrote:
> On Wed, Mar 03 2004, Alistair John Strachan wrote:
>> On Wednesday 03 March 2004 11:37, you wrote:
[...]
>> Is this a general optimisation, i.e. will the rip methods used by
>> cdda2wav and cdparanoia, etc. be optimised, or do you need some
>> specific userspace tools to utilise it?

> The patch only affects CDROMREADAUDIO ioctl. cdda2wav (with recent
> libscg) will use SG_IO, which works equally well already. cdparanoia
> uses CDROMREADAUDIO as well iirc, if it can use /dev/sg* sg v2
> interface. I'm not completely sure, if you send me an strace of the
> process in question I can tell you for sure :)

Here the patch boosted cdparanoia, but it is far from cdda2wav results
(don't understand the tech talk so just reporting on outcome)

Celeron 800MHz @ 1075MHz, 360Meg mem.  Hewlett-Packard CD-Writer Plus 9100
cdparanoia III release 9.8 (March 23, 2001)
cdda2wav Version 2.01a18

_2.6.4-rc2_ (unpatched)

# time cdda2wav -D /dev/cdrom
[...]
samplefile size will be 52190924 bytes.
recording 295.8666 seconds stereo with 16 bits @ 44100.0 Hz ->'audio'...
overlap:min/max/cur, jitter, percent_done:
 0/ 0/ 1/      0  99%EnableCdda_cooked (CDIOCSETCDDA) is not available...
 0/ 0/ 1/      0 100%  track  1 successfully recorded

real    0m37.923s
user    0m0.144s
sys     0m0.796s

--- (reboot)

# time cdparanoia 1
[...]
real    2m43.071s
user    0m9.039s
sys     0m1.798s

+++

_2.6.4-rc2-cddaDMA_ (patched)

# time cdda2wav -D /dev/cdrom
[same results as unpatched]

--- (reboot)

# time cdparanoia 1
[...]
real    1m54.289s
user    0m6.538s
sys     0m1.381s

# md5sum *.wav
510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-audio.wav
510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cdda.wav
510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cddaDMA-audio.wav
510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cddaDMA-cdda.wav

(and yes, they all play correctly ;-)

PS. Something really strange happened when I wanted to confirm the
playability - which I did _after_ the whole ripping, booting, ripping. I
had lost sound... Total silence from the speaker jack. Alsa was loaded,
/dev/dsp* /dev/audio* was still correct. Every player software acted as
if everything was ok. Speakers functional (confirmed with another
computer). Rebooted with older kernels, shut down completely to give the
hardware a rest. Still silence. Deleted /etc/asound.state and
reconfigured with alsamixer. No change. Perplexed as I've never lost
sound like this, I booted into an old Slackware partition (OSS sound
based). No sound problem there. Booting back out I suddenly had working
sound in my normal environment 2.6.4-rc2. Might be a fluke, space xray
thing, but I would feel more stupid not mentioning it than I do with
these words. DS

Mvh
Mats Johannesson

Re: [PATCH] 2.6 ide-cd DMA ripping

[Sean Neakums]

Jens Axboe <axboe@suse.de> writes:

> Hi,
>
> 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
> optimal of course... This patch uses the block layer infrastructure to
> enable zero copy DMA ripping through CDROMREADAUDIO.
>
> I'd appreciate people giving this a test spin. Patch is against
> 2.6.4-rc1 (well current BK, actually).

Applied successfully to 2.6.4-rc1-mm2, and it works great.  For some
reason, on two different machines, ripping with cdparanoia used to
somehow crowd out the serial port, but now everything just works.

Thanks a lot!

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Fri, Mar 05 2004, Voluspa wrote:
> On 2004-03-03 12:25:06 Jens Axboe wrote:
> > On Wed, Mar 03 2004, Alistair John Strachan wrote:
> >> On Wednesday 03 March 2004 11:37, you wrote:
> [...]
> >> Is this a general optimisation, i.e. will the rip methods used by
> >> cdda2wav and cdparanoia, etc. be optimised, or do you need some
> >> specific userspace tools to utilise it?
> 
> > The patch only affects CDROMREADAUDIO ioctl. cdda2wav (with recent
> > libscg) will use SG_IO, which works equally well already. cdparanoia
> > uses CDROMREADAUDIO as well iirc, if it can use /dev/sg* sg v2
> > interface. I'm not completely sure, if you send me an strace of the
> > process in question I can tell you for sure :)
> 
> Here the patch boosted cdparanoia, but it is far from cdda2wav results
> (don't understand the tech talk so just reporting on outcome)
> 
> Celeron 800MHz @ 1075MHz, 360Meg mem.  Hewlett-Packard CD-Writer Plus 9100
> cdparanoia III release 9.8 (March 23, 2001)
> cdda2wav Version 2.01a18
> 
> _2.6.4-rc2_ (unpatched)
> 
> # time cdda2wav -D /dev/cdrom
> [...]
> samplefile size will be 52190924 bytes.
> recording 295.8666 seconds stereo with 16 bits @ 44100.0 Hz ->'audio'...
> overlap:min/max/cur, jitter, percent_done:
>  0/ 0/ 1/      0  99%EnableCdda_cooked (CDIOCSETCDDA) is not available...
>  0/ 0/ 1/      0 100%  track  1 successfully recorded
> 
> real    0m37.923s
> user    0m0.144s
> sys     0m0.796s
> 
> --- (reboot)
> 
> # time cdparanoia 1
> [...]
> real    2m43.071s
> user    0m9.039s
> sys     0m1.798s
> 
> +++
> 
> _2.6.4-rc2-cddaDMA_ (patched)
> 
> # time cdda2wav -D /dev/cdrom
> [same results as unpatched]
> 
> --- (reboot)
> 
> # time cdparanoia 1
> [...]
> real    1m54.289s
> user    0m6.538s
> sys     0m1.381s
> 
> # md5sum *.wav
> 510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-audio.wav
> 510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cdda.wav
> 510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cddaDMA-audio.wav
> 510e2fb29d9f67c3f80b380bd9b66566  2.6.4-rc2-cddaDMA-cdda.wav

That all looks expected, I think. cdda2wav uses SG_IO so it utilizes
zero copy dma with an unpatched kernel already, my CDROMREADAUDIO dma
patch makes zero difference for that io path (it's already fully
optimized). WRT the difference in run time between cdparanoia w/patched
kernel and cdda2wav, it probably has to do with the various jitter
corrections and scratch resistance stuff that cdparanoia does. If you
disable some of those options, its runtime will probably get a lot
closer to that of cdda2wav.

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Sat, Mar 06 2004, Sean Neakums wrote:
> Jens Axboe <axboe@suse.de> writes:
> 
> > Hi,
> >
> > 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
> > optimal of course... This patch uses the block layer infrastructure to
> > enable zero copy DMA ripping through CDROMREADAUDIO.
> >
> > I'd appreciate people giving this a test spin. Patch is against
> > 2.6.4-rc1 (well current BK, actually).
> 
> Applied successfully to 2.6.4-rc1-mm2, and it works great.  For some
> reason, on two different machines, ripping with cdparanoia used to
> somehow crowd out the serial port, but now everything just works.

cd ripping was highly cpu intensive when it ran in pio, so it's very
likely that this screwed up your serial port communication. It doesn't
matter with the patch, but had you used hdparm -u1 on your cd device
on an unpatched kernel, you would have had better luck.

-- 
Jens Axboe

Re: [PATCH] 2.6 ide-cd DMA ripping

[Sean Neakums]

Jens Axboe <axboe@suse.de> writes:

> On Sat, Mar 06 2004, Sean Neakums wrote:
>> Jens Axboe <axboe@suse.de> writes:
>> 
>> > Hi,
>> >
>> > 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
>> > optimal of course... This patch uses the block layer infrastructure to
>> > enable zero copy DMA ripping through CDROMREADAUDIO.
>> >
>> > I'd appreciate people giving this a test spin. Patch is against
>> > 2.6.4-rc1 (well current BK, actually).
>> 
>> Applied successfully to 2.6.4-rc1-mm2, and it works great.  For some
>> reason, on two different machines, ripping with cdparanoia used to
>> somehow crowd out the serial port, but now everything just works.
>
> cd ripping was highly cpu intensive when it ran in pio, so it's very
> likely that this screwed up your serial port communication. It doesn't
> matter with the patch, but had you used hdparm -u1 on your cd device
> on an unpatched kernel, you would have had better luck.

I had a look, just for pig iron, and hdparm -u on one of the machines
reports that it is already enabled.  That machine is SMP with two
1.13GHz PIIIs.  I can't check the other machine as the drive in
question is no longer functional.

Re: [PATCH] 2.6 ide-cd DMA ripping

[Jens Axboe]

On Sun, Mar 07 2004, Sean Neakums wrote:
> Jens Axboe <axboe@suse.de> writes:
> 
> > On Sat, Mar 06 2004, Sean Neakums wrote:
> >> Jens Axboe <axboe@suse.de> writes:
> >> 
> >> > Hi,
> >> >
> >> > 2.6 still uses PIO for CDROMREADAUDIO cdda ripping, which is less than
> >> > optimal of course... This patch uses the block layer infrastructure to
> >> > enable zero copy DMA ripping through CDROMREADAUDIO.
> >> >
> >> > I'd appreciate people giving this a test spin. Patch is against
> >> > 2.6.4-rc1 (well current BK, actually).
> >> 
> >> Applied successfully to 2.6.4-rc1-mm2, and it works great.  For some
> >> reason, on two different machines, ripping with cdparanoia used to
> >> somehow crowd out the serial port, but now everything just works.
> >
> > cd ripping was highly cpu intensive when it ran in pio, so it's very
> > likely that this screwed up your serial port communication. It doesn't
> > matter with the patch, but had you used hdparm -u1 on your cd device
> > on an unpatched kernel, you would have had better luck.
> 
> I had a look, just for pig iron, and hdparm -u on one of the machines
> reports that it is already enabled.  That machine is SMP with two
> 1.13GHz PIIIs.  I can't check the other machine as the drive in
> question is no longer functional.

Then isr runtime was likely too high, even with interrupt masking
enabled. So just be glad that it works with dma :)

-- 
Jens Axboe