Re: todays git: WARNING: at drivers/ata/libata-sff.c:1017 ata_sff_hsm_move+0x45e/0x750()

James Bottomley
2009-01-10T15:23:16Z

On Sat, 2009-01-10 at 15:12 +0000, Alan Cox wrote:
> O> Your problem is that the *device* is wanting to transfer a set of bytes
> > not divisible by four. If you want to use word based PIO, you'll have
> > to fall back to collecting bytes for the last two. Alternatively, you
>
> Nope.. you can't do that with ATA block transfers - it isn't the same as
> SCSI

Well, in that case you'll have to do byte PIO for all ATAPI devices,
because this is an expected device to host transfer length.

> > could just do byte PIO for all reply lengths like this ... they occur
> > all over the SCSI standard, but not usually in critical paths.
>
> The problem we have is that the sg list the drivers were given had a
> segment which was not divisible in length by four and was *NOT* the last
> segment in the sg list

Yes it was, look at the debugging info:

Christian Borntraeger wrote:

> [ 1.499843] scsi 1:0:0:0: CD-ROM HL-DT-ST DVDRAM
> GSA-U10N 1.05 PQ: 0 ANSI: 5
> [ 1.508389] count: 18 sg->length: 96 qc->cursg_ofs: 0 bytes: 18
> [ 1.509369] qc->cdb[]:03000000600000000000000000000000
> That one is broken.

That's a single element sg list with a 96 byte buffer. That's precisely
what SCSI sets up to receive incoming sense data. There's basically no
setup commands which are multiple element because the sg list is set up
on a kmalloc'd buffer.

The problem is that bytes is 18 and that comes from this:

ap->ops->sff_tf_read(ap, &qc->result_tf);
ireason = qc->result_tf.nsect;
bc_lo = qc->result_tf.lbam;
bc_hi = qc->result_tf.lbah;
bytes = (bc_hi << 8) | bc_lo;

So we're asking the device how much data it wants to transfer. It said
18 bytes. There's nothing padding or aligning in block can do about
this.

> The logic in the ATA PIO code is basically
>
>
> for each sg entry
>
> compute the number of bytes to transfer staying within
> the page
> transfer that many bytes (but may be more)
>
> if and only if the transfer is NOT the last segment but is
> more than the bytes requested - WARN
>
>
> The block alignment is set to 4 bytes so the block code should be handing
> down stuff which is safe. Some of the sense and other stuff is "adjusted"
> by the libata-scsi conversion code which at this point I suspect is the
> offender.

It is. The receive buffer is 96 bytes. It will be page aligned and 96
is divisible by 4.

> This is why libata uses the pad buffers we don't get to be quite so exact
> about transfer sizes as SCSI is.

The block layer does do pad buffers, but in this case there was no need
of one.

James