On Thu, Jun 21, 2012 at 5:23 PM, Praveen Murali <pmurali@xxxxxxxxxxxx> wrote: > On Thu, 2012-06-21 at 17:04 -0700, Dan Williams wrote: >> On Thu, Jun 21, 2012 at 4:42 PM, Praveen Murali <pmurali@xxxxxxxxxxxx> wrote: >> > For my large capacity drives connected to my SAS HBA i get improper >> > sector values when i use hdparm to get the native max. Also, the drives >> > have known bad sectors, but when read fails, the sector information >> > printed in the sense buffer is incorrect. >> > In fill_result_tf() of libata-core.c the flags for the resultant tf is >> > copied from the request tf before calling the qc_fill_rtf() for the >> > driver. In case of SAS we convert the FIS to tf after receiving the >> > response and memcpy this tf over to the result tf in >> > sas_ata_qc_fill_rtf(); which results in overwriting the flags >> > for rtf. As a result, when ata_tf_read_block() is called by >> > ata_gen_ata_sense() ata_tf_read_block() does not find ATA_TFLAG_LBA or >> > ATA_TFLAG_LBA48 in the rtf flags. Hence, the returned block value is not >> > calculated correctly. >> > >> > The fix sets the flags in result tf from tf after calling qc_fill_rtf() >> > to take care of overwrites (as in libsas). Also, if the overwrite was >> > intentional, we will only setup the right bits here. >> >> Hmm, I think libsas is at fault here and should be using >> ata_tf_from_fis() like the other ->qc_fill_rtf() implementations. > libsas calls ata_tf_from_fis in sas_ata.c sas_ata_task_done() to copy > the result FIS. But then it copies this information to sata_dev.tf in > the domain device. It mem copies this (sata_dev.tf) over to the rtf in > sas_ata_qc_fill_rtf(). > Another way to fix this might be to save the flags in sata_dev.tf from > qc->tf in sas_ata_task_done() before/after it calls ata_tf_from_fis(). > This was it can still keep doing the mem copy and the order in libata > need not be changed. > What do you think? tf->flags is an internal detail of libata to record context about the in-flight command. So just fixing sas_ata_qc_fill_rtf() to not trample on the non-fis fields in the tf should be all that is needed, i.e. just a copy from the original qc. -- Dan -- To unsubscribe from this list: 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
