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

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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