On 10/8/22 08:47, Niklas Cassel wrote: > On Sat, Oct 08, 2022 at 07:33:54AM +0900, Damien Le Moal wrote: >> On 10/7/22 22:23, Niklas Cassel wrote: >>> ata_read_log_page() first tries to read the log using READ LOG DMA EXT. >>> If that fails it will instead try to read the log using READ LOG EXT. >>> >>> ata_exec_internal_sg() is synchronous, so it will wait for the command to >>> finish. If we actually got an error back from the device, it is correct >>> to retry. However, if the command timed out, ata_exec_internal_sg() will >>> freeze the port. >>> >>> There is no point in retrying if the port is frozen, as >>> ata_exec_internal_sg() will return AC_ERR_SYSTEM on a frozen port, >>> without ever sending the command down to the drive. >>> >>> Therefore, avoid retrying if the first command froze the port, as that >>> will result in a misleading AC_ERR_SYSTEM error print, instead of printing >>> the error that actually caused the port to be frozen (AC_ERR_TIMEOUT). >> >> Beside ata_read_log_page(), are there any other path that do a retry after >> ata_exec_internal_sg() fails ? > > Let me check and get back to you. > >> >> Another note: this is patch 4/4 but I did not get any patch 3/4... > > It is here: > https://lore.kernel.org/linux-scsi/20221007132342.1590367-1-niklas.cassel@xxxxxxx/T/#t > > My scripts probably didn't add you since you are not a maintainer or > reviewer for drivers/scsi. I should probably have added you manually. Yes please, always send full series. > >> >>> >>> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx> >>> --- >>> drivers/ata/libata-core.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c >>> index 1cf326dd7c41..6ae5787103e7 100644 >>> --- a/drivers/ata/libata-core.c >>> +++ b/drivers/ata/libata-core.c >>> @@ -2000,7 +2000,8 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, >>> if (err_mask) { >>> if (dma) { >>> dev->horkage |= ATA_HORKAGE_NO_DMA_LOG; >>> - goto retry; >>> + if (!ata_port_is_frozen(dev->link->ap)) >>> + goto retry; >>> } >>> ata_dev_err(dev, >>> "Read log 0x%02x page 0x%02x failed, Emask 0x%x\n", >> >> -- >> Damien Le Moal >> Western Digital Research -- Damien Le Moal Western Digital Research
