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

Tejun Heo
2007-11-03T01:17:36Z

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

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

[-- Attachment #2: drain.patch --]
[-- Type: text/x-patch, Size: 1420 bytes --]

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;
}