Re: [PATCH v9 3/5] libata: support concurrent positioning ranges log

From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>,
	Damien Le Moal <damien.lemoal@wdc.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, linux-ide@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, linux-renesas@vger.kernel.org
Subject: Re: [PATCH v9 3/5] libata: support concurrent positioning ranges log
Date: Tue, 2 Nov 2021 20:42:07 +0900	[thread overview]
Message-ID: <63c29948-24ac-1cc3-5c1a-1e5b82c9b19f@opensource.wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2111021130020.2311589@ramsan.of.borg>

On 2021/11/02 19:40, Geert Uytterhoeven wrote:
>  	Hi Damien,
> 
> On Wed, 27 Oct 2021, Damien Le Moal wrote:
>> Add support to discover if an ATA device supports the Concurrent
>> Positioning Ranges data log (address 0x47), indicating that the device
>> is capable of seeking to multiple different locations in parallel using
>> multiple actuators serving different LBA ranges.
>>
>> Also add support to translate the concurrent positioning ranges log
>> into its equivalent Concurrent Positioning Ranges VPD page B9h in
>> libata-scsi.c.
>>
>> The format of the Concurrent Positioning Ranges Log is defined in ACS-5
>> r9.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> 
> Thanks for your patch, which is now commit fe22e1c2f705676a ("libata:
> support concurrent positioning ranges log") upstream.
> 
> During resume from s2ram on Renesas Salvator-XS, I now see more scary
> messages than before:
> 
>       ata1: link resume succeeded after 1 retries
>       ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>      +ata1.00: qc timeout (cmd 0x2f)
>      +ata1.00: Read log page 0x00 failed, Emask 0x4
>      +ata1.00: ATA Identify Device Log not supported
>      +ata1.00: failed to set xfermode (err_mask=0x40)
>       ata1: link resume succeeded after 1 retries
>       ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>      +ata1.00: ATA Identify Device Log not supported
>      +ata1.00: ATA Identify Device Log not supported
>       ata1.00: configured for UDMA/133
> 
> I guess this is expected?

Nope, it is not. The problem is actually not the concurrent positioning log, or
any other log, being supported or not.

Notice the qc timeout ? On device scan after coming out of sleep, or even simply
doing a rmmod ahci+modprobe ahci, the read log commands issued during device
revalidate timeout fairly easily as they are issued while the drive is not
necessarilly fully restarted yet. These errors happen fairly easily due to the
command timeout setting in libata being too short, I think, for the "restart"
case. On a clean boot, they do not happen as longer timeouts are used in that case.

I identified this problem recently while testing stuff: I was doing rmmod of ata
modules and then modprobe of newly compiled modules for tests and noticed these
timeouts. Increasing the timeout values, they disappear. I am however still
scratching my head about the best way to address this. Still digging about this
to first make sure this is really about timeouts being set too short.

> 
> The hard drive (old Maxtor 6L160M0 that received a third life as a test
> bed for Renesas SATA regression testing) seems to still work fine.

I have plenty of brand new drives in my box that show similar error patterns.
The drive is not at fault and libata recovers so the user may not notice the
error. I didn't notice for a while too...

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>  						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>  							    -- Linus Torvalds
> 

-- 
Damien Le Moal
Western Digital Research