On Tue, Aug 23 2005, Douglas Gilbert wrote:
> Jens Axboe wrote:
> > On Mon, Aug 22 2005, Douglas Gilbert wrote:
> >
> >> if (scsicmd[0] == READ_6 || scsicmd[0] == WRITE_6) {
> >>- qc->nsect = tf->nsect = scsicmd[4];
> >>+ if (scsicmd[4] == 0) {
> >>+ /*
> >>+ * For READ_6 and WRITE_6 (only)
> >>+ * transfer_len==0 -> 256 blocks !!
> >>+ */
> >>+ if (lba48) {
> >>+ tf->hob_nsect = 1;
> >>+ qc->nsect = 256;
> >>+ } else
> >>+ return 1;
> >
> >
> > This isn't quite right, for 28-bit lba a 0 sector value means 256
> > sectors to transfer as well. So just make that:
> >
> > if (lba48) {
> > tf->hob_nsect = 1;
> > qc->nsect = 256;
> > }
> >
> > /* continue */
> >
> > and it should work fine. Similarly for 48-bit lba, 0 means 16^2 sectors.
>
> Jens,
> Since for 28-bit lba a 0 sector value means 256 sectors
> do I need to check for the lba48 case at all? As proposed
> to Jeff is this ok (for READ_6 and WRITE_6):
>
> if (scsicmd[4] == 0) {
> /*
> * For READ_6 and WRITE_6 (only)
> * transfer_len==0 -> 256 blocks !!
> */
> qc->nsect = 256;
> } else
> qc->nsect = scsicmd[4];
> tf->nsect = scsicmd[4];
This will break for lba48 devices, since if you have scsicmd[4] == 0, a
lba48 read/write will want to transfer 65536 sectors instead of the
intended 256.
Your qc->nsect logic is correct, but you need to set tf->hob_nsect 1
for lba48 if scsicmd[4] == 0 to correctly tell that command to transfer
256 sectors.
> Also I noticed while testing the original code with READ_6
> (sectors=0) that the device locked up (power cycle required).
> So given the point you make for 48-bit lba, 0 means 16^2
> sectors, then the READ_10 (sectors=0) and READ_16 (sectors=0)
> which are valid nops according to SBC-2 may also lock up
> in libata.
Try with the corrected sector counts, should work. I didn't check the
other READ_X/WRITE_X, so you should probably audit them as well :)
--
Jens Axboe
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
