Re: "Fix ATAPI transfer lengths" causes CD writing regression

[Tejun Heo]

Daniel Drake wrote:
> Tejun Heo wrote:
>> Yeap, the SG command is fine.  The drive is being weird tho.  The
>> allocation length field says 10 bytes, so it should just have
>> transferred 10 bytes without causing HSM violation.
>>
>> Can you please apply the attached patch and report what the kernel says
>> after triggering the error condition?
> 
> Thanks for looking at this, the kernel now says:
> 
> <4>ata2.00: HSM violation: eh_analyze_tf: BUSY|DRQ
> <3>ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
> <3>ata2.00: cmd a0/00:00:00:0a:00/00:00:00:00:00/a0 tag 0 cdb 0x5a data
> 10 in
> <4>         res 58/00:02:00:0a:00/00:00:00:00:00/a0 Emask 0x2 (HSM
> violation)
> <3>ata2.00: status: { DRDY DRQ }
> <6>ata2: soft resetting link
> <6>ata2.00: configured for UDMA/33
> <6>ata2: EH complete

Does this patch fix the problem?

-- 
tejun
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8ee56e5..5488637 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4994,19 +4994,15 @@ static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
 static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
 {
 	int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
-	struct scatterlist *sg = qc->__sg;
-	struct scatterlist *lsg = sg_last(qc->__sg, qc->n_elem);
 	struct ata_port *ap = qc->ap;
+	struct scatterlist *sg;
 	struct page *page;
 	unsigned char *buf;
 	unsigned int offset, count;
-	int no_more_sg = 0;
 
-	if (qc->curbytes + bytes >= qc->nbytes)
-		ap->hsm_task_state = HSM_ST_LAST;
-
-next_sg:
-	if (unlikely(no_more_sg)) {
+ next_sg:
+	sg = qc->cursg;
+	if (unlikely(!sg)) {
 		/*
 		 * The end of qc->sg is reached and the device expects
 		 * more data to transfer. In order not to overrun qc->sg
@@ -5024,13 +5020,9 @@ next_sg:
 
 		for (i = 0; i < words; i++)
 			ap->ops->data_xfer(qc->dev, (unsigned char *)pad_buf, 2, do_write);
-
-		ap->hsm_task_state = HSM_ST_LAST;
 		return;
 	}
 
-	sg = qc->cursg;
-
 	page = sg_page(sg);
 	offset = sg->offset + qc->cursg_ofs;
 
@@ -5068,9 +5060,6 @@ next_sg:
 	qc->cursg_ofs += count;
 
 	if (qc->cursg_ofs == sg->length) {
-		if (qc->cursg == lsg)
-			no_more_sg = 1;
-
 		qc->cursg = sg_next(qc->cursg);
 		qc->cursg_ofs = 0;
 	}