On 22/02/2011, at 3:44 AM, Miod Vallat wrote:
>> we do some scsi ops where we give a device a large buffer and expect
>> it to only fill part of it because we dont know how big that part
>> will be.
>>
>> the underrun handling in mpii always caused this to fail, so devices
>> relying on this didnt work as expected (eg, ses).
>>
>> according to the solaris driver we only get underruns on ok scsi
>> commands, so we dont have to check the NO_SCSI_STATUS bit, we can
>> trust that its ok but with a residual calculation.
>>
>> tests please :)
>
> Are you sure you don't want to /* FALLTHROUGH */ ?
no, because xs->resid will be overwritten.
> It would probably be better to do something like
>
> set xs->resid
> if (NO_SCSI_STATUS)
> xs->status = SCSI_OK; /* otherwise nonsensical value */
> /* FALLTHROUGH */
>
> instead.
the solaris code only lets this through if the scsi status was OK. perhaps
this is better reflects that:
case MPII_IOCSTATUS_SCSI_DATA_UNDERRUN:
switch (xs->status) {
case SCSI_OK:
xs->resid = xs->datalen -
letoh32(sie->transfer_count);
break;
default:
xs->error = XS_DRIVER_STUFFUP;
break;
}
break;
>
>> Index: mpii.c
>> ===================================================================
>> RCS file: /cvs/src/sys/dev/pci/mpii.c,v
>> retrieving revision 1.38
>> diff -u -p -r1.38 mpii.c
>> --- mpii.c 21 Feb 2011 09:36:15 -0000 1.38
>> +++ mpii.c 21 Feb 2011 13:03:15 -0000
>> @@ -4649,11 +4649,7 @@ mpii_scsi_cmd_done(struct mpii_ccb *ccb)
>> switch (letoh16(sie->ioc_status) & MPII_IOCSTATUS_MASK) {
>> case MPII_IOCSTATUS_SCSI_DATA_UNDERRUN:
>> xs->resid = xs->datalen - letoh32(sie->transfer_count);
>> - if (sie->scsi_state & MPII_SCSIIO_ERR_STATE_NO_SCSI_STATUS) {
>> - xs->error = XS_DRIVER_STUFFUP;
>> - break;
>> - }
>> - /* FALLTHROUGH */
>> + break;
>> case MPII_IOCSTATUS_SUCCESS:
>> case MPII_IOCSTATUS_SCSI_RECOVERED_ERROR:
>> switch (xs->status) {
