Re: does scsi_io_completion need to dump sense data for ata pass through (ck_cond = 1) ?

["Krzysztof Błaszkowski" <kb@sysmikro.com.pl>]

On Wednesday 23 December 2009 06:45, Douglas Gilbert wrote:
> Krzysztof Błaszkowski wrote:
> > On Tuesday 22 December 2009 20:44, Douglas Gilbert wrote:
> >> Krzysztof Błaszkowski wrote:
> >>> Hello Eric,
> >>>
> >>> I think it is not necessary and current behavior of kernel like 2.6.31
> >>> confuses QA people.
> >>> They need explanation that message like below:
> >>>
> >>> sd 5:0:1:0: [sdb] Sense Key : Recovered Error [current] [descriptor]
> >>> Descriptor sense data with sense descriptors (in hex):
> >>>         72 01 00 1d 00 00 00 0e 09 0c 00 00 00 00 00 00
> >>>         00 00 00 00 00 50
> >>> sd 5:0:1:0: [sdb] Add. Sense: ATA pass through information available
> >>>
> >>> stands for that sense data contains ata registers file and the command
> >>> was executed properly.
> >>>
> >>> Here is additional condition i used in scsi_io_completion() which
> >>> suppresses this message. Is it okay ?
> >>
> >> Close but I would like to see a filter on the additional
> >> sense code ATA PASS-THROUGH INFORMATION AVAILABLE [0x0, 0x1d].
> >> There is nothing in SAT that precludes the return of a RECOVERED
> >> ERROR sense code for a read on a SATA disk that was recovered
> >> after some extra work (for example).
> >
> > of course but is it good idea to log sense data if:
> > - it is created on demand by application. (sat-2 ch 12.2.2 and .3)
> > - it doesn't contain some failure status
> >
> > ?
> >
> >> With that filter in place there is no need to filter on the command.
> >
> > right but OTOH if filter used asc/ascq then it would filter out all
> > commands even these with some failures.
> >
> > i am not sure if this is really what you want to have, ie block all dumps
> > of ATA PASS-THROUGH sense data.
> >
> >> IOWs just filter on sense code RECOVERED ERROR and additional
> >> sense code ATA PASS-THROUGH INFORMATION AVAILABLE. SAT and SAT-2
> >> compliant implementations should then dump less to the log.
> >
> > i would say that asc/ascq filter will stop logging all SAT/SAT-2 check
> > conditions instead of "less".
> >
> > that improvement i sent has better "granularity".
>
> I disagree.
>
> This is the code sequence I had in mind:
>
>          if (sense_valid && (sshdr.sense_key == RECOVERED_ERROR)) {
>                  /* if ATA PASS-THROUGH INFORMATION AVAILABLE skip
>                   * print since app wants ATA registers */
>                  if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d))
>                          ;
>                  else if (!(req->cmd_flags & REQ_QUIET))
>                          scsi_print_sense("", cmd);
>                  result = 0;
>                  /* BLOCK_PC may have set error */
>                  error = 0;
>          }
>
> This will suppress logging the sense data associated with the
> ATA PASS-THROUGH INFORMATION AVAILABLE additional sense code.
> That is only generated in the situation when a SCSI ATA PASS-THROUGH
> command sets the CK_COND bit _and_ there was no other error from
> the ATA disk or the transport (e.g. SAS can issue an ABORTED COMMAND
> sense key). This is shown in sat2r09.pdf section 12.2.5 table 113.

I think you are right that checking CK_COND bit here is not necessary because 
if command was failed due to drive error then sense key wouldn't be 
RECOVERED_ERROR according to section 11 (sat2).

Krzysztof Blaszkowski
>
> Doug Gilbert
>
> P.S. Haven't seen Eric Youngdale's email address for a while


P.S. I took his address from scsi_lib header (2.6.31.5) and i don't think i 
should bother if this address is valid or invalid or if he wishes to reply or 
he doesn't. This is why i posted my conclusions to linux-scsi too.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html