PATCH/RFC libata-2.6 5/5] Odd request buffer length handling in __atapi_pio_bytes()

PATCH/RFC libata-2.6 5/5] Odd request buffer length handling in __atapi_pio_bytes()

[Albert Lee]

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

Hi Jeff,

Problem:
   When testing the k3b CD burning program, k3b issued a request (read TOC) with 81 bytes of buff length.
The CD-RW device didn't like it and returned 82 bytes of data. Detailed log below:

Mar 17 02:18:56 plinuxt01 kernel: ata_scsi_dump_cdb: CDB (1:0,0,0) 43 00 02 00 00 00 01 00 51
Mar 17 02:18:56 plinuxt01 kernel: ata_scsi_translate: ENTER
Mar 17 02:18:56 plinuxt01 kernel: ata_sg_init: Enter
Mar 17 02:18:56 plinuxt01 kernel: ata_sg_init: Exit
Mar 17 02:18:56 plinuxt01 kernel: ata_dev_select: ENTER, ata1: device 0, wait 1
Mar 17 02:18:56 plinuxt01 kernel: ata_tf_load_pio: feat 0x0 nsect 0x0 lba 0x0 0x0 0x20
Mar 17 02:18:56 plinuxt01 kernel: ata_tf_load_pio: device 0xA0
Mar 17 02:18:56 plinuxt01 kernel: ata_exec_command_pio: ata1: cmd 0xA0
Mar 17 02:18:56 plinuxt01 kernel: ata_scsi_translate: EXIT
Mar 17 02:18:56 plinuxt01 kernel: atapi_packet_task: busy wait
Mar 17 02:18:56 plinuxt01 kernel: atapi_packet_task: send cdb
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: qc->curbytes 0, qc->nbytes 81 bytes 82
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: qc->cursg [0]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344004696, sgoff 0 offset 0
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344004696, sgoff 0 offset 0
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: sg_dma_len(sg) [0] sg->length[81]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 81
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 81, PAGE_SIZE - offset [4096]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: 81 bytes data read
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: goto next sg
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: qc->cursg [1]  => Oops, invalid sg get written.
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344704864, sgoff 0 offset 0
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344704864, sgoff 0 offset 0
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: sg_dma_len(sg) [4096] sg->length[4096]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 1
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 1, PAGE_SIZE - offset [4096]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: 1 bytes data read
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: qc->curbytes 82, qc->nbytes 81 bytes 82
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: qc->cursg [1]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344704864, sgoff 0 offset 1
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: page virtual 13835058055344704864, sgoff 0 offset 1
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: sg_dma_len(sg) [4096] sg->length[4096]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 82
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: count 82, PAGE_SIZE - offset [4095]
Mar 17 02:18:56 plinuxt01 kernel: __atapi_pio_bytes: 82 bytes data read
Mar 17 02:18:56 plinuxt01 kernel: ata_qc_complete: EXIT

Changes:
- When an odd-lengthed buffer is seen, round it to the next even length.
   As atapi_input_bytes() does in the ide-iops.

   I am not sure whether this is the correct. It will overrun the buffer by one byte.
I've tested it on ppc64 without problem. Need comments on this.
Attached please find the patch against the libata-2.6 tree for your review. Thanks.

Albert

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---------------------------------------
--- libata-2.6-extrabytes/drivers/scsi/libata-core.c	2005-03-18 14:00:34.000000000 +0800
+++ libata-2.6-odd/drivers/scsi/libata-core.c	2005-03-18 14:39:24.000000000 +0800
@@ -2361,6 +2361,13 @@
  	/* don't cross page boundaries */
  	count = min(count, (unsigned int)PAGE_SIZE - offset);

+	/* handle the odd condition */
+	if (unlikely(count & 0x01)) {
+		printk(KERN_WARNING "ata%u: odd count %u rounded: qc->nbytes %u, bytes %u\n",
+		       ap->id, count, qc->nbytes, bytes);
+		count++;
+	}
+
  	buf = kmap(page) + offset;

  	qc->curbytes += count;


[-- Attachment #2: oddbuffer.diff --]
[-- Type: text/plain, Size: 554 bytes --]

--- libata-2.6-extrabytes/drivers/scsi/libata-core.c	2005-03-18 14:00:34.000000000 +0800
+++ libata-2.6-odd/drivers/scsi/libata-core.c	2005-03-18 14:39:24.000000000 +0800
@@ -2361,6 +2361,13 @@
 	/* don't cross page boundaries */
 	count = min(count, (unsigned int)PAGE_SIZE - offset);
 
+	/* handle the odd condition */
+	if (unlikely(count & 0x01)) {
+		printk(KERN_WARNING "ata%u: odd count %u rounded: qc->nbytes %u, bytes %u\n", 
+		       ap->id, count, qc->nbytes, bytes);
+		count++;
+	}
+
 	buf = kmap(page) + offset;
 
 	qc->curbytes += count;

Re: PATCH/RFC libata-2.6 5/5] Odd request buffer length handling in __atapi_pio_bytes()

[Albert Lee]

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

Resubmit since the offset changed by patch 4/5.
Also changed printk() in previous patch to DPRINTK().

Albert

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---------------------------------------
--- libata-2.6-extrabytes/drivers/scsi/libata-core.c	2005-03-29 20:23:24.000000000 +0800
+++ libata-2.6-odd/drivers/scsi/libata-core.c	2005-03-29 20:51:52.000000000 +0800
@@ -2370,6 +2370,13 @@
  	/* don't cross page boundaries */
  	count = min(count, (unsigned int)PAGE_SIZE - offset);

+	/* handle the odd condition */
+	if (unlikely(count & 0x01)) {
+		DPRINTK("ata%u: odd count %u rounded: qc->nbytes %u, bytes %u\n",
+			ap->id, count, qc->nbytes, bytes);
+		count++;
+	}
+
  	buf = kmap(page) + offset;

  	qc->curbytes += count;


[-- Attachment #2: oddbuffer.diff --]
[-- Type: text/plain, Size: 536 bytes --]

--- libata-2.6-extrabytes/drivers/scsi/libata-core.c	2005-03-29 20:23:24.000000000 +0800
+++ libata-2.6-odd/drivers/scsi/libata-core.c	2005-03-29 20:51:52.000000000 +0800
@@ -2370,6 +2370,13 @@
 	/* don't cross page boundaries */
 	count = min(count, (unsigned int)PAGE_SIZE - offset);
 
+	/* handle the odd condition */
+	if (unlikely(count & 0x01)) {
+		DPRINTK("ata%u: odd count %u rounded: qc->nbytes %u, bytes %u\n", 
+			ap->id, count, qc->nbytes, bytes);
+		count++;
+	}
+
 	buf = kmap(page) + offset;
 
 	qc->curbytes += count;